Skip to content
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

Update OS process metrics name according to semconv #2662

Open
emdneto opened this issue Jul 3, 2024 · 7 comments · May be fixed by #2727
Open

Update OS process metrics name according to semconv #2662

emdneto opened this issue Jul 3, 2024 · 7 comments · May be fixed by #2727
Labels

Comments

@emdneto
Copy link
Member

emdneto commented Jul 3, 2024

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 #2652 (comment)

Would you like to implement a fix?

None

@emdneto emdneto changed the title Update instrumentation-system-metrics OS process metrics name according to semconv Update OS process metrics name according to semconv Jul 3, 2024
@arunk1988
Copy link

@emdneto May i take this one

@emdneto
Copy link
Member Author

emdneto commented Jul 9, 2024

@arunk1988 Sure. Please open a PR!

@xrmx xrmx added the good first issue Good for newcomers label Jul 11, 2024
@arunk1988
Copy link

@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
Copy link
Member Author

emdneto commented Jul 12, 2024

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

@arunk1988
Copy link

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

@emdneto
Copy link
Member Author

emdneto commented Jul 15, 2024

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

brianwarner pushed a commit to fidelity-contributions/open-telemetry-opentelemetry-python-contrib that referenced this issue Jul 22, 2024
@arunk1988 arunk1988 linked a pull request Jul 23, 2024 that will close this issue
11 tasks
@arunk1988
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants