feat: add span metrics processor#849
Conversation
| "@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.
| 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.
trentm
left a comment
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).
| "@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 |
trentm
left a comment
There was a problem hiding this comment.
Sorry for taking so long to re-review.
| 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 <trent.mick@elastic.co>
Co-authored-by: Trent Mick <trent.mick@elastic.co>
trentm
left a comment
There was a problem hiding this comment.
I just saw open-telemetry/semantic-conventions#2431
We should understand that before merging this.
|
Closing since we need to refine what we want to measure |
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.