feat: add span metrics processor #849
Conversation
| @@ -129,6 +130,7 @@ | |||
| "@opentelemetry/sdk-logs": "^0.202.0", | |||
| "@opentelemetry/sdk-metrics": "^2.0.0", | |||
| "@opentelemetry/sdk-node": "^0.202.0", | |||
| "@opentelemetry/sdk-trace-base": "^2.0.1", | |||
There was a problem hiding this comment.
note to reviewer: added to make the linter happy but this is a dependency of @opentelemetry/sdk-node which is going to be installed regardless if here or not. I worry if this may lead to some side effects
There was a problem hiding this comment.
Should be fine. However, I'd look into updating the other ^2.0.0 otel deps to ^2.0.1 as well. E.g.: sdk-metrics.
| @@ -34,6 +34,9 @@ const testFixtures = [ | |||
| t.ok(hasLog(`name: 'nodejs.eventloop.delay.min'`)); | |||
| t.ok(hasLog(`name: 'nodejs.eventloop.delay.max'`)); | |||
| t.ok(hasLog(`name: 'process.cpu.utilization'`)); | |||
| t.ok(hasLog(`name: 'process.cpu.utilization'`)); | |||
There was a problem hiding this comment.
note to reviewer: I think this is the best place to check the metrics EDOT is collecting. If disagree I can create a new test file.
There was a problem hiding this comment.
If the intent is to just implement a subset (two) of the SDK metrics mentioned in https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics please update the description to say so. Also need to update to https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics as the reference because the PR is out of date (e.g. otel.sdk.span.live.count -> otel.sdk.span.live).
| @@ -129,6 +130,7 @@ | |||
| "@opentelemetry/sdk-logs": "^0.202.0", | |||
| "@opentelemetry/sdk-metrics": "^2.0.0", | |||
| "@opentelemetry/sdk-node": "^0.202.0", | |||
| "@opentelemetry/sdk-trace-base": "^2.0.1", | |||
There was a problem hiding this comment.
Should be fine. However, I'd look into updating the other ^2.0.0 otel deps to ^2.0.1 as well. E.g.: sdk-metrics.
Updated the title and description of the PR :) |
|
ready for another round of review @trentm |
| @@ -34,6 +34,9 @@ const testFixtures = [ | |||
| t.ok(hasLog(`name: 'nodejs.eventloop.delay.min'`)); | |||
| t.ok(hasLog(`name: 'nodejs.eventloop.delay.max'`)); | |||
| t.ok(hasLog(`name: 'process.cpu.utilization'`)); | |||
| t.ok(hasLog(`name: 'process.cpu.utilization'`)); | |||
Co-authored-by: Trent Mick <[email protected]>
Co-authored-by: Trent Mick <[email protected]>
There was a problem hiding this comment.
I just saw open-telemetry/semantic-conventions#2431
We should understand that before merging this.
This PR adds the following OTEL SDK metrics:
Summary of changes:
processors.jsmodule to resolve processors set via env or via configuration optionssdk-metrics.jsmodule which exportssetupSdkMetricsmethod. The method is used to add a new Span processor which keeps the counters up to date whenever a Span starts or ends.