Prometheus client metrics support

[libretto]

This code provides the option to use Prometheus for collecting statistics in Karapace. The branch is based on karapace-metrics branch.

[libretto]

@jjaakola-aiven @aiven-anton could You review this PR too?

[aiven-anton]

Hello @libretto,

Sorry for the long wait for a response here. We've convened internally about how to move forward with your contribution, and I'll summarize it here.

There are three issues we that must be addressed for this to be mergable:

• Expose the Prometheus exporter under existing server instead of starting a separate one. Remove the configuration options for Prometheus host and port. Let's leave this as-is.

• Incorrect metric semantics. This has been iterated a couple times in review already. This means for instance that request-size needs to be changed into two separate metrics: request_size_total + request_count`. This applies to other metrics in the PR as well.

• Remove the psutil connection count probing. This should rather be gathered by an OS-level metric, we don't want the application probing the OS for this itself. If the application is to report this, it needs to be based on some application level metric, like some actual internal counter of handled requests, and not by probing the OS for amount of process connections.

  Since the psutil and schedule dependencies were brought in for this purpose only, let's also skip adding those.

• Let's skip the updates to README.md, so that we can let these changes be undocumented and subject to change while we're stabilizing it.

Since the branches have diverged, let's also close the original PR now, and make any future changes to this branch only.

[libretto]

Hello @aiven-anton, I just try answer Your comments:

Hello @libretto,

Sorry for the long wait for a response here. We've convened internally about how to move forward with your contribution, and I'll summarize it here.

There are three issues we that must be addressed for this to be mergable:

* ~Expose the Prometheus exporter under existing server instead of starting a separate one. Remove the configuration options for Prometheus host and port.~ _Let's leave this as-is._

* Incorrect metric semantics. This has been iterated a couple times in review already. This means for instance that `request-size` needs to be changed into two separate metrics: `request_size_total + `request_count`. This applies to other metrics in the PR as well.

The metric semantics applied in this context are determined by the SchemaRegistry product, as detailed in their documentation. To achieve exact compatibility with SchemaRegistry, adherence to their specified metric names is necessary.

* Remove the psutil connection count probing. This should rather be gathered by an OS-level metric, we don't want the application probing the OS for this itself. If the application is to report this, it needs to be based on some application level metric, like some actual internal counter of handled requests, and not by probing the OS for amount of process connections.

Is there a method to determine the number of connections in the Karapace application? I haven't been able to find a way to access the list of connections within our app. Is it possible I overlooked something?

  Since the `psutil` and `schedule` dependencies were brought in for this purpose only, let's also skip adding those.

* Let's skip the updates to README.md, so that we can let these changes be undocumented and subject to change while we're stabilizing it.

Ok

Since the branches have diverged, let's also close the original PR now, and make any future changes to this branch only.

Ok

[aiven-anton]

@libretto

The metric semantics applied in this context are determined by the SchemaRegistry product, as detailed in their documentation. To achieve exact compatibility with SchemaRegistry, adherence to their specified metric names is necessary.

Metrics is not an area where we'll aim for 1-1 compatibility with Confluent Schema Registry.

Is there a method to determine the number of connections in the Karapace application? I haven't been able to find a way to access the list of connections within our app. Is it possible I overlooked something?

It's possible that this would have to be developed. For now, I would I just skip this and address it later. Do note that we have a long-term plan to switch from the homegrown web framework to FastAPI, so any such work might become moot.

[libretto]

• Incorrect metric semantics. This has been iterated a couple times in review already. This means for instance that request-size needs to be changed into two separate metrics: request_size_total + request_count`. This applies to other metrics in the PR as well.

@aiven-anton Do You mean it must be coded in the following way?

      
def request(self, size: int) -> None:
        self.request_size_total += size
        self.request_count += 1
        if not self.is_ready or self.stats_client is None:
            return
        if not isinstance(self.stats_client, StatsClient):
            raise RuntimeError("no StatsClient available")
        self.stats_client.gauge("request-size-total", self.request_size_total)
        self.stats_client.gauge("request-count", self.request_count) 

[aiven-anton]

@libretto Yes, with the exception that these should be counters, and not gauges. So I'd expect to see calls to .increase() instead of .gauge().

[libretto]

@aiven-anton done. Review please.

[libretto]

BTW. Is the usage of .gauge() and .timing() in the following functions acceptable?

def are_we_master(self, is_master: bool) -> None:
    if not self.is_ready or self.stats_client is None:
        return
    if not isinstance(self.stats_client, StatsClient):
        raise RuntimeError("no StatsClient available")
    self.stats_client.gauge("master-slave-role", int(is_master))


def latency(self, latency_ms: float) -> None:
    if not self.is_ready or self.stats_client is None:
        return
    if not isinstance(self.stats_client, StatsClient):
        raise RuntimeError("no StatsClient available")
    self.stats_client.timing("latency_ms", latency_ms)

[eliax1996]

Can we have some tests?

[libretto]

Can we have some tests?

We can certainly add some tests. However, the current version of Karapace Stats lacks test coverage, so we have no existing framework to guide us. At this point, it seems that only unit tests are feasible. I'm unsure how to implement integration tests for this part task.

[eliax1996]

Can we have some tests?

We can certainly add some tests. However, the current version of Karapace Stats lacks test coverage, so we have no existing framework to guide us. At this point, it seems that only unit tests are feasible. I'm unsure how to implement integration tests for this part task.

Let's kick off with the unit tests. I'll provide an example soon on how to execute an integration test. It's crucial to have a tangible test that ensures our output its compliant to the standard format.
We need to verify that compliant consumers can effectively utilize and extract the metrics.

Without integration tests, even if the feature is technically sound, we lack a mechanism preventing us from the introduction of code that might break the consumers