-
Notifications
You must be signed in to change notification settings - Fork 819
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
feat: add diag warning when metric name is invalid #2122
feat: add diag warning when metric name is invalid #2122
Conversation
If you are using the metric kind `counter` Prometheus expects the metric name to have the suffix `_total` as it will throw the following error: ``` failed_requests counter metrics should have "_total" suffix ``` This commit logs a warning message when the diag log level is DEBUG to assist implementors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## main #2122 +/- ##
=======================================
Coverage 92.75% 92.76%
=======================================
Files 140 140
Lines 4981 4987 +6
Branches 1028 1029 +1
=======================================
+ Hits 4620 4626 +6
Misses 361 361
|
Added unit test :) |
packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
Outdated
Show resolved
Hide resolved
@naseemkullah I have moved it into its own function for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, hope you can address the comment before merging
packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
Outdated
Show resolved
Hide resolved
updated it :) |
Which problem is this PR solving?
If you are using the metric kind
counter
Prometheus expects themetric name to have the suffix
_total
as it will throw the followingerror:
This commit logs a warning message when the diag log level is DEBUG to
assist implementors
Short description of the changes
Adds a log warning when an invalid metric name is given