process runtime gc count should not have bytes metrics

[david-gang]

Describe your environment

Not relevant

What happened?

In

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py

Line 460 in a912c9e

if "process.runtime.gc_count" in self._config:

        if "process.runtime.gc_count" in self._config:
            if self._python_implementation == "pypy":
                _logger.warning(
                    "The process.runtime.gc_count metric won't be collected because the interpreter is PyPy"
                )
            else:
                self._meter.create_observable_counter(
                    name=f"process.runtime.{self._python_implementation}.gc_count",
                    callbacks=[self._get_runtime_gc_count],
                    description=f"Runtime {self._python_implementation} GC count",
                    unit="By",
                )

the unit is bytes. This results in a metrics in prometheus called: process_runtime_cpython_gc_count_bytes_total

but this is incorrect. we are counting gc cycles per generation through

    def _get_runtime_gc_count(
        self, options: CallbackOptions
    ) -> Iterable[Observation]:
        """Observer callback for garbage collection"""
        for index, count in enumerate(gc.get_count()):
            self._runtime_gc_count_labels["count"] = str(index)
            yield Observation(count, self._runtime_gc_count_labels.copy())

Steps to Reproduce

just see the code

Expected Result

not having a bytes suffix.

it should be something like process_runtime_cpython_gc_count_total

Actual Result

process_runtime_cpython_gc_count_bytes_total

Additional context

the change would break lots of stuff and i am not sure people wil be happy.

Would you like to implement a fix?

Yes

[david-gang]

I would like to implement this style: https://github.com/open-telemetry/semantic-conventions/blob/2bc97890c1ad82232745b4dd74fd3144476b6a5c/docs/runtime/cpython-metrics.md

it also aligns with what the prometheus client does.

But i would like to get an approval of a core contributor before starting the work, because i want to get it merged. especially due to the fact that it will break the existing metrics.

[emdneto]

@xrmx since you introduced those metrics to semconv: I don't mind adding them here while keeping the "old one", similar to what was done for process.runtime

[xrmx]

@xrmx since you introduced those metrics to semconv: I don't mind adding them here while keeping the "old one", similar to what was done for process.runtime

Yeah, I forgot to comment there about using the cpython one instead of the process.runtime ones. As @emdneto suggests we can add them on top of the current ones so that people can switch to the newer one before dropping the deprecated ones.

[david-gang]

@emdneto , please let me know if you are planning to work on this, if not i will try to get some time next week to work on this issue.

[emdneto]

@emdneto , please let me know if you are planning to work on this, if not i will try to get some time next week to work on this issue.

Please feel free to open the PR