-
Notifications
You must be signed in to change notification settings - Fork 739
add gc_count metrics with correct collection unit . #3617
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
base: main
Are you sure you want to change the base?
add gc_count metrics with correct collection unit . #3617
Conversation
@@ -469,6 +470,12 @@ def _instrument(self, **kwargs: Any): | |||
description=f"Runtime {self._python_implementation} GC count", | |||
unit="By", | |||
) | |||
self._meter.create_observable_counter( | |||
name=f"process.runtime.{self._python_implementation}.gc_count", |
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.
I think the name should be cpython.gc.collections
no?
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.
thanks for the catch. added this metrics name. this also aligns to your other comment. Now it gets its own if
@@ -469,6 +470,12 @@ def _instrument(self, **kwargs: Any): | |||
description=f"Runtime {self._python_implementation} GC count", | |||
unit="By", | |||
) | |||
self._meter.create_observable_counter( |
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.
Since it doesn't work with pypy, can we skip for pypy from here?
Short question: Should we comment that the old metric is deprecated ? |
d4d91eb
to
4003bb0
Compare
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #3549
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
The change is trivial as
opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py
Line 952 in b74633a
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.
I try to keep this pr as small as possible. After this is merged i want to add the cpython.gc.collected_objects and cpython.gc.uncollectable_objects metrics from https://github.com/open-telemetry/semantic-conventions/blob/2bc97890c1ad82232745b4dd74fd3144476b6a5c/docs/runtime/cpython-metrics.md#metric-cpythongccollected_objects