should instrumentation-perf-hooks move to packages/node-runtime-metrics or similar?

[trentm]

(I'm opening a separate issue so my Q on a merged PR doesn't get lost.
Some original discussion is here: #1902 (comment))

A new instrumentation was recently merged that uses perf_hooks.performance.eventLoopUtilization to provide a nodejs.event_loop.utilization metric.
I wonder if this could not be an instrumentation, but instead a package like "packages/opentelemetry-host-metrics".

Perhaps a name like @opentelemetry/node-runtime-metrics (see https://opentelemetry.io/docs/specs/semconv/runtime/#metrics)?

/cc @d4nyll, @pichlermarc

If we think this would make more sense, it would be nice to do this before a first release of @opentelemetry/instrumentation-perf-hooks.

[trentm]

Perhaps a name like @opentelemetry/node-runtime-metrics

Using this name somewhat puts a stake in the ground for this package possibly growing support for reporting things like Node.js runtime GC, heap, etc. metrics.

[pichlermarc]

(I'm opening a separate issue so my Q on a merged PR doesn't get lost. Some original discussion is here: #1902 (comment))

Thanks for raising the issue. 👍

A new instrumentation was recently merged that uses perf_hooks.performance.eventLoopUtilization to provide a nodejs.event_loop.utilization metric. I wonder if this could not be an instrumentation, but instead a package like "packages/opentelemetry-host-metrics".

Hmm, you're right, I think we have some inconsistency here. In my opinion having it be an instrumentation makes the most sense:

• it allows it to be enabled like an instrumentation (with registerInstrumentations) and users don't have to get used to another interface to start getting telemetry.
• an instrumentation using diagnostics channels would usually also not wrap things, but I'd still be considered an instrumentation. I see a similar case can be made for metrics that can be produced without having to wrap anything.

I think HostMetrics is an oddity and we should consider changing it to be an instrumentation for the same reasons listed above. I've actually gone back and looked at the PR that added it (#227), and it looks like the reason why it is like that today is that InstrumentationBase did not support metrics yet (it was all prototypes back then). Now that InstrumentationBase has facilities for metrics, using the base can make more sense.

Perhaps a name like @opentelemetry/node-runtime-metrics (see https://opentelemetry.io/docs/specs/semconv/runtime/#metrics)?

I think having it be a different name makes a lot of sense if we want to add more metrics to it. Maybe @opentelemetry/instrumentation-node-runtime? 🤔 Edit: or @opentelemetry/instrumentation-runtime-node? we usually have the name of the runtime last (@opentelemetry/sdk-trace-web @opentelemetry/sdk-trace-node)

If we think this would make more sense, it would be nice to do this before a first release of @opentelemetry/instrumentation-perf-hooks.

Agreed. 👍

[pichlermarc]

Created a PR which applies a the rename I suggested (#1970). Just a suggestion for now, will update it depending on further discussion here 🙂

[trentm]

In my opinion having it be an instrumentation makes the most sense:

Interesting. I'm adapting the impression I'd developed that "instrumentation" was about somehow watching (by shimming, or some mechanism like diagnostic channels) usage of some library to instrumentation being "anything that produces telemetry data". Sounds reasonable to me.

Maybe @opentelemetry/instrumentation-node-runtime? 🤔 Edit: or @opentelemetry/instrumentation-runtime-node?

FWIW, in Java-land it is called "runtime-telemetry17" (https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/runtime-telemetry/runtime-telemetry-java17/library) -- the "17" is there I think because they also have a "runtime-telemetry8" to support ancient Java versions. So something with "runtime" and "node" in the name sounds good. I kind of wonder whether "metrics" should be in the name, but I guess it doesn't need to be.

In other words: you suggested instrumentation-runtime-node sounds good to me.

[trentm]

I think HostMetrics is an oddity and we should consider changing it to be an instrumentation for the same reasons listed above.

I'm kinda curious why opentelemetry-java-instrumentation doesn't have a host-metrics-related instr: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md

[pichlermarc]

I think HostMetrics is an oddity and we should consider changing it to be an instrumentation for the same reasons listed above.

I'm kinda curious why opentelemetry-java-instrumentation doesn't have a host-metrics-related instr: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md

Hmm, I think also our naming is out-of-date, in the semantic conventions these metrics metrics are actually called System Metrics today. The oshi instrumentation seems to be how these metrics are generated in Java-land.

[pichlermarc]

Merged #1970 which took care of the rename.
Closing this issue 🙂