diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 06717c2ebe2..5772786c8b2 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -16,6 +16,7 @@ * refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna * refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna * refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna +* feat!: align merge resource behavior with spec in all SDKs. [#5219](https://github.com/open-telemetry/opentelemetry-js/pull/5219) @david-luna ### :rocket: (Enhancement) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts index 833fa2ba7d1..cf2ca33e4a2 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts @@ -53,6 +53,7 @@ const udsAddr = 'unix:///tmp/otlp-metrics.sock'; type TestParams = { address?: string; useTLS?: boolean; + useDefaultResource?: boolean; metadata?: grpc.Metadata; }; @@ -60,7 +61,7 @@ const metadata = new grpc.Metadata(); metadata.set('k', 'v'); const testOTLPMetricExporter = (params: TestParams) => { - const { address = httpAddr, useTLS, metadata } = params; + const { address = httpAddr, useTLS, metadata, useDefaultResource } = params; return describe(`OTLPMetricExporter - node ${ useTLS ? 'with' : 'without' } TLS, ${metadata ? 'with' : 'without'} metadata, target ${address}`, () => { @@ -143,7 +144,7 @@ const testOTLPMetricExporter = (params: TestParams) => { temporalityPreference: AggregationTemporalityPreference.CUMULATIVE, }); - setUp(); + setUp(!!useDefaultResource); const counter = mockCounter(); mockObservableGauge(observableResult => { @@ -290,7 +291,7 @@ const testOTLPMetricExporter = (params: TestParams) => { ['0', '2', '0'] ); assert.ok(typeof resource !== 'undefined', "resource doesn't exist"); - ensureResourceIsCorrect(resource); + ensureResourceIsCorrect(resource, !!useDefaultResource); ensureMetadataIsCorrect(reqMetadata, metadata); @@ -304,5 +305,6 @@ const testOTLPMetricExporter = (params: TestParams) => { testOTLPMetricExporter({ useTLS: true }); testOTLPMetricExporter({ useTLS: false }); testOTLPMetricExporter({ metadata }); +testOTLPMetricExporter({ useDefaultResource: true }); // skip UDS tests on windows process.platform !== 'win32' && testOTLPMetricExporter({ address: udsAddr }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts index 352169212a7..a8394e232ac 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -66,10 +66,10 @@ export async function collect() { return (await reader.collect())!; } -export function setUp() { +export function setUp(useDefaultResource: boolean) { reader = new TestMetricReader(); meterProvider = new MeterProvider({ - resource: testResource, + resource: useDefaultResource ? undefined : testResource, views: [ { aggregation: { @@ -211,61 +211,72 @@ export function ensureExportedHistogramIsCorrect( assert.deepStrictEqual(dp.explicitBounds, explicitBounds); } -export function ensureResourceIsCorrect(resource: IResource) { - assert.deepStrictEqual(resource, { - attributes: [ - { - key: 'service.name', - value: { - stringValue: `unknown_service:${process.argv0}`, - value: 'stringValue', +export function ensureResourceIsCorrect( + resource: IResource, + checkDefault: boolean +) { + if (checkDefault) { + assert.deepStrictEqual(resource, { + attributes: [ + { + key: 'service.name', + value: { + stringValue: `unknown_service:${process.argv0}`, + value: 'stringValue', + }, }, - }, - { - key: 'telemetry.sdk.language', - value: { - stringValue: 'nodejs', - value: 'stringValue', + { + key: 'telemetry.sdk.language', + value: { + stringValue: 'nodejs', + value: 'stringValue', + }, }, - }, - { - key: 'telemetry.sdk.name', - value: { - stringValue: 'opentelemetry', - value: 'stringValue', + { + key: 'telemetry.sdk.name', + value: { + stringValue: 'opentelemetry', + value: 'stringValue', + }, }, - }, - { - key: 'telemetry.sdk.version', - value: { - stringValue: VERSION, - value: 'stringValue', + { + key: 'telemetry.sdk.version', + value: { + stringValue: VERSION, + value: 'stringValue', + }, }, - }, - { - key: 'service', - value: { - stringValue: 'ui', - value: 'stringValue', + ], + droppedAttributesCount: 0, + }); + } else { + assert.deepStrictEqual(resource, { + attributes: [ + { + key: 'service', + value: { + stringValue: 'ui', + value: 'stringValue', + }, }, - }, - { - key: 'version', - value: { - intValue: '1', - value: 'intValue', + { + key: 'version', + value: { + intValue: '1', + value: 'intValue', + }, }, - }, - { - key: 'cost', - value: { - doubleValue: 112.12, - value: 'doubleValue', + { + key: 'cost', + value: { + doubleValue: 112.12, + value: 'doubleValue', + }, }, - }, - ], - droppedAttributesCount: 0, - }); + ], + droppedAttributesCount: 0, + }); + } } export function ensureMetadataIsCorrect( diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index ba4c8b48568..4d19aaa1051 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -103,7 +103,6 @@ export class NodeSDK { private _resource: IResource; private _resourceDetectors: Array; - private _mergeResourceWithDefaults: boolean; private _autoDetectResources: boolean; @@ -139,8 +138,6 @@ export class NodeSDK { this._configuration = configuration; this._resource = configuration.resource ?? new Resource({}); - this._mergeResourceWithDefaults = - configuration.mergeResourceWithDefaults ?? true; this._autoDetectResources = configuration.autoDetectResources ?? true; if (!this._autoDetectResources) { this._resourceDetectors = []; @@ -263,7 +260,6 @@ export class NodeSDK { this._tracerProvider = new NodeTracerProvider({ ...this._configuration, resource: this._resource, - mergeResourceWithDefaults: this._mergeResourceWithDefaults, spanProcessors, }); @@ -281,7 +277,6 @@ export class NodeSDK { if (this._loggerProviderConfig) { const loggerProvider = new LoggerProvider({ resource: this._resource, - mergeResourceWithDefaults: this._mergeResourceWithDefaults, }); for (const logRecordProcessor of this._loggerProviderConfig @@ -303,7 +298,6 @@ export class NodeSDK { resource: this._resource, views: this._meterProviderConfig?.views ?? [], readers: readers, - mergeResourceWithDefaults: this._mergeResourceWithDefaults, }); this._meterProvider = meterProvider; diff --git a/experimental/packages/opentelemetry-sdk-node/src/types.ts b/experimental/packages/opentelemetry-sdk-node/src/types.ts index 4a143692149..0683a3d886f 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/types.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/types.ts @@ -40,7 +40,6 @@ export interface NodeSDKConfiguration { instrumentations: (Instrumentation | Instrumentation[])[]; resource: IResource; resourceDetectors: Array; - mergeResourceWithDefaults?: boolean; sampler: Sampler; serviceName?: string; /** @deprecated use spanProcessors instead*/ diff --git a/experimental/packages/sdk-logs/src/LoggerProvider.ts b/experimental/packages/sdk-logs/src/LoggerProvider.ts index bdb402db933..575f178178e 100644 --- a/experimental/packages/sdk-logs/src/LoggerProvider.ts +++ b/experimental/packages/sdk-logs/src/LoggerProvider.ts @@ -28,28 +28,13 @@ import { LoggerProviderSharedState } from './internal/LoggerProviderSharedState' export const DEFAULT_LOGGER_NAME = 'unknown'; -function prepareResource( - mergeWithDefaults: boolean, - providedResource: Resource | undefined -) { - const resource = providedResource ?? Resource.empty(); - - if (mergeWithDefaults) { - return Resource.default().merge(resource); - } - return resource; -} - export class LoggerProvider implements logsAPI.LoggerProvider { private _shutdownOnce: BindOnceFuture; private readonly _sharedState: LoggerProviderSharedState; constructor(config: LoggerProviderConfig = {}) { const mergedConfig = merge({}, loadDefaultConfig(), config); - const resource = prepareResource( - mergedConfig.mergeResourceWithDefaults, - config.resource - ); + const resource = config.resource ?? Resource.default(); this._sharedState = new LoggerProviderSharedState( resource, mergedConfig.forceFlushTimeoutMillis, diff --git a/experimental/packages/sdk-logs/src/config.ts b/experimental/packages/sdk-logs/src/config.ts index b61163b2fa8..91b2c3e4884 100644 --- a/experimental/packages/sdk-logs/src/config.ts +++ b/experimental/packages/sdk-logs/src/config.ts @@ -31,7 +31,6 @@ export function loadDefaultConfig() { attributeCountLimit: getEnv().OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT, }, includeTraceContext: true, - mergeResourceWithDefaults: true, }; } diff --git a/experimental/packages/sdk-logs/src/types.ts b/experimental/packages/sdk-logs/src/types.ts index de316a0e34a..27aefa540fe 100644 --- a/experimental/packages/sdk-logs/src/types.ts +++ b/experimental/packages/sdk-logs/src/types.ts @@ -28,12 +28,6 @@ export interface LoggerProviderConfig { /** Log Record Limits*/ logRecordLimits?: LogRecordLimits; - - /** - * Merge resource with {@link Resource.default()}? - * Default: {@code true} - */ - mergeResourceWithDefaults?: boolean; } export interface LogRecordLimits { diff --git a/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts b/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts index b2499fce7e9..be1a7321515 100644 --- a/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts +++ b/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts @@ -63,39 +63,15 @@ describe('LoggerProvider', () => { assert.deepStrictEqual(resource, Resource.default()); }); - it('should fallback to default resource attrs', () => { - const passedInResource = new Resource({ foo: 'bar' }); - const provider = new LoggerProvider({ resource: passedInResource }); - const { resource } = provider['_sharedState']; - assert.deepStrictEqual( - resource, - Resource.default().merge(passedInResource) - ); - }); - - it('should not merge with default resource attrs when flag is set to false', function () { + it('should not have default resource if passed', function () { const passedInResource = new Resource({ foo: 'bar' }); const provider = new LoggerProvider({ resource: passedInResource, - mergeResourceWithDefaults: false, }); const { resource } = provider['_sharedState']; assert.deepStrictEqual(resource, passedInResource); }); - it('should merge with default resource attrs when flag is set to true', function () { - const passedInResource = new Resource({ foo: 'bar' }); - const provider = new LoggerProvider({ - resource: passedInResource, - mergeResourceWithDefaults: true, - }); - const { resource } = provider['_sharedState']; - assert.deepStrictEqual( - resource, - Resource.default().merge(passedInResource) - ); - }); - it('should have default forceFlushTimeoutMillis if not pass', () => { const provider = new LoggerProvider(); const sharedState = provider['_sharedState']; diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts index b99d6a664f1..234a613b1e0 100644 --- a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts @@ -77,11 +77,7 @@ export class BasicTracerProvider implements TracerProvider { loadDefaultConfig(), reconfigureLimits(config) ); - this._resource = mergedConfig.resource ?? Resource.empty(); - - if (mergedConfig.mergeResourceWithDefaults) { - this._resource = Resource.default().merge(this._resource); - } + this._resource = mergedConfig.resource ?? Resource.default(); this._config = Object.assign({}, mergedConfig, { resource: this._resource, diff --git a/packages/opentelemetry-sdk-trace-base/src/config.ts b/packages/opentelemetry-sdk-trace-base/src/config.ts index f2b97ff8119..f97a383599f 100644 --- a/packages/opentelemetry-sdk-trace-base/src/config.ts +++ b/packages/opentelemetry-sdk-trace-base/src/config.ts @@ -54,7 +54,6 @@ export function loadDefaultConfig() { _env.OTEL_SPAN_ATTRIBUTE_PER_EVENT_COUNT_LIMIT, attributePerLinkCountLimit: _env.OTEL_SPAN_ATTRIBUTE_PER_LINK_COUNT_LIMIT, }, - mergeResourceWithDefaults: true, }; } diff --git a/packages/opentelemetry-sdk-trace-base/src/types.ts b/packages/opentelemetry-sdk-trace-base/src/types.ts index f351c0ce072..9c3bc222a8c 100644 --- a/packages/opentelemetry-sdk-trace-base/src/types.ts +++ b/packages/opentelemetry-sdk-trace-base/src/types.ts @@ -35,12 +35,6 @@ export interface TracerConfig { /** Span Limits */ spanLimits?: SpanLimits; - /** - * Merge resource with {@link Resource.default()}? - * Default: {@code true} - **/ - mergeResourceWithDefaults?: boolean; - /** Resource associated with trace telemetry */ resource?: IResource; diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts index 62395128f9f..a52e8775df7 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts @@ -932,25 +932,12 @@ describe('BasicTracerProvider', () => { assert.deepStrictEqual(tracerProvider['_resource'], Resource.default()); }); - it('should not merge with defaults when flag is set to false', function () { - const expectedResource = new Resource({ foo: 'bar' }); - const tracerProvider = new BasicTracerProvider({ - mergeResourceWithDefaults: false, - resource: expectedResource, - }); - assert.deepStrictEqual(tracerProvider['_resource'], expectedResource); - }); - - it('should merge with defaults when flag is set to true', function () { + it('should use not use the default if resource passed', function () { const providedResource = new Resource({ foo: 'bar' }); const tracerProvider = new BasicTracerProvider({ - mergeResourceWithDefaults: true, resource: providedResource, }); - assert.deepStrictEqual( - tracerProvider['_resource'], - Resource.default().merge(providedResource) - ); + assert.deepStrictEqual(tracerProvider['_resource'], providedResource); }); }); diff --git a/packages/sdk-metrics/src/MeterProvider.ts b/packages/sdk-metrics/src/MeterProvider.ts index e852519f698..0ad601d1546 100644 --- a/packages/sdk-metrics/src/MeterProvider.ts +++ b/packages/sdk-metrics/src/MeterProvider.ts @@ -36,27 +36,6 @@ export interface MeterProviderOptions { resource?: IResource; views?: ViewOptions[]; readers?: MetricReader[]; - /** - * Merge resource with {@link Resource.default()}? - * Default: {@code true} - */ - mergeResourceWithDefaults?: boolean; -} - -/** - * @param mergeWithDefaults - * @param providedResource - */ -function prepareResource( - mergeWithDefaults: boolean, - providedResource: Resource | undefined -) { - const resource = providedResource ?? Resource.empty(); - - if (mergeWithDefaults) { - return Resource.default().merge(resource); - } - return resource; } /** @@ -68,10 +47,7 @@ export class MeterProvider implements IMeterProvider { constructor(options?: MeterProviderOptions) { this._sharedState = new MeterProviderSharedState( - prepareResource( - options?.mergeResourceWithDefaults ?? true, - options?.resource - ) + options?.resource ?? Resource.default() ); if (options?.views != null && options.views.length > 0) { for (const viewOption of options.views) { diff --git a/packages/sdk-metrics/test/MeterProvider.test.ts b/packages/sdk-metrics/test/MeterProvider.test.ts index d7ab799fa96..01f2289d02f 100644 --- a/packages/sdk-metrics/test/MeterProvider.test.ts +++ b/packages/sdk-metrics/test/MeterProvider.test.ts @@ -67,14 +67,13 @@ describe('MeterProvider', () => { assert.deepStrictEqual(resourceMetrics.resource, Resource.default()); }); - it('should not merge with defaults when flag is set to false', async function () { + it('should use the resource passed in constructor', async function () { const reader = new TestMetricReader(); const expectedResource = new Resource({ foo: 'bar' }); const meterProvider = new MeterProvider({ readers: [reader], resource: expectedResource, - mergeResourceWithDefaults: false, }); // Create meter and instrument, otherwise nothing will export @@ -87,14 +86,10 @@ describe('MeterProvider', () => { assert.deepStrictEqual(resourceMetrics.resource, expectedResource); }); - it('should merge with defaults when flag is set to true', async function () { + it('should use default resource if not passed in constructor', async function () { const reader = new TestMetricReader(); - const providedResource = new Resource({ foo: 'bar' }); - const meterProvider = new MeterProvider({ readers: [reader], - resource: providedResource, - mergeResourceWithDefaults: true, }); // Create meter and instrument, otherwise nothing will export @@ -104,10 +99,7 @@ describe('MeterProvider', () => { // Perform collection. const { resourceMetrics } = await reader.collect(); - assert.deepStrictEqual( - resourceMetrics.resource, - Resource.default().merge(providedResource) - ); + assert.deepStrictEqual(resourceMetrics.resource, Resource.default()); }); });