kprom: Multiple clients

[lukasrynt]

Issue

The main goal of this PR is to add the ability for kprom plugin to differentiate between multiple clients.

The current implementation of the WithClientLabel() option in the kprom package assigns metrics to the most recently registered kgo client, which leads to metrics being grouped under a static client_id. The goal is to keep the existing API but make the client_id label dynamic, allowing it to reflect the actual client making the request rather than being statically set at first registration (see twmb#815).

Only a subset of metrics will be affected by this change:

• write_bytes_total
• write_errors_total
• write_wait_seconds
• write_time_seconds
• read_bytes_total
• read_errors_total
• read_time_seconds
• request_duration_e2e_seconds
• request_throttled_seconds
• produce_compressed_bytes_total
• produce_(uncompressed_)bytes_total
• produce_batches_total
• produce_records_total
• fetch_compressed_bytes_total
• fetch_(uncompressed_)bytes_total
• fetch_batches_total
• fetch_records_total

Metrics that will continue to have a constant client_id:

• connects_total
• connect_errors_total
• disconnects_total
• buffered_fetch_records_total
• buffered_produce_records_total

Testing

I tested the updated metrics locally using a combination of consumers and producers. When scraping Prometheus, the metrics are now properly separated by the client_id label, even when writing to and reading from the same topic using different client IDs for two producers and consumers:

# HELP test_buffered_fetch_records_total Total number of records buffered within the client ready to be consumed
# TYPE test_buffered_fetch_records_total gauge
test_buffered_fetch_records_total{client_id="primary"} 0
# HELP test_buffered_produce_records_total Total number of records buffered within the client ready to be produced
# TYPE test_buffered_produce_records_total gauge
test_buffered_produce_records_total{client_id="primary"} 0
# HELP test_connects_total Total number of connections opened
# TYPE test_connects_total counter
test_connects_total{client_id="primary",node_id="1"} 8
test_connects_total{client_id="primary",node_id="seed_0"} 4
# HELP test_fetch_bytes_total Total number of uncompressed bytes fetched
# TYPE test_fetch_bytes_total counter
test_fetch_bytes_total{client_id="primary",node_id="1",topic="primary"} 800
test_fetch_bytes_total{client_id="secondary",node_id="1",topic="primary"} 800
# HELP test_produce_bytes_total Total number of uncompressed bytes produced
# TYPE test_produce_bytes_total counter
test_produce_bytes_total{client_id="primary",node_id="1",topic="primary"} 400
test_produce_bytes_total{client_id="secondary",node_id="1",topic="primary"} 400
# HELP test_read_bytes_total Total number of bytes read
# TYPE test_read_bytes_total counter
test_read_bytes_total{client_id="primary",node_id="1"} 14029
test_read_bytes_total{client_id="primary",node_id="seed_0"} 1166
test_read_bytes_total{client_id="secondary",node_id="1"} 13876
test_read_bytes_total{client_id="secondary",node_id="seed_0"} 1166
# HELP test_write_bytes_total Total number of bytes written
# TYPE test_write_bytes_total counter
test_write_bytes_total{client_id="primary",node_id="1"} 10080
test_write_bytes_total{client_id="primary",node_id="seed_0"} 172
test_write_bytes_total{client_id="secondary",node_id="1"} 10072
test_write_bytes_total{client_id="secondary",node_id="seed_0"} 180

Implementation

Since kgo doesn’t export client_id in a way that supports dynamic labeling in hooks, I had to make some modifications to the core library. My goal was to minimize changes to the kgo package itself and to avoid breaking any existing APIs (such as hooks). To that end, I chose not to add dynamic labels to the connection and buffered metrics, keeping those labels static.

[twmb]

I forgot this included changes to kgo when prepping for the release I just cut. I'm aiming to work on the next batch of requests (caching improvements largely) for 1.19 over the next month, but that's going to delay my merge of this. Really sorry about missing that.

In the meantime, my first comment is ClientID is pointer to ID of the client that made the request => these aren't pointers (maybe you started with a pointer); the comment can be touched up. I'll review the rest of this more closely soon.

The main thing I'll want to ensure is that the default metrics do not change for anybody (no breaking change); any changes here will be opt in.

[vtolstov]

may be create additional method to create hooks with some predefined labels? like NewHook(labels ...string) , so we can create hook that already populate labels for all metrics ? And default behaviour - fill client_id label like now?