open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
692 stars 569 forks source link

Update OS process metrics name according to semconv #2662

Open emdneto opened 1 month ago

emdneto commented 1 month ago

What problem do you want to solve?

Some OS process metrics are being captured in system-metrics instrumentations with a runtime prefix. Following this, it seems we should decide if we set the metric name independent of runtime.

Describe the solution you'd like

OS process metrics following the semantic conventions and no runtime prefix in system-metrics-instrumentation

Describe alternatives you've considered

No response

Additional Context

A result of a discussion https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2652#discussion_r1664550127

Would you like to implement a fix?

None

arunk1988 commented 1 month ago

@emdneto May i take this one

emdneto commented 1 month ago

@arunk1988 Sure. Please open a PR!

arunk1988 commented 1 month ago

@emdneto I am still working on this and made some progress. how does this snippet look, please advise.

def test_process_metrics_instrument(self): process_config = { "process.memory": ["rss", "vms"], "process.cpu.time": ["user", "system"], "process.thread.count": None, "process.cpu.utilization": None, "process.context_switches": ["involuntary", "voluntary"], }

    if self.implementation != "pypy":
        process_config["process.gc_count"] = None

    reader = InMemoryMetricReader()
    meter_provider = MeterProvider(metric_readers=[reader])
    process_metrics = SystemMetricsInstrumentor(config=process_config)
    process_metrics.instrument(meter_provider=meter_provider)

    metric_names = []
    for resource_metrics in reader.get_metrics_data().resource_metrics:
        for scope_metrics in resource_metrics.scope_metrics:
            for metric in scope_metrics.metrics:
                metric_names.append(metric.name)

    observer_names = [
        "process.memory",
        "process.cpu_time",
        "process.thread.count",
        "process.context_switches",
        "process.cpu.utilization",
    ]

    if self.implementation == "pypy":
        self.assertEqual(len(metric_names), 5)
    else:
        self.assertEqual(len(metric_names), 6)
    observer_names.append(
        "process.gc_count",
    )
emdneto commented 1 month ago

@arunk1988 Could you please open a PR? It's difficult to answer without seeing the changes

arunk1988 commented 1 month ago

@emdneto can you please review the draft and let me know your inputs

emdneto commented 1 month ago

@arunk1988 I understand you're still working on the PR. If it's ready, please mark as "Ready for Review"

arunk1988 commented 1 month ago

@emdneto Please review, i have already added your recommendation to this