Metric names are mutated/sanitized for Kafka Micrometer non-universally

[ctoestreich]

TLDR;

• Why are the kafka metrics sanitized?
• Is this approach applied to ALL metrics across ALL scopes, or just to kafka?
• Why would this not be done with an appropriate micrometer NamingConvention?

Details

For some reason Kafka metrics in Boot names are sanitzed replacing - with . via this function. Why is this done and why is this not done universally? Should these names be coordinated with the Micrometer team to change these if the names using - are not adequate. The replacement of these characters mutates the names in Micrometer. Micrometer provides NameConversion strategies that will then mutate these even further. Here is one example of the core metrics library usage of dash names.

Micrometer Kafka Names

Boot Mixed Name Conventions

Kafka metrics from boot are mixed usage of - and . which doesn't even align with the above intention.

[ctoestreich]

@sobychacko Looks like you added this method. Do you have context for these questions?

[sobychacko]

@ctoestreich Thanks for reporting this. I will take a closer look soon and get back to you.

[sobychacko]

@ctoestreich What kind of issues that you are running into with this sanitization done in the binder? The code that you are referring to is specific to the Kafka Streams binder. I believe that sanitization was introduced to avoid some clashing with the regular Kafka binder. Kafka Streams binder metrics are published as the metrics key + "." + metric. If you have suggestions or if you want to submit a PR fixing any issues, that is quite welcomed.

[sobychacko]

@ctoestreich Eventually, I would like to integrate with the native Kafka Streams metrics support in micrometer when micrometer 1.4.0 is available through Boot dependency. See this issue. When it was added to the binder, this was not available. Until we can do that, in the meantime we should address your issue. Please chime in with more details on what kind of issues you are running into and any potential suggestions for fixing them. Thank you.

[ctoestreich]

Metrics work fine of course, but the names that are produced out of that class above use a different naming standard than the micrometer names. There is not parity with names present in micrometer library for Kafka for ease of use in metrics aggregation and time series reporting systems.

Is the intent to use the micrometer names for Kafka post micrometer 1.4.0 integration? This will change the names, which I think is correct, but all current dashboards using this metrics will break.

[sobychacko]

@ctoestreich Thanks for the response. As I alluded to above, when we added these metrics feature to Kafka Streams binder, micrometer did not have native Kafka Streams metrics capabilities. They only added that as an incubating feature in 1.4.0 from what I understand. I assume, therefore, it will be a few more release cycles on our end to properly integrate with that native micrometer feature (will have to wait until Spring Boot picks that up). Most likely, this integration will be part of the next major release of Spring Cloud Stream. I understand that the existing dashboards using these metrics will break at that time, but if we want to change the current ad hoc micrometer metrics implementation in the Kafka Streams binder in Spring Cloud Stream, then we have to do that. At least, we can wait until the next major release for this integration, giving time for downstream users to adjust. What do you think about that approach? Is it viable?

In any case, unless you are using the Kafka Streams binder, then it should not be an issue. You will see the metrics that you are referring to only if you are using the Kafka Streams binder in the application. When implementing this solution in the binder, we used the same sanitizing approach that Micrometer uses internally (i.e. changing dashes to dots), but now I understand that approach may have consequences when sanitizing synthetic tags that we provide by combining the metrics key and tag.

Can you accommodate with the existing tags until we integrate natively with Micrometer for Kafka Streams?

[tzolov]

@ctoestreich, first I will try to answer your questions and clarify how the Micrometer metrics naming works. Then we will look into the issue about inconsistent naming.

Why are the kafka metrics sanitized?

The Micrometer naming meters employs a convention that separates lowercase words with a '.' (dot) character. That means that internally Micrometer expects all meeter names to follow this unified naming convention.

You can see that the Micrometer's own Metrics Binder implementations use the same sanitization rules:
KafkaConsumerMetrics#sanitize
The KafkaConsumerMetrics provided by Micrometer 1.3.x doesn't expose metrics for Kafka Streams. The purpose of the KafkaStreamsBinderMetrics is to fill this gap until Micrometer 1.4.0 become part of the Spring Boot.
The KafkaConsumerMetrics uses MBean/JMX for extracting the common Kafka metrics while the KafkaStreamsBinderMetrics leverages the proprietary metric infrastructure provided by Kafka Streams. Still both Metrics Binders should provide the same unified dot-based meter names.

Is this approach applied to ALL metrics across ALL scopes, or just to kafka?

Not sure what all scopes mean but as pointed above all meter names registered with Micrometer should follow the convention of separated lowercase words with a '.' (dot) character.

Why would this not be done with an appropriate micrometer NamingConvention?

While internally the Micrometer expects all meter names to follow the dotted convention, externally it allows to convert the internal names to formats expected by the target Monitoring system (e.g Influxdb, Prometheus, Atlas ..).
The NamingConvention is used to implement this conversion from the internal representation to the format of the target monitoring system. Check the http.server.requests example in the documentation

@ctoestreich please let me know if this answer your questions so we can move to the potential issue of inconsistent naming you have observed?

[sobychacko]

@ctoestreich Any feedback that you want to provide based on what @tzolov commented above?

[sabbyanandan]

Closing due to no activity.