Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-monotonic kafka metric reported as counter #3300

Closed
jack-berg opened this issue Jul 18, 2022 · 3 comments · Fixed by #3496
Closed

Non-monotonic kafka metric reported as counter #3300

jack-berg opened this issue Jul 18, 2022 · 3 comments · Fixed by #3496
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Milestone

Comments

@jack-berg
Copy link

Describe the bug
KafkaMetrics converts the metric kafka.consumer.connection.count to a counter despite the number of connections not being monotonically increasing.

This is causing issues in in the opentelemetry micrometer shim, which relies counters being monotonically increasing for a proper translation to the opentelemetry data model.

Environment

  • Micrometer version: latest
  • Micrometer registry: opentelemetry micrometer shim
  • OS: any
  • Java version: any

To Reproduce
Run the KafkaClientMetricsIntegrationTest and observe that the printed meters should kafka.consumer.connection.count is a counter where it should probably be a gauge.

Expected behavior
kafka.consumer.connection.count and other non-monotonically increasing kafka metrics should probably be gauges instead of counters.

Originally discussed in CNCF slack here.

@shakuzen shakuzen added bug A general bug module: micrometer-core An issue that is related to our core module labels Jul 19, 2022
@shakuzen shakuzen modified the milestones: 1.8.9, 1.8.x Jul 19, 2022
@shakuzen
Copy link
Member

Thank you for reporting the issue.
It looks like it would be correct for us to remove the OR conditional checking for names ending in count when deciding whether to make a counter or gauge since all the names ending in count appear to be an active count (gauge).

if (meterName.endsWith("total") || meterName.endsWith("count")) {

@jack-berg
Copy link
Author

It looks like it would be correct for us to remove the OR conditional checking for names ending in count when deciding whether to make a counter or gauge

That does appear to be the case. I implemented something similar in opentelemetry recently and inspected the type of the KafkaMetric's org.apache.kafka.common.metrics.Measurable, and defined a mapping from measurable implementation to corresponding instrument type. This seemed safer to me than relying on naming naming conventions.

@shakuzen
Copy link
Member

That does sound like a safer way to do it. Thank you for sharing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants