Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :rocket: Features

* feat(sdk-trace): implement span start/end metrics [#1851](https://github.com/open-telemetry/opentelemetry-js/pull/6213) @anuraaga

### :bug: Bug Fixes

* fix(opentelemetry-sdk-node): the custom value from env variable for service.instance.id should take priority over random uuid as backup [#6345](https://github.com/open-telemetry/opentelemetry-js/pull/6345) @maryliag
Expand Down
11 changes: 8 additions & 3 deletions e2e-tests/verify.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ for (const line of lines) {
verifiedSpan = true;
}
if (parsed.resourceMetrics) {
console.log('found metric');
verifyMetric(parsed.resourceMetrics[0].scopeMetrics[0].metrics[0]);
verifiedMetric = true;
const scopeMetrics = parsed.resourceMetrics[0].scopeMetrics.find(
sm => sm.scope.name === 'example-meter'
);
if (scopeMetrics) {
console.log('found metric');
verifyMetric(scopeMetrics.metrics[0]);
verifiedMetric = true;
}
}
if (parsed.resourceLogs) {
console.log('found log');
Expand Down
15 changes: 15 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,21 @@ Additionally, you can specify other applicable environment variables that apply
- [OTLP exporter environment configuration](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options)
- [Zipkin exporter environment configuration](https://github.com/open-telemetry/opentelemetry-specification/blob/6ce62202e5407518e19c56c445c13682ef51a51d/specification/sdk-environment-variables.md#zipkin-exporter)

## Enable OpenTelemetry SDK internal metrics from environment

OpenTelemetry defines [metrics for monitoring SDK components](https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/).
Until this spec is stabilized, the following environment variable must be used
to enable these metrics:

```bash
OTEL_NODE_EXPERIMENTAL_SDK_METRICS=true
```

Currently a subset of the specified metrics are implemented. See the following
linkes for details:

- Span metrics: [TracerMetrics.ts](../../../packages/opentelemetry-sdk-trace-base/src/TracerMetrics.ts)

## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
Expand Down
50 changes: 28 additions & 22 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,43 @@ export class NodeSDK {
})
);

if (
this._meterProviderConfig?.readers &&
// only register if there is a reader, otherwise we waste compute/memory.
this._meterProviderConfig.readers.length > 0
) {
const meterProvider = new MeterProvider({
resource: this._resource,
views: this._meterProviderConfig?.views ?? [],
readers: this._meterProviderConfig.readers,
});

this._meterProvider = meterProvider;
metrics.setGlobalMeterProvider(meterProvider);

// TODO: This is a workaround to fix https://github.com/open-telemetry/opentelemetry-js/issues/3609
// If the MeterProvider is not yet registered when instrumentations are registered, all metrics are dropped.
// This code is obsolete once https://github.com/open-telemetry/opentelemetry-js/issues/3622 is implemented.
for (const instrumentation of this._instrumentations) {
instrumentation.setMeterProvider(metrics.getMeterProvider());
}
}

const spanProcessors = this._tracerProviderConfig
? this._tracerProviderConfig.spanProcessors
: getSpanProcessorsFromEnv();

// Only register if there is a span processor
if (spanProcessors.length > 0) {
// While SDK metrics are unstable, we require an opt-in.
// https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/
const sdkMetricsEnabled = getBooleanFromEnv(
'OTEL_NODE_EXPERIMENTAL_SDK_METRICS'
);
this._tracerProvider = new NodeTracerProvider({
...this._configuration,
resource: this._resource,
meterProvider: sdkMetricsEnabled ? this._meterProvider : undefined,
spanProcessors,
});
trace.setGlobalTracerProvider(this._tracerProvider);
Expand All @@ -351,28 +379,6 @@ export class NodeSDK {

logs.setGlobalLoggerProvider(loggerProvider);
}

if (
this._meterProviderConfig?.readers &&
// only register if there is a reader, otherwise we waste compute/memory.
this._meterProviderConfig.readers.length > 0
) {
const meterProvider = new MeterProvider({
resource: this._resource,
views: this._meterProviderConfig?.views ?? [],
readers: this._meterProviderConfig.readers,
});

this._meterProvider = meterProvider;
metrics.setGlobalMeterProvider(meterProvider);

// TODO: This is a workaround to fix https://github.com/open-telemetry/opentelemetry-js/issues/3609
// If the MeterProvider is not yet registered when instrumentations are registered, all metrics are dropped.
// This code is obsolete once https://github.com/open-telemetry/opentelemetry-js/issues/3622 is implemented.
for (const instrumentation of this._instrumentations) {
instrumentation.setMeterProvider(metrics.getMeterProvider());
}
}
}

public shutdown(): Promise<void> {
Expand Down
101 changes: 77 additions & 24 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ describe('Node SDK', () => {

setGlobalTracerProviderSpy = Sinon.spy(trace, 'setGlobalTracerProvider');
setGlobalLoggerProviderSpy = Sinon.spy(logs, 'setGlobalLoggerProvider');

// need to set these to none, since the default value is 'otlp'. Tests either
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wiring up metrics to tracer caused some impact on unrelated tests. This seemed like a reasonable default, but let me know how it looks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I think this is reasonable. I think these process.env settings can bleed out to other running .test.ts files in the same package, because mocha runs all the tests in the same process. However, that was already the case here.

// provide exporters programatically or reset to an appropriate value.
process.env.OTEL_TRACES_EXPORTER = 'none';
process.env.OTEL_LOGS_EXPORTER = 'none';
process.env.OTEL_METRICS_EXPORTER = 'none';
});

afterEach(() => {
Expand All @@ -124,13 +130,10 @@ describe('Node SDK', () => {
delete process.env.OTEL_METRICS_EXPORTER;
delete process.env.OTEL_PROPAGATORS;
delete process.env.OTEL_TRACES_EXPORTER;
delete process.env.OTEL_NODE_EXPERIMENTAL_SDK_METRICS;
});

it('should not register more than the minimal SDK components', async () => {
// need to set these to none, since the default value is 'otlp'
process.env.OTEL_TRACES_EXPORTER = 'none';
process.env.OTEL_LOGS_EXPORTER = 'none';
process.env.OTEL_METRICS_EXPORTER = 'none';
const sdk = new NodeSDK({
autoDetectResources: false,
});
Expand Down Expand Up @@ -272,9 +275,6 @@ describe('Node SDK', () => {
});

it('should register a meter provider if a reader is provided', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
// which sets up an exporter and affects the context manager
process.env.OTEL_TRACES_EXPORTER = 'none';
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
Expand Down Expand Up @@ -303,9 +303,6 @@ describe('Node SDK', () => {
});

it('should register a meter provider if multiple readers are provided', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
// which sets up an exporter and affects the context manager
process.env.OTEL_TRACES_EXPORTER = 'none';
const consoleExporter = new ConsoleMetricExporter();
const inMemoryExporter = new InMemoryMetricExporter(
AggregationTemporality.CUMULATIVE
Expand Down Expand Up @@ -347,9 +344,6 @@ describe('Node SDK', () => {
});

it('should show deprecation warning when using metricReader option', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
// which sets up an exporter and affects the context manager
process.env.OTEL_TRACES_EXPORTER = 'none';
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
Expand Down Expand Up @@ -378,9 +372,6 @@ describe('Node SDK', () => {
});

it('should not show deprecation warning when using metricReaders option', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
// which sets up an exporter and affects the context manager
process.env.OTEL_TRACES_EXPORTER = 'none';
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
Expand Down Expand Up @@ -409,8 +400,6 @@ describe('Node SDK', () => {
});

it('should not register meter provider when metricReaders is empty array', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
process.env.OTEL_TRACES_EXPORTER = 'none';
const sdk = new NodeSDK({
metricReaders: [],
autoDetectResources: false,
Expand All @@ -431,8 +420,68 @@ describe('Node SDK', () => {
await sdk.shutdown();
});

it('should register a meter provider to the tracer provider if both initialized and metrics enabled', async () => {
process.env.OTEL_NODE_EXPERIMENTAL_SDK_METRICS = 'true';
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100,
});

const sdk = new NodeSDK({
metricReader: metricReader,
traceExporter: new ConsoleSpanExporter(),
autoDetectResources: false,
});

sdk.start();

assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
const tracerProvider = setGlobalTracerProviderSpy.lastCall.args[0];
assert.ok(tracerProvider instanceof NodeTracerProvider);
assert.ok(
(tracerProvider as any)._config.meterProvider instanceof MeterProvider
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't great but couldn't think of anything better. Let me know if you have any ideas

);

assert.ok(metrics.getMeterProvider() instanceof MeterProvider);

await sdk.shutdown();
});

it('should not register a meter provider to the tracer provider if both initialized but metrics disabled', async () => {
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100,
});

const sdk = new NodeSDK({
metricReader: metricReader,
traceExporter: new ConsoleSpanExporter(),
autoDetectResources: false,
});

sdk.start();

assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
const tracerProvider = setGlobalTracerProviderSpy.lastCall.args[0];
assert.ok(tracerProvider instanceof NodeTracerProvider);
assert.equal((tracerProvider as any)._config.meterProvider, undefined);

assert.ok(metrics.getMeterProvider() instanceof MeterProvider);

await sdk.shutdown();
});

it('should register a logger provider if a log record processor is provided', async () => {
process.env.OTEL_TRACES_EXPORTER = 'none';
const logRecordExporter = new InMemoryLogRecordExporter();
const logRecordProcessor = new SimpleLogRecordProcessor(
logRecordExporter
Expand Down Expand Up @@ -604,9 +653,6 @@ describe('Node SDK', () => {
}

it('should register meter views when provided', async () => {
// need to set OTEL_TRACES_EXPORTER to none since default value is otlp
// which sets up an exporter and affects the context manager
process.env.OTEL_TRACES_EXPORTER = 'none';
const exporter = new InMemoryMetricExporter(
AggregationTemporality.CUMULATIVE
);
Expand Down Expand Up @@ -1163,6 +1209,9 @@ describe('Node SDK', () => {

beforeEach(() => {
stubLogger = Sinon.stub(diag, 'info');
delete process.env.OTEL_LOGS_EXPORTER;
delete process.env.OTEL_METRICS_EXPORTER;
delete process.env.OTEL_TRACES_EXPORTER;
});

afterEach(() => {
Expand Down Expand Up @@ -1303,8 +1352,6 @@ describe('Node SDK', () => {

it('should apply OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT and OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT', async () => {
// arrange
process.env.OTEL_TRACES_EXPORTER = 'none';
process.env.OTEL_METRICS_EXPORTER = 'none';
process.env.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT = '2';
process.env.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT = '10';

Expand Down Expand Up @@ -1355,6 +1402,9 @@ describe('Node SDK', () => {
beforeEach(() => {
infoStub = Sinon.stub(diag, 'info');
warnStub = Sinon.stub(diag, 'warn');
delete process.env.OTEL_LOGS_EXPORTER;
delete process.env.OTEL_METRICS_EXPORTER;
delete process.env.OTEL_TRACES_EXPORTER;
});

afterEach(() => {
Expand Down Expand Up @@ -1684,6 +1734,9 @@ describe('Node SDK', () => {

beforeEach(() => {
stubLoggerError = Sinon.stub(diag, 'warn');
delete process.env.OTEL_LOGS_EXPORTER;
delete process.env.OTEL_METRICS_EXPORTER;
delete process.env.OTEL_TRACES_EXPORTER;
});

afterEach(() => {
Expand Down
51 changes: 51 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/opentelemetry-sdk-trace-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
},
"devDependencies": {
"@opentelemetry/api": ">=1.3.0 <1.10.0",
"@opentelemetry/sdk-metrics": "2.2.0",
"@types/benchmark": "2.1.5",
"@types/mocha": "10.0.10",
"@types/node": "18.19.130",
Expand Down
Loading