feat(instrumentation-runtime-node): add prom-client-metrics

[pikalovArtemN]

Which problem is this PR solving?

• Make a part of request in Runtime instrumentation #1106

Short description of the changes

• Add implementation for collecting event loop lag, garbage collector, heap size and heap space

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: pikalovArtemN / name: LowArt (d7c5108, 046072e, ed6596f, ad88f98)

[maryliag]

curious about this change, can you give more context why the rename?

[pikalovArtemN]

Because right now i use it for every metrics, not only for eventLoopUtilization. You think I needed to add parameters for every new metrics group ?

[JCMais]

I am super excited for this one, I think it was the main thing missing from OpenTelemetry's Node.js instrumentation! Thanks for opening this PR.

[dyladan]

Can you please take a look at open-telemetry/semantic-conventions#991 and give your input there? It looks like you made some different decisions than what was made there and I want to make sure we're all on the same page before merging something.

[pikalovArtemN]

Can you please take a look at open-telemetry/semantic-conventions#991 and give your input there? It looks like you made some different decisions than what was made there and I want to make sure we're all on the same page before merging something.

Thanks. Yes, i have some mismatches.

[maryliag]

@pikalovArtemN I would love to hear your opinion on the mismatches. Let me know what you think makes more sense 😄

[pikalovArtemN]

@pikalovArtemN I would love to hear your opinion on the mismatches. Let me know what you think makes more sense 😄

I have mismatches in names like eventloop => event_loop, and i need to fix units, i totaly forgot to set it properly XD. And fix attributes.

I decided to devide the metrics not by label, but names like it was in the prom-client library, but i think you do a better solution in the convension.

I think we need to colobarate, not all of metrics and attributes that i added exists in convention such as:

1. attribute nodejs.eventloop.lag.type -> stddev and mean not exists
2. nodejs.eventloop.utilization metric does't exist in specification
3. nodejs.active_handles.count -> does't exist in runtime-node, do i need to add it ?
4. nodejs.memory.size -> i have 2 splited metrics heap size and heap space, i think we need to split it by memory, heap size, and heap space in different metrics with attributes nodejs.memory.state: (total and used), nodejs.heapsize.state (total and used), nodejs.heapspace.state( total, used, available) and i add nodejs.memory.size in instumentation-runtime-node
5. nodejs.active_libuv_requests.count -> does't exist in runtime-node, do i need to add it ?

[maryliag]

attribute nodejs.eventloop.lag.type -> stddev and mean not exists

I can add that

nodejs.eventloop.utilization metric does't exist in specification

I can add that

nodejs.active_handles.count -> does't exist in runtime-node, do i need to add it ?

I think it would be good, I got that from the prom-client metrics list, so should be possible

nodejs.memory.size -> i have 2 splited metrics heap size and heap space, i think we need to split it by memory, heap size, and heap space in different metrics with attributes nodejs.memory.state: (total and used), nodejs.heapsize.state (total and used), nodejs.heapspace.state( total, used, available) and i add nodejs.memory.size in instumentation-runtime-node

I can make those changes on the convention

nodejs.active_libuv_requests.count -> -> does't exist in runtime-node, do i need to add it ?

I think it would be good, I also got that from prom-client, but it doesn't have libuv in the name, I added on the convention because of a feedback to make it more clear what it is

[maryliag]

heads up: because the semantic convention could be for all JS runtime (not just nodejs, but also others such as denojs or bunjs), it was suggested to not use nodejs on the metric name, so I replaced with jsruntime. I'm still waiting on more feedback if people think that is a good name, but that means you would also probably need to make the same update here

[pikalovArtemN]

heads up: because the semantic convention could be for all JS runtime (not just nodejs, but also others such as denojs or bunjs), it was suggested to not use nodejs on the metric name, so I replaced with jsruntime. I'm still waiting on more feedback if people think that is a good name, but that means you would also probably need to make the same update here

Ok

[Flarna]

If it is about any js runtime the node.js specific parts should be removed/renamed/...

• anything coming from v8 is not automatically applicable to js engine in firefox or safari
• browsers do not use libuv as far as I know (not sure about bun either)

[maryliag]

anything coming from v8

I don't understand what you mean here.

For things that are node specific, we can still create the metrics for it, such as the active libuv requests, I just need remove that from the semantic convention, but this PR can still keep it because can be useful.
This way we can move comments about the semantic convention to that PR, to keep things in one place

[Flarna]

There are JS engines (e.g. quickjs) which use a different heap structure/gc then google v8 (the JS engine used by node.js or deno).
The proposed gc/heap metrics match fine to node.js because it uses v8 but they might not fit well to other JS engines.

As a result I'm not sure if GC metrics should have v8 in the name to allow to distinguish them from other engines.
Alternative would be to create universal gc metrics for all types of runtimes using a gc. But I doubt that will be easy.

[pikalovArtemN]

I think right now we need to concentrate on nodejs realization only, because Node is most popular and in the next iteration adopt instrumentation to work with different engines. Right now it's so much work to do