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

Correct tag names according to Micrometer's docs #645

Conversation

seckin206
Copy link
Contributor

@seckin206 seckin206 commented Mar 21, 2022

See. https://micrometer.io/docs/concepts#_tag_naming

With the current implementation and using official prometheus exporter,
we end up with label names methodType and statusCode. These labels
don't follow the snake_case naming convention that prometheus follows
for labels.
Using dot for word separator makes micrometer automatically convert
label names to snake case out of the box.
See. https://github.com/micrometer-metrics/micrometer/blob/main/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusNamingConvention.java

See. https://micrometer.io/docs/concepts#_tag_naming

With the current implementation and using official prometheus exporter,
we end up with label keys "methodType" and "statusCode". These labels
don't follow the snake_case naming convention that prometheus follows
for labels.
Using dot for word separator makes micrometer automatically convert
label names to snake case correctly for different exporters
out of the box.
@ST-DDT
Copy link
Collaborator

ST-DDT commented Mar 21, 2022

This looks good to me.
Please also create a PR for micrometer and link it here:
https://github.com/micrometer-metrics/micrometer/blob/e4dfd439d20d36a778e688f2f21d9a84fafcbeaf/micrometer-binders/src/main/java/io/micrometer/binder/grpc/AbstractMetricCollectingInterceptor.java#L52-L59
As this implementation will be replaced with the upstream one mid term.

@ST-DDT ST-DDT added the enhancement A feature request or improvement label Mar 21, 2022
@seckin206
Copy link
Contributor Author

Opened the PR against micrometer: micrometer-metrics/micrometer#3088

@yidongnan yidongnan merged commit 1485544 into grpc-ecosystem:master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants