diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index fbe207e8cb..3f75c1b155 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) +* fix(add-views-to-node-sdk): added the ability to define meter views in `NodeSDK` [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3124) @weyert * feature(add-console-metrics-exporter): add ConsoleMetricExporter [#3120](https://github.com/open-telemetry/opentelemetry-js/pull/3120) @weyert * feature(prometheus-serialiser): export the unit block when unit is set in metric descriptor [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3041) @weyert diff --git a/experimental/packages/opentelemetry-sdk-node/README.md b/experimental/packages/opentelemetry-sdk-node/README.md index a21de3c741..5b288ba1f3 100644 --- a/experimental/packages/opentelemetry-sdk-node/README.md +++ b/experimental/packages/opentelemetry-sdk-node/README.md @@ -128,6 +128,10 @@ Configure a trace exporter. If an exporter OR span processor is not configured, Configure tracing parameters. These are the same trace parameters used to [configure a tracer](../../../packages/opentelemetry-sdk-trace-base/src/types.ts#L71). +### views + +Configure views of your instruments and accepts an array of [View](../opentelemetry-sdk-metrics-base/src/view/View.ts)-instances. The parameter can be used to configure the explicit bucket sizes of histogram metrics. + ### serviceName Configure the [service name](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/README.md#service). diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index cd920637cf..2c6005806d 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -27,7 +27,7 @@ import { Resource, ResourceDetectionConfig } from '@opentelemetry/resources'; -import { MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics'; +import { MeterProvider, MetricReader, View } from '@opentelemetry/sdk-metrics'; import { BatchSpanProcessor, SpanProcessor @@ -37,6 +37,20 @@ import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions' import { NodeSDKConfiguration } from './types'; /** This class represents everything needed to register a fully configured OpenTelemetry Node.js SDK */ + +export type MeterProviderConfig = { + /** + * Reference to the MetricReader instance by the NodeSDK + */ + reader?: MetricReader + /** + * Lists the views that should be passed when meterProvider + * + * Note: This is only getting used when NodeSDK is responsible for + * instantiated an instance of MeterProvider + */ + views?: View[] +}; export class NodeSDK { private _tracerProviderConfig?: { tracerConfig: NodeTracerConfig; @@ -44,8 +58,8 @@ export class NodeSDK { contextManager?: ContextManager; textMapPropagator?: TextMapPropagator; }; + private _meterProviderConfig?: MeterProviderConfig; private _instrumentations: InstrumentationOption[]; - private _metricReader?: MetricReader; private _resource: Resource; @@ -87,8 +101,17 @@ export class NodeSDK { ); } - if (configuration.metricReader) { - this.configureMeterProvider(configuration.metricReader); + if (configuration.metricReader || configuration.views) { + const meterProviderConfig: MeterProviderConfig = {}; + if (configuration.metricReader) { + meterProviderConfig.reader = configuration.metricReader; + } + + if (configuration.views) { + meterProviderConfig.views = configuration.views; + } + + this.configureMeterProvider(meterProviderConfig); } let instrumentations: InstrumentationOption[] = []; @@ -114,8 +137,32 @@ export class NodeSDK { } /** Set configurations needed to register a MeterProvider */ - public configureMeterProvider(reader: MetricReader): void { - this._metricReader = reader; + public configureMeterProvider(config: MeterProviderConfig): void { + // nothing is set yet, we can set config and return. + if (this._meterProviderConfig == null) { + this._meterProviderConfig = config; + return; + } + + // make sure we do not override existing views with other views. + if (this._meterProviderConfig.views != null && config.views != null) { + throw new Error('Views passed but Views have already been configured.'); + } + + // set views, but make sure we do not override existing views with null/undefined. + if (config.views != null) { + this._meterProviderConfig.views = config.views; + } + + // make sure we do not override existing reader with another reader. + if (this._meterProviderConfig.reader != null && config.reader != null) { + throw new Error('MetricReader passed but MetricReader has already been configured.'); + } + + // set reader, but make sure we do not override existing reader with null/undefined. + if (config.reader != null) { + this._meterProviderConfig.reader = config.reader; + } } /** Detect resource attributes */ @@ -123,7 +170,7 @@ export class NodeSDK { config?: ResourceDetectionConfig ): Promise { const internalConfig: ResourceDetectionConfig = { - detectors: [ envDetector, processDetector], + detectors: [envDetector, processDetector], ...config, }; @@ -146,7 +193,7 @@ export class NodeSDK { this._resource = this._serviceName === undefined ? this._resource : this._resource.merge(new Resource( - {[SemanticResourceAttributes.SERVICE_NAME]: this._serviceName} + { [SemanticResourceAttributes.SERVICE_NAME]: this._serviceName } )); if (this._tracerProviderConfig) { @@ -164,12 +211,15 @@ export class NodeSDK { }); } - if (this._metricReader) { + if (this._meterProviderConfig) { const meterProvider = new MeterProvider({ resource: this._resource, + views: this._meterProviderConfig?.views ?? [], }); - meterProvider.addMetricReader(this._metricReader); + if (this._meterProviderConfig.reader) { + meterProvider.addMetricReader(this._meterProviderConfig.reader); + } this._meterProvider = meterProvider; diff --git a/experimental/packages/opentelemetry-sdk-node/src/types.ts b/experimental/packages/opentelemetry-sdk-node/src/types.ts index c6c2fbb269..425b375bac 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/types.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/types.ts @@ -18,7 +18,7 @@ import type { ContextManager, SpanAttributes } from '@opentelemetry/api'; import { TextMapPropagator } from '@opentelemetry/api'; import { InstrumentationOption } from '@opentelemetry/instrumentation'; import { Resource } from '@opentelemetry/resources'; -import { MetricReader } from '@opentelemetry/sdk-metrics'; +import { MetricReader, View } from '@opentelemetry/sdk-metrics'; import { Sampler, SpanExporter, @@ -32,6 +32,7 @@ export interface NodeSDKConfiguration { defaultAttributes: SpanAttributes; textMapPropagator: TextMapPropagator; metricReader: MetricReader; + views: View[] instrumentations: InstrumentationOption[]; resource: Resource; sampler: Sampler; diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 817041bb1a..edad60c5ec 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -28,7 +28,7 @@ import { AsyncLocalStorageContextManager, } from '@opentelemetry/context-async-hooks'; import { CompositePropagator } from '@opentelemetry/core'; -import { ConsoleMetricExporter, MeterProvider, PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics'; +import { AggregationTemporality, ConsoleMetricExporter, InMemoryMetricExporter, InstrumentType, MeterProvider, PeriodicExportingMetricReader, View } from '@opentelemetry/sdk-metrics'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { assertServiceResource, @@ -147,6 +147,133 @@ describe('Node SDK', () => { }); }); + async function waitForNumberOfMetrics(exporter: InMemoryMetricExporter, numberOfMetrics: number): Promise { + if (numberOfMetrics <= 0) { + throw new Error('numberOfMetrics must be greater than or equal to 0'); + } + + let totalExports = 0; + while (totalExports < numberOfMetrics) { + await new Promise(resolve => setTimeout(resolve, 20)); + const exportedMetrics = exporter.getMetrics(); + totalExports = exportedMetrics.length; + } + } + + it('should register meter views when provided', async () => { + const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE); + const metricReader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100 + }); + + const sdk = new NodeSDK({ + metricReader: metricReader, + views: [ + new View({ + name: 'test-view', + instrumentName: 'test_counter', + instrumentType: InstrumentType.COUNTER, + }) + ], + autoDetectResources: false, + }); + + await sdk.start(); + + assert.strictEqual(context['_getContextManager'](), ctxManager, 'context manager should not change'); + assert.strictEqual(propagation['_getGlobalPropagator'](), propagator, 'propagator should not change'); + assert.strictEqual((trace.getTracerProvider() as ProxyTracerProvider).getDelegate(), delegate, 'tracer provider should not have changed'); + + const meterProvider = metrics.getMeterProvider() as MeterProvider; + assert.ok(meterProvider); + + const meter = meterProvider.getMeter('NodeSDKViews', '1.0.0'); + const counter = meter.createCounter('test_counter', { + description: 'a test description', + }); + counter.add(10); + + await waitForNumberOfMetrics(exporter, 1); + const exportedMetrics = exporter.getMetrics(); + const [firstExportedMetric] = exportedMetrics; + assert.ok(firstExportedMetric, 'should have one exported metric'); + const [firstScopeMetric] = firstExportedMetric.scopeMetrics; + assert.ok(firstScopeMetric, 'should have one scope metric'); + assert.ok(firstScopeMetric.scope.name === 'NodeSDKViews', 'scope should match created view'); + assert.ok(firstScopeMetric.metrics.length > 0, 'should have at least one metrics entry'); + const [firstMetricRecord] = firstScopeMetric.metrics; + assert.ok(firstMetricRecord.descriptor.name === 'test-view', 'should have renamed counter metric'); + + await sdk.shutdown(); + }); + + it('should throw error when calling configureMeterProvider when views are already configured', () => { + const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE); + const metricReader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100 + }); + + const sdk = new NodeSDK({ + metricReader: metricReader, + views: [ + new View({ + name: 'test-view', + instrumentName: 'test_counter', + instrumentType: InstrumentType.COUNTER, + }) + ], + autoDetectResources: false, + }); + + assert.throws(() => { + sdk.configureMeterProvider({ + reader: metricReader, + views: [ + new View({ + name: 'test-view', + instrumentName: 'test_counter', + instrumentType: InstrumentType.COUNTER, + }) + ] + }); + }, (error: Error) => { + return error.message.includes('Views passed but Views have already been configured'); + }); + }); + + it('should throw error when calling configureMeterProvider when metricReader is already configured', () => { + const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE); + const metricReader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100 + }); + + const sdk = new NodeSDK({ + metricReader: metricReader, + views: [ + new View({ + name: 'test-view', + instrumentName: 'test_counter', + instrumentType: InstrumentType.COUNTER, + }) + ], + autoDetectResources: false, + }); + + assert.throws(() => { + sdk.configureMeterProvider({ + reader: metricReader, + }); + }, (error: Error) => { + return error.message.includes('MetricReader passed but MetricReader has already been configured.'); + }); + }); + describe('detectResources', async () => { beforeEach(() => { process.env.OTEL_RESOURCE_ATTRIBUTES = @@ -163,12 +290,12 @@ describe('Node SDK', () => { autoDetectResources: true, }); await sdk.detectResources({ - detectors: [ processDetector, { + detectors: [processDetector, { detect() { throw new Error('Buggy detector'); } }, - envDetector ] + envDetector] }); const resource = sdk['_resource']; @@ -277,7 +404,7 @@ describe('Node SDK', () => { }); it('should configure service name via OTEL_SERVICE_NAME env var', async () => { - process.env.OTEL_SERVICE_NAME='env-set-name'; + process.env.OTEL_SERVICE_NAME = 'env-set-name'; const sdk = new NodeSDK(); await sdk.start(); @@ -290,7 +417,7 @@ describe('Node SDK', () => { }); it('should favor config set service name over OTEL_SERVICE_NAME env set service name', async () => { - process.env.OTEL_SERVICE_NAME='env-set-name'; + process.env.OTEL_SERVICE_NAME = 'env-set-name'; const sdk = new NodeSDK({ serviceName: 'config-set-name', });