-
Notifications
You must be signed in to change notification settings - Fork 650
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
metrics: Remove LabelSet #527
metrics: Remove LabelSet #527
Conversation
Remove LabelSet according to OTEP 90: https://github.com/open-telemetry/oteps/blob/master/text/0090-remove-labelset-from-metrics-api.md
Codecov Report
@@ Coverage Diff @@
## master #527 +/- ##
==========================================
- Coverage 89.56% 89.47% -0.09%
==========================================
Files 43 43
Lines 2213 2195 -18
Branches 249 247 -2
==========================================
- Hits 1982 1964 -18
Misses 159 159
Partials 72 72
Continue to review full report at Codecov.
|
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.
Minor comments 👍
...opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/metrics_exporter/__init__.py
Show resolved
Hide resolved
labels = {"key": "value"} | ||
key_labels = tuple(sorted(labels.items())) |
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.
These lines can be moved outside of the for
loop.
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.
Right, moved outside.
@@ -70,13 +70,13 @@ | |||
# is essentially metric data that corresponds to a specific set of labels. | |||
# Therefore, getting a bound metric instrument using the same set of labels | |||
# will yield the same bound metric instrument. | |||
bound_requests_counter = requests_counter.bind(label_set) | |||
bound_requests_counter = requests_counter.bind(labels) | |||
bound_requests_counter.add(100) | |||
time.sleep(5) | |||
|
|||
print("Updating using batch calling convention...") | |||
# You can record metrics in a batch by passing in a labelset and a sequence of | |||
# (metric, value) pairs. The value would be recorded for each metric using the | |||
# specified labelset for each. |
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.
# specified labelset for each. | |
# specified labels for each. |
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.
Minor comments. LGTM!
Co-Authored-By: Leighton Chen <lechen@microsoft.com>
Remove LabelSet according to OTEP 90:
https://github.com/open-telemetry/oteps/blob/master/text/0090-remove-labelset-from-metrics-api.md
Fixes #518.