-
Notifications
You must be signed in to change notification settings - Fork 821
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!: remove label keys as they are no longer part of the spec #1126
feat!: remove label keys as they are no longer part of the spec #1126
Conversation
Signed-off-by: Naseem <[email protected]>
af0d3dc
to
6b50ba9
Compare
Codecov Report
@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
- Coverage 92.32% 92.31% -0.02%
==========================================
Files 116 116
Lines 3402 3395 -7
Branches 689 688 -1
==========================================
- Hits 3141 3134 -7
Misses 261 261
|
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.
Added a few comments.
packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts
Outdated
Show resolved
Hide resolved
Are you planning to update examples (https://github.com/open-telemetry/opentelemetry-js/blob/master/examples/prometheus/index.js#L23) in separate PR? |
Co-authored-by: Mayur Kale <[email protected]>
Ah, no, will do it here, thanks. edit: done |
Signed-off-by: Naseem <[email protected]>
…etry-js into remove-label-keys
…(_batchMap) Signed-off-by: Naseem <[email protected]>
Signed-off-by: Naseem <[email protected]>
4d0730d
to
9a3b37d
Compare
@open-telemetry/javascript-approvers would like to get more reviews on this. |
@@ -19,7 +19,6 @@ const meter = new MeterProvider({ | |||
|
|||
const otelCpuUsage = meter.createObserver('metric_observer', { |
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.
out of curiosity, how am I able to create a labels at this moment after this change. What would be a correct way of doing it now, if for any reason I want to have labels ?
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.
You can't. Labels are now applied only by binding them to an instrument, or calling the instrument method with labels as the second argument like add(10, { core: 1 })
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
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
…-telemetry#1126) Co-authored-by: Mayur Kale <[email protected]>
…-telemetry#1126) Co-authored-by: Mayur Kale <[email protected]>
Which problem is this PR solving?
labelKeys
from metric creation #1119Short description of the changes
Removed or replaced all occurrences of
labelKeys
Removed or replaced all occurrences of
prom-clients
'slabelValues
Removed no longer used
private _getLabelValues()
All metrics now have all labels associated to them, tests adjusted accordingly