Skip to content

feat(sdk-node): wire up metric producers from declarative config #6712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MikeGoldsmith wants to merge 6 commits into
base: main
Choose a base branch
from
+63 −0
Open

feat(sdk-node): wire up metric producers from declarative config #6712

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2549aaa
wire up metric producers from declarative config
MikeGoldsmith May 13, 2026
1e473e4
add tests for metric producer configuration
MikeGoldsmith May 13, 2026
cce2e63
Merge remote-tracking branch 'upstream/main' into mike/set-meter-prod…
MikeGoldsmith May 15, 2026
3a8eea9
merge upstream/main, export MetricProducerConfigModel per review nit
MikeGoldsmith May 15, 2026
76f26f3
Merge branch 'main' into mike/set-meter-producer
trentm May 15, 2026
9f88a33
fix missing closing brace from merge conflict resolution
MikeGoldsmith May 18, 2026
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 experimental/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :rocket: Features

* feat(sdk-node): wire up metric producers from declarative config [#6712](https://github.com/open-telemetry/opentelemetry-js/pull/6712) @MikeGoldsmith

### :bug: Bug Fixes

### :books: Documentation
Expand Down
34 changes: 34 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import type {
LoggerProviderOptions,
LogRecordProcessor,
} from '@opentelemetry/sdk-logs';
import type { MetricProducer } from '@opentelemetry/sdk-metrics';
import {
BatchLogRecordProcessor,
ConsoleLogRecordExporter,
Expand Down Expand Up @@ -509,6 +510,33 @@ export function getOtlpMetricExporterFromEnv(): PushMetricExporter {
return new OTLPProtoMetricExporter();
}

function getMetricProducersFromConfiguration(
producers: PeriodicMetricReaderConfigModel['producers']
Comment thread
MikeGoldsmith marked this conversation as resolved.
Outdated
): MetricProducer[] | undefined {
if (!producers || producers.length === 0) {
return undefined;
}
const result: MetricProducer[] = [];
for (const producer of producers) {
if (producer.opencensus) {
try {
const {
OpenCensusMetricProducer,
// eslint-disable-next-line @typescript-eslint/no-require-imports
} = require('@opentelemetry/shim-opencensus');
Copy link
Copy Markdown
Contributor

@trentm trentm May 19, 2026

Choose a reason for hiding this comment

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

This will be one of the very few conditional imports that we have in OTel JS.

FWIW, we currently do this for:

  • require(): lazy load of @grpc/grpc-js in otlp-grpc-exporter-base to avoid loading it before instrumentation-grpc is possibly in place
  • await import(): for platform-specific code in packages/opentelemetry-resources/src/detectors/platform/node/machine-id/getMachineId.ts
  • await import(): lazy load of http or https in experimental/packages/otlp-exporter-base/src/transport/http-exporter-transport.ts and experimental/packages/otlp-exporter-base/src/configuration/otlp-node-http-configuration.ts

I think having a dynamic import here is a better option than having a dep on shim-opencensus and always importing it.

Aside: I'm kinda curious if, given https://opentelemetry.io/blog/2023/sunsetting-opencensus/, we could eventually/soon/now remove opencensus support. https://opentelemetry.io/blog/2023/sunsetting-opencensus/#fn:3 mentions supporting the OpenCensus shims for a year. It has been more than a year since the shim-opencensus package was released.

@maryliag Do you happen to know?

Whereever we have Node.js-specific usage docs for declarative config, I think we should add a note that using producers: ["opencensus"] requires the user to separately install the @opentelemetry/shim-opencensus dep.

Copy link
Copy Markdown
Contributor

@maryliag maryliag May 20, 2026

Choose a reason for hiding this comment

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

Do you happen to know?

I don't have any extra info on top of what you shared, but that is a good point! Since this is a new feature, I think make sense to don't support opencensus, so we don't have to worry about maintenance of a package that is already unmaintained

Copy link
Copy Markdown
Contributor

@maryliag maryliag May 20, 2026

Choose a reason for hiding this comment

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

Declarative config needs to follow the spec, but the spec says opencensus is stable. So to make this optional/not implemented at all in declarative config, it needs to be updated in the spec first.
created an issue for that open-telemetry/opentelemetry-specification#5109

result.push(new OpenCensusMetricProducer());
} catch {
diag.warn(
'OpenCensus metric producer configured but @opentelemetry/shim-opencensus is not installed.'
);
}
} else {
diag.warn('Unsupported metric producer configured.');
}
}
return result.length > 0 ? result : undefined;
}

export function getPeriodicMetricReaderFromConfiguration(
periodic: PeriodicMetricReaderConfigModel
): IMetricReader | undefined {
Expand Down Expand Up @@ -543,17 +571,23 @@ export function getPeriodicMetricReaderFromConfiguration(
});
}

const metricProducers = getMetricProducersFromConfiguration(
periodic.producers
);

if (exporter) {
// TODO(6425): add cardinality_limits
return new PeriodicExportingMetricReader({
exportIntervalMillis: periodic.interval ?? 60_000,
exportTimeoutMillis: periodic.timeout ?? 30_000,
exporter,
metricProducers,
});
}
if (periodic.exporter.console) {
return new PeriodicExportingMetricReader({
exporter: new ConsoleMetricExporter(),
metricProducers,
});
}
}
Expand Down
30 changes: 29 additions & 1 deletion experimental/packages/opentelemetry-sdk-node/test/start.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ import {
ATTR_SERVICE_INSTANCE_ID,
} from '../src/semconv';
import { ATTR_OS_TYPE } from '@opentelemetry/resources/src/semconv';
import { getLogRecordExporter, setupContextManager } from '../src/utils';
import {
getLogRecordExporter,
getPeriodicMetricReaderFromConfiguration,
setupContextManager,
} from '../src/utils';
import { NOOP_SDK } from '../src/start';
import {
ConsoleMetricExporter,
Expand Down Expand Up @@ -962,6 +966,30 @@ describe('startNodeSDK', function () {
assert.equal(getLogRecordExporter(exporter), undefined);
});

it('should create metric reader with opencensus producer when shim is available', async () => {
const reader = getPeriodicMetricReaderFromConfiguration({
exporter: { console: {} },
producers: [{ opencensus: {} }],
});
assert.ok(reader !== undefined);
await (reader as PeriodicExportingMetricReader).shutdown();
});
Copy link
Copy Markdown
Contributor

@trentm trentm May 19, 2026

Choose a reason for hiding this comment

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

This test isn't actually testing that the shim producer is working. If I make this change:

diff --git a/experimental/packages/opentelemetry-sdk-node/src/utils.ts b/experimental/packages/opentelemetry-sdk-node/src/utils.ts
index d80eb9a36..9495a2d01 100644
--- a/experimental/packages/opentelemetry-sdk-node/src/utils.ts
+++ b/experimental/packages/opentelemetry-sdk-node/src/utils.ts
@@ -529,7 +529,7 @@ function getMetricProducersFromConfiguration(
         const {
           OpenCensusMetricProducer,
           // eslint-disable-next-line @typescript-eslint/no-require-imports
-        } = require('@opentelemetry/shim-opencensus');
+        } = require('@opentelemetry/shim-opencensusXXX');
         result.push(new OpenCensusMetricProducer());
       } catch {
         diag.warn(

the test case still passes.

I think this could cheat and look at reader._metricProducers[0].constructor.name matches "OpenCensusMetricProducer".


it('should warn for unsupported metric producer', async () => {
const warnSpy = Sinon.spy(diag, 'warn');
const reader = getPeriodicMetricReaderFromConfiguration({
exporter: { console: {} },
producers: [{ 'unknown/producer': {} }],
});
assert.ok(reader !== undefined);
assert.ok(
warnSpy.args.some(args =>
String(args[0]).includes('Unsupported metric producer')
)
);
await (reader as PeriodicExportingMetricReader).shutdown();
});

it('null context manager', async () => {
setupContextManager(null);
assert.equal(
Expand Down
Loading