Skip to content

Conversation

david-gang
Copy link
Contributor

@david-gang david-gang commented Jul 31, 2025

Description

opentelemetry-instrumentation-system-metrics: Add cpython.gc.collected_objects and cpython.gc.uncollectable_objects metrics

Fixes #3649

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update - I updated changelog. Not sure if enough.

How Has This Been Tested?

  • Added unit tests for new metrics. Should be enough but will be happy to get feedback

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated - Not sure if needed but if yes, will be happy for guidance.

@david-gang david-gang requested a review from a team as a code owner July 31, 2025 12:34
@david-gang
Copy link
Contributor Author

david-gang commented Jul 31, 2025

Well murphy's law :-) I had the merit to write the 1001th line

tox -e lint-instrumentation-system-metrics
lint-instrumentation-system-metrics: commands[0]> sh -c 'cd instrumentation && pylint --rcfile ../.pylintrc opentelemetry-instrumentation-system-metrics'
************* Module opentelemetry.instrumentation.system_metrics.init
opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/init.py:1:0: C0302: Too many lines in module (1001/1000) (too-many-lines)

…cted_objects` and `cpython.gc.uncollectable_objects` metrics
@david-gang
Copy link
Contributor Author

Hey @emdneto — WDYT about relaxing the 1000-line limit in this case?

The system_metrics module is a bit of an outlier — it’s a bag of low-level metrics with dispersed logic. I’m currently at 993 lines, and supporting additional metrics cleanly would push me just over 1000.

If relaxing the limit isn’t an option, I can work around it by wrapping the callback function with functools.partial, adding parameters dynamically. Here’s a simplified sketch:

def _get_runtime_gc_stats(
        self, options: CallbackOptions, stat_key: str, labels: dict[str, str]
    ) -> Iterable[Observation]:
        """Observer callback for garbage collection stats"""
        for index, stat in enumerate(gc.get_stats()):
            labels_copy = labels.copy()
            labels_copy["generation"] = str(index)
            yield Observation(stat[stat_key], labels_copy)
             if "cpython.gc.collections" in self._config:
            if self._python_implementation == "pypy":
                _logger.warning(
                    "The cpython.gc.collections metric won't be collected because the interpreter is PyPy"
                )
            else:
                self._meter.create_observable_counter(
                    name="cpython.gc.collections",
                    callbacks=[
                        functools.partial(
                            self._get_runtime_gc_stats,
                            stat_key="collections",
                            labels=self._runtime_gc_collections_labels,
                        )
                    ],
                    description="The number of times a generation was collected since interpreter start.",
                    unit="{collection}",
                )

        if "cpython.gc.collected_objects" in self._config:
            if self._python_implementation == "pypy":
                _logger.warning(
                    "The cpython.gc.collected_objects metric won't be collected because the interpreter is PyPy"
                )
            else:
                self._meter.create_observable_counter(
                    name="cpython.gc.collected_objects",
                    callbacks=[
                        functools.partial(
                            self._get_runtime_gc_stats,
                            stat_key="collected",
                            labels=self._runtime_gc_collected_objects_labels,
                        )
                    ],
                    description="The total number of objects collected since interpreter start.",
                    unit="{object}",
                )

        if "cpython.gc.uncollectable_objects" in self._config:
            if self._python_implementation == "pypy":
                _logger.warning(
                    "The cpython.gc.uncollectable_objects metric won't be collected because the interpreter is PyPy"
                )
            else:
                self._meter.create_observable_counter(
                    name="cpython.gc.uncollectable_objects",
                    callbacks=[
                        functools.partial(
                            self._get_runtime_gc_stats,
                            stat_key="uncollectable",
                            labels=self._runtime_gc_uncollectable_objects_labels,
                        )
                    ],
                    description="The total number of uncollectable objects found since interpreter start.",
                    unit="{object}",
                )

But I’m not sure this workaround justifies the tradeoff in readability and complexity for a few lines saved.

Two ideas I’m considering:
1. Allowing an exception to the 1000-line rule for this file, given its nature and role.
2. Splitting it — e.g., moving all callback definitions to a system_metrics_callbacks.py module. But this feels like a repo-level architectural decision, and I’d prefer not to do that unilaterally without guidance on the preferred direction.

Thoughts?

@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Aug 4, 2025
@xrmx
Copy link
Contributor

xrmx commented Aug 19, 2025

Hey @emdneto — WDYT about relaxing the 1000-line limit in this case?

The system_metrics module is a bit of an outlier — it’s a bag of low-level metrics with dispersed logic. I’m currently at 993 lines, and supporting additional metrics cleanly would push me just over 1000.

...

Two ideas I’m considering:

  1. Allowing an exception to the 1000-line rule for this file, given its nature and role.
  2. Splitting it — e.g., moving all callback definitions to a system_metrics_callbacks.py module. But this feels like a repo-level architectural decision, and I’d prefer not to do that unilaterally without guidance on the preferred direction.

Thoughts?

  1. is fine

Copy link
Member

@emdneto emdneto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @david-gang sorry! I'm fine with option 1.

@xrmx xrmx requested a review from emdneto August 22, 2025 13:17
@xrmx xrmx enabled auto-merge (squash) August 23, 2025 13:10
@xrmx xrmx merged commit 973d10d into open-telemetry:main Aug 23, 2025
632 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add the cpython gc object statistics
3 participants