open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
685 stars 502 forks source link

Process metrics should not be reported (at least by default) by the instrumentation libraries #2322

Open lmolkova opened 3 months ago

lmolkova commented 3 months ago

Description

See https://github.com/open-telemetry/semantic-conventions/issues/1215 for the context

Node.js process instrumentation reports process metrics, which are not an intended for instrumentation libraries.

Individual runtimes are expected to report their own CPU and memory metrics even if process.* ones are suitable. There is no clear conclusion on whether runtimes are not allowed to report them vs they should be opt-in.

Expected behavior

JS/node.js may define runtime-specific CPU/memory metrics and should not report process ones (by default)

JamieDanielson commented 2 months ago

These metrics have to be explicitly enabled (so they are not reported by default), so we think we are compliant here.

lmolkova commented 2 months ago

I believe the compliance means that the metric is reported with appropriate process resources being set, which instrumentation cannot guarantee. Still, the attachment to process resource is not documented and I'd not take any action on this bug until semconv issue is resolved.

I created this one to create more awareness wrt process vs runtime metrics. The approach we're taking (for java and .NET) is to emit {runtime}.cpu.time metric and leave process ones for collector. I.e. JS (node) should probably define new process-like metrics and report them instead.

github-actions[bot] commented 1 day ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.