From ccb234738bb6bf126de9c3faca220cae126237ed Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Thu, 12 Jun 2025 18:06:43 +0400 Subject: [PATCH 01/17] feat(opentelemetry-resources): add schema url --- .../src/common/internal-types.ts | 3 + .../otlp-transformer/src/common/internal.ts | 7 +- .../opentelemetry-resources/src/Resource.ts | 7 ++ .../src/ResourceImpl.ts | 54 +++++++-- .../test/Resource.test.ts | 110 ++++++++++++++++++ 5 files changed, 169 insertions(+), 12 deletions(-) diff --git a/experimental/packages/otlp-transformer/src/common/internal-types.ts b/experimental/packages/otlp-transformer/src/common/internal-types.ts index 93e07a50a6f..315fa46f984 100644 --- a/experimental/packages/otlp-transformer/src/common/internal-types.ts +++ b/experimental/packages/otlp-transformer/src/common/internal-types.ts @@ -21,6 +21,9 @@ export interface Resource { /** Resource droppedAttributesCount */ droppedAttributesCount: number; + + /** Resource schemaUrl */ + schemaUrl?: string; } /** Properties of an InstrumentationScope. */ diff --git a/experimental/packages/otlp-transformer/src/common/internal.ts b/experimental/packages/otlp-transformer/src/common/internal.ts index 80f61e0553d..ded6a527af0 100644 --- a/experimental/packages/otlp-transformer/src/common/internal.ts +++ b/experimental/packages/otlp-transformer/src/common/internal.ts @@ -24,10 +24,15 @@ import { InstrumentationScope } from '@opentelemetry/core'; import { Resource as ISdkResource } from '@opentelemetry/resources'; export function createResource(resource: ISdkResource): Resource { - return { + const result: Resource = { attributes: toAttributes(resource.attributes), droppedAttributesCount: 0, }; + + const schemaUrl = resource.getSchemaUrl?.(); + if (schemaUrl && schemaUrl !== '') result.schemaUrl = schemaUrl; + + return result; } export function createInstrumentationScope( diff --git a/packages/opentelemetry-resources/src/Resource.ts b/packages/opentelemetry-resources/src/Resource.ts index 41d14269af9..9acb4cfac35 100644 --- a/packages/opentelemetry-resources/src/Resource.ts +++ b/packages/opentelemetry-resources/src/Resource.ts @@ -59,4 +59,11 @@ export interface Resource { merge(other: Resource | null): Resource; getRawAttributes(): RawResourceAttribute[]; + + /** + * Get the schema URL for this resource. + * + * @returns the schema URL or undefined if not set + */ + getSchemaUrl?(): string | undefined; } diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 88dd8c178fc..138f19632bb 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -35,13 +35,15 @@ import { isPromiseLike } from './utils'; class ResourceImpl implements Resource { private _rawAttributes: RawResourceAttribute[]; private _asyncAttributesPending = false; + private _schemaUrl?: string; private _memoizedAttributes?: Attributes; static FromAttributeList( - attributes: [string, MaybePromise][] + attributes: [string, MaybePromise][], + schemaUrl?: string ): Resource { - const res = new ResourceImpl({}); + const res = new ResourceImpl({}, schemaUrl); res._rawAttributes = guardedRawAttributes(attributes); res._asyncAttributesPending = attributes.filter(([_, val]) => isPromiseLike(val)).length > 0; @@ -54,7 +56,8 @@ class ResourceImpl implements Resource { * information about the entity as numbers, strings or booleans * TODO: Consider to add check/validation on attributes. */ - resource: DetectedResource + resource: DetectedResource, + schemaUrl?: string ) { const attributes = resource.attributes ?? {}; this._rawAttributes = Object.entries(attributes).map(([k, v]) => { @@ -67,6 +70,7 @@ class ResourceImpl implements Resource { }); this._rawAttributes = guardedRawAttributes(this._rawAttributes); + this._schemaUrl = schemaUrl; } public get asyncAttributesPending(): boolean { @@ -120,28 +124,35 @@ class ResourceImpl implements Resource { return this._rawAttributes; } + public getSchemaUrl(): string | undefined { + return this._schemaUrl; + } + public merge(resource: Resource | null): Resource { if (resource == null) return this; // Order is important // Spec states incoming attributes override existing attributes - return ResourceImpl.FromAttributeList([ - ...resource.getRawAttributes(), - ...this.getRawAttributes(), - ]); + const mergedSchemaUrl = mergeSchemaUrl(this, resource); + return ResourceImpl.FromAttributeList( + [...resource.getRawAttributes(), ...this.getRawAttributes()], + mergedSchemaUrl + ); } } export function resourceFromAttributes( - attributes: DetectedResourceAttributes + attributes: DetectedResourceAttributes, + schemaUrl?: string ): Resource { - return ResourceImpl.FromAttributeList(Object.entries(attributes)); + return ResourceImpl.FromAttributeList(Object.entries(attributes), schemaUrl); } export function resourceFromDetectedResource( - detectedResource: DetectedResource + detectedResource: DetectedResource, + schemaUrl?: string ): Resource { - return new ResourceImpl(detectedResource); + return new ResourceImpl(detectedResource, schemaUrl); } export function emptyResource(): Resource { @@ -177,3 +188,24 @@ function guardedRawAttributes( return [k, v]; }); } + +function mergeSchemaUrl( + base: Resource, + other: Resource | null +): string | undefined { + if (other?.getSchemaUrl) { + const otherSchemaUrl = other.getSchemaUrl(); + if (otherSchemaUrl !== undefined && otherSchemaUrl !== '') { + return otherSchemaUrl; + } + } + + if (base?.getSchemaUrl) { + const baseSchemaUrl = base.getSchemaUrl(); + if (baseSchemaUrl !== undefined && baseSchemaUrl !== '') { + return baseSchemaUrl; + } + } + + return undefined; +} diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index a3d4fd85367..a3ec5d64d5b 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -287,6 +287,116 @@ describe('Resource', () => { }); }); + describe('schema URL support', () => { + it('should create resource with schema URL', () => { + const schemaUrl = 'https://example.com/schema'; + const resource = resourceFromAttributes({ attr: 'value' }, schemaUrl); + + assert.strictEqual(resource.getSchemaUrl?.(), schemaUrl); + }); + + it('should create resource without schema URL', () => { + const resource = resourceFromAttributes({ attr: 'value' }); + + assert.strictEqual(resource.getSchemaUrl?.(), undefined); + }); + + it('should merge resources with schema URL priority given to other resource', () => { + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + 'https://schema1.com' + ); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + 'https://schema2.com' + ); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual( + mergedResource.getSchemaUrl?.(), + 'https://schema2.com' + ); + }); + + it('should retain schema URL from base resource when other has no schema URL', () => { + const schemaUrl = 'https://example.com/schema'; + const resource1 = resourceFromAttributes({ attr1: 'value1' }, schemaUrl); + const resource2 = resourceFromAttributes({ attr2: 'value2' }); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual(mergedResource.getSchemaUrl?.(), schemaUrl); + }); + + it('should retain schema URL from the resource that has it when merging', () => { + const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + 'https://example.com/schema' + ); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual( + mergedResource.getSchemaUrl?.(), + 'https://example.com/schema' + ); + }); + + it('should have empty schema URL when merging resources with no schema URL', () => { + const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); + const resource2 = resourceFromAttributes({ attr2: 'value2' }, ''); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); + }); + + it('should handle merging with empty string schema URLs', () => { + const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + 'https://valid.schema' + ); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual( + mergedResource.getSchemaUrl?.(), + 'https://valid.schema' + ); + }); + + it('should maintain backward compatibility - getSchemaUrl is optional', () => { + const resource = emptyResource(); + + // This should not throw even if getSchemaUrl is not implemented + const schemaUrl = resource.getSchemaUrl?.(); + assert.strictEqual(schemaUrl, undefined); + }); + + it('should work with async attributes and schema URLs', async () => { + const resource = resourceFromAttributes( + { + sync: 'fromsync', + async: new Promise(resolve => + setTimeout(() => resolve('fromasync'), 1) + ), + }, + 'https://async.schema' + ); + + await resource.waitForAsyncAttributes?.(); + + assert.deepStrictEqual(resource.attributes, { + sync: 'fromsync', + async: 'fromasync', + }); + assert.strictEqual(resource.getSchemaUrl?.(), 'https://async.schema'); + }); + }); + describeNode('.default()', () => { it('should return a default resource', () => { const resource = defaultResource(); From 72c7d8d498d1edea0f4e18a13d85c1ca8b0f4833 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Thu, 12 Jun 2025 18:19:37 +0400 Subject: [PATCH 02/17] chore: add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5db2c1ad02f..5f8d952bc2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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(opentelemetry-resources): add schema url [#5070](https://github.com/open-telemetry/opentelemetry-js/pull/5753) @c-ehrlich + ### :bug: Bug Fixes ### :books: Documentation From e69c81df8ac7799dc3bb86fd5b202b481ba8260a Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Fri, 13 Jun 2025 21:11:58 +0400 Subject: [PATCH 03/17] feat: implement options param for Resource --- .../src/ResourceImpl.ts | 23 ++++++---- packages/opentelemetry-resources/src/types.ts | 7 +++ .../test/Resource.test.ts | 45 ++++++++++++------- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 138f19632bb..8e1b4a6dd1a 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -29,6 +29,7 @@ import { DetectedResourceAttributes, MaybePromise, RawResourceAttribute, + ResourceOptions, } from './types'; import { isPromiseLike } from './utils'; @@ -41,9 +42,9 @@ class ResourceImpl implements Resource { static FromAttributeList( attributes: [string, MaybePromise][], - schemaUrl?: string + options?: ResourceOptions ): Resource { - const res = new ResourceImpl({}, schemaUrl); + const res = new ResourceImpl({}, options); res._rawAttributes = guardedRawAttributes(attributes); res._asyncAttributesPending = attributes.filter(([_, val]) => isPromiseLike(val)).length > 0; @@ -57,7 +58,7 @@ class ResourceImpl implements Resource { * TODO: Consider to add check/validation on attributes. */ resource: DetectedResource, - schemaUrl?: string + options?: ResourceOptions ) { const attributes = resource.attributes ?? {}; this._rawAttributes = Object.entries(attributes).map(([k, v]) => { @@ -70,7 +71,7 @@ class ResourceImpl implements Resource { }); this._rawAttributes = guardedRawAttributes(this._rawAttributes); - this._schemaUrl = schemaUrl; + this._schemaUrl = options?.schemaUrl; } public get asyncAttributesPending(): boolean { @@ -134,25 +135,29 @@ class ResourceImpl implements Resource { // Order is important // Spec states incoming attributes override existing attributes const mergedSchemaUrl = mergeSchemaUrl(this, resource); + const mergedOptions: ResourceOptions | undefined = mergedSchemaUrl + ? { schemaUrl: mergedSchemaUrl } + : undefined; + return ResourceImpl.FromAttributeList( [...resource.getRawAttributes(), ...this.getRawAttributes()], - mergedSchemaUrl + mergedOptions ); } } export function resourceFromAttributes( attributes: DetectedResourceAttributes, - schemaUrl?: string + options?: ResourceOptions ): Resource { - return ResourceImpl.FromAttributeList(Object.entries(attributes), schemaUrl); + return ResourceImpl.FromAttributeList(Object.entries(attributes), options); } export function resourceFromDetectedResource( detectedResource: DetectedResource, - schemaUrl?: string + options?: ResourceOptions ): Resource { - return new ResourceImpl(detectedResource, schemaUrl); + return new ResourceImpl(detectedResource, options); } export function emptyResource(): Resource { diff --git a/packages/opentelemetry-resources/src/types.ts b/packages/opentelemetry-resources/src/types.ts index 427ef147b4e..988b8c93f87 100644 --- a/packages/opentelemetry-resources/src/types.ts +++ b/packages/opentelemetry-resources/src/types.ts @@ -59,3 +59,10 @@ export type RawResourceAttribute = [ string, MaybePromise, ]; + +/** + * Options for creating a {@link Resource}. + */ +export type ResourceOptions = { + schemaUrl?: string; +}; diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index a3ec5d64d5b..eb13aac6eeb 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -290,7 +290,7 @@ describe('Resource', () => { describe('schema URL support', () => { it('should create resource with schema URL', () => { const schemaUrl = 'https://example.com/schema'; - const resource = resourceFromAttributes({ attr: 'value' }, schemaUrl); + const resource = resourceFromAttributes({ attr: 'value' }, { schemaUrl }); assert.strictEqual(resource.getSchemaUrl?.(), schemaUrl); }); @@ -301,27 +301,28 @@ describe('Resource', () => { assert.strictEqual(resource.getSchemaUrl?.(), undefined); }); - it('should merge resources with schema URL priority given to other resource', () => { + it('should handle schema URL conflicts by returning undefined', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, - 'https://schema1.com' + { schemaUrl: 'https://schema1.com' } ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - 'https://schema2.com' + { schemaUrl: 'https://schema2.com' } ); const mergedResource = resource1.merge(resource2); - assert.strictEqual( - mergedResource.getSchemaUrl?.(), - 'https://schema2.com' - ); + // When both resources have different non-empty schema URLs, return undefined + assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); }); it('should retain schema URL from base resource when other has no schema URL', () => { const schemaUrl = 'https://example.com/schema'; - const resource1 = resourceFromAttributes({ attr1: 'value1' }, schemaUrl); + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl } + ); const resource2 = resourceFromAttributes({ attr2: 'value2' }); const mergedResource = resource1.merge(resource2); @@ -330,10 +331,13 @@ describe('Resource', () => { }); it('should retain schema URL from the resource that has it when merging', () => { - const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl: '' } + ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - 'https://example.com/schema' + { schemaUrl: 'https://example.com/schema' } ); const mergedResource = resource1.merge(resource2); @@ -345,8 +349,14 @@ describe('Resource', () => { }); it('should have empty schema URL when merging resources with no schema URL', () => { - const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); - const resource2 = resourceFromAttributes({ attr2: 'value2' }, ''); + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl: '' } + ); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + { schemaUrl: '' } + ); const mergedResource = resource1.merge(resource2); @@ -354,10 +364,13 @@ describe('Resource', () => { }); it('should handle merging with empty string schema URLs', () => { - const resource1 = resourceFromAttributes({ attr1: 'value1' }, ''); + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl: '' } + ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - 'https://valid.schema' + { schemaUrl: 'https://valid.schema' } ); const mergedResource = resource1.merge(resource2); @@ -384,7 +397,7 @@ describe('Resource', () => { setTimeout(() => resolve('fromasync'), 1) ), }, - 'https://async.schema' + { schemaUrl: 'https://async.schema' } ); await resource.waitForAsyncAttributes?.(); From eae14ba0b724ce8cb4a3083fceca9927802158f9 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Fri, 13 Jun 2025 21:12:32 +0400 Subject: [PATCH 04/17] fix: correct behavior for merging schema urls --- .../src/ResourceImpl.ts | 35 ++++++++++------ .../test/Resource.test.ts | 41 +++++++++++++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 8e1b4a6dd1a..035dd637f47 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -195,22 +195,33 @@ function guardedRawAttributes( } function mergeSchemaUrl( - base: Resource, - other: Resource | null + old: Resource, + updating: Resource | null ): string | undefined { - if (other?.getSchemaUrl) { - const otherSchemaUrl = other.getSchemaUrl(); - if (otherSchemaUrl !== undefined && otherSchemaUrl !== '') { - return otherSchemaUrl; - } + const oldSchemaUrl = old?.getSchemaUrl?.(); + const updatingSchemaUrl = updating?.getSchemaUrl?.(); + + const isOldEmpty = oldSchemaUrl === undefined || oldSchemaUrl === ''; + const isUpdatingEmpty = + updatingSchemaUrl === undefined || updatingSchemaUrl === ''; + + if (isOldEmpty) { + return updatingSchemaUrl; } - if (base?.getSchemaUrl) { - const baseSchemaUrl = base.getSchemaUrl(); - if (baseSchemaUrl !== undefined && baseSchemaUrl !== '') { - return baseSchemaUrl; - } + if (isUpdatingEmpty) { + return oldSchemaUrl; } + if (oldSchemaUrl === updatingSchemaUrl) { + return oldSchemaUrl; + } + + diag.warn( + 'Schema URL merge conflict: old resource has "%s", updating resource has "%s". Resulting resource will have undefined Schema URL.', + oldSchemaUrl, + updatingSchemaUrl + ); + return undefined; } diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index eb13aac6eeb..708a13f81c9 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -408,6 +408,47 @@ describe('Resource', () => { }); assert.strictEqual(resource.getSchemaUrl?.(), 'https://async.schema'); }); + + it('should merge schema URLs according to OpenTelemetry spec - same URLs', () => { + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl: 'https://same.schema' } + ); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + { schemaUrl: 'https://same.schema' } + ); + + const mergedResource = resource1.merge(resource2); + + assert.strictEqual( + mergedResource.getSchemaUrl?.(), + 'https://same.schema' + ); + }); + + it('should merge schema URLs according to OpenTelemetry spec - conflict case', () => { + const warnStub = sinon.spy(diag, 'warn'); + + const resource1 = resourceFromAttributes( + { attr1: 'value1' }, + { schemaUrl: 'https://schema1.com' } + ); + const resource2 = resourceFromAttributes( + { attr2: 'value2' }, + { schemaUrl: 'https://schema2.com' } + ); + + const mergedResource = resource1.merge(resource2); + + // Implementation-specific: we return undefined to indicate error state + // This aligns with Go, Java, and PHP SDKs which return null/empty for conflicts + assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); + // Should log a warning about the conflict + assert.ok(warnStub.calledWithMatch('Schema URL merge conflict')); + + warnStub.restore(); + }); }); describeNode('.default()', () => { From 39bc4fe659035993004590c39bcedc071886feeb Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 11:10:50 +0700 Subject: [PATCH 05/17] feat: add schemaUrl handling to `internal` --- .../otlp-transformer/src/logs/internal.ts | 25 ++++++++------- .../otlp-transformer/src/metrics/internal.ts | 5 +-- .../otlp-transformer/src/trace/internal.ts | 6 ++-- .../otlp-transformer/test/logs.test.ts | 23 +++++++++++++ .../otlp-transformer/test/metrics.test.ts | 32 +++++++++++++++++++ .../otlp-transformer/test/trace.test.ts | 23 +++++++++++++ 6 files changed, 98 insertions(+), 16 deletions(-) diff --git a/experimental/packages/otlp-transformer/src/logs/internal.ts b/experimental/packages/otlp-transformer/src/logs/internal.ts index f3dd227aebc..510206d37c9 100644 --- a/experimental/packages/otlp-transformer/src/logs/internal.ts +++ b/experimental/packages/otlp-transformer/src/logs/internal.ts @@ -79,17 +79,20 @@ function logRecordsToResourceLogs( encoder: Encoder ): IResourceLogs[] { const resourceMap = createResourceMap(logRecords); - return Array.from(resourceMap, ([resource, ismMap]) => ({ - resource: createResource(resource), - scopeLogs: Array.from(ismMap, ([, scopeLogs]) => { - return { - scope: createInstrumentationScope(scopeLogs[0].instrumentationScope), - logRecords: scopeLogs.map(log => toLogRecord(log, encoder)), - schemaUrl: scopeLogs[0].instrumentationScope.schemaUrl, - }; - }), - schemaUrl: undefined, - })); + return Array.from(resourceMap, ([resource, ismMap]) => { + const processedResource = createResource(resource); + return { + resource: processedResource, + scopeLogs: Array.from(ismMap, ([, scopeLogs]) => { + return { + scope: createInstrumentationScope(scopeLogs[0].instrumentationScope), + logRecords: scopeLogs.map(log => toLogRecord(log, encoder)), + schemaUrl: scopeLogs[0].instrumentationScope.schemaUrl, + }; + }), + schemaUrl: processedResource.schemaUrl, + }; + }); } function toLogRecord(log: ReadableLogRecord, encoder: Encoder): ILogRecord { diff --git a/experimental/packages/otlp-transformer/src/metrics/internal.ts b/experimental/packages/otlp-transformer/src/metrics/internal.ts index edcc0cddce8..50feb9c60fa 100644 --- a/experimental/packages/otlp-transformer/src/metrics/internal.ts +++ b/experimental/packages/otlp-transformer/src/metrics/internal.ts @@ -47,9 +47,10 @@ export function toResourceMetrics( options?: OtlpEncodingOptions ): IResourceMetrics { const encoder = getOtlpEncoder(options); + const processedResource = createResource(resourceMetrics.resource); return { - resource: createResource(resourceMetrics.resource), - schemaUrl: undefined, + resource: processedResource, + schemaUrl: processedResource.schemaUrl, scopeMetrics: toScopeMetrics(resourceMetrics.scopeMetrics, encoder), }; } diff --git a/experimental/packages/otlp-transformer/src/trace/internal.ts b/experimental/packages/otlp-transformer/src/trace/internal.ts index bbc4d4a53b8..d9a517401d6 100644 --- a/experimental/packages/otlp-transformer/src/trace/internal.ts +++ b/experimental/packages/otlp-transformer/src/trace/internal.ts @@ -170,11 +170,11 @@ function spanRecordsToResourceSpans( } ilmEntry = ilmIterator.next(); } - // TODO SDK types don't provide resource schema URL at this time + const processedResource = createResource(resource); const transformedSpans: IResourceSpans = { - resource: createResource(resource), + resource: processedResource, scopeSpans: scopeResourceSpans, - schemaUrl: undefined, + schemaUrl: processedResource.schemaUrl, }; out.push(transformedSpans); diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index ff4412059e8..c1ee79fd448 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -292,6 +292,29 @@ describe('Logs', () => { assert.ok(exportRequest); assert.strictEqual(exportRequest.resourceLogs?.length, 2); }); + + it('supports schema URL on resource', () => { + const resourceWithSchema = resourceFromAttributes( + {}, + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } + ); + + const logWithSchema: ReadableLogRecord = { + ...log_1_1_1, + resource: resourceWithSchema, + }; + + const exportRequest = createExportLogsServiceRequest([logWithSchema], { + useHex: true, + }); + + assert.ok(exportRequest); + assert.strictEqual(exportRequest.resourceLogs?.length, 1); + assert.strictEqual( + exportRequest.resourceLogs?.[0].schemaUrl, + 'https://opentelemetry.test/schemas/1.2.3' + ); + }); }); describe('ProtobufLogsSerializer', function () { diff --git a/experimental/packages/otlp-transformer/test/metrics.test.ts b/experimental/packages/otlp-transformer/test/metrics.test.ts index 22d01ed5bdb..725d1e3ad2e 100644 --- a/experimental/packages/otlp-transformer/test/metrics.test.ts +++ b/experimental/packages/otlp-transformer/test/metrics.test.ts @@ -773,6 +773,38 @@ describe('Metrics', () => { }); }); }); + + it('serializes a metric record with resource schema URL', () => { + const resourceWithSchema = resourceFromAttributes( + {}, + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } + ); + + const resourceMetricsWithSchema: ResourceMetrics = { + resource: resourceWithSchema, + scopeMetrics: [ + { + scope: { + name: 'mylib', + version: '0.1.0', + schemaUrl: expectedSchemaUrl, + }, + metrics: [createCounterData(10, AggregationTemporality.DELTA)], + }, + ], + }; + + const exportRequest = createExportMetricsServiceRequest([ + resourceMetricsWithSchema, + ]); + + assert.ok(exportRequest); + assert.strictEqual(exportRequest.resourceMetrics?.length, 1); + assert.strictEqual( + exportRequest.resourceMetrics?.[0].schemaUrl, + 'https://opentelemetry.test/schemas/1.2.3' + ); + }); }); describe('ProtobufMetricsSerializer', function () { diff --git a/experimental/packages/otlp-transformer/test/trace.test.ts b/experimental/packages/otlp-transformer/test/trace.test.ts index 6b0cd633bcb..c885dc540b7 100644 --- a/experimental/packages/otlp-transformer/test/trace.test.ts +++ b/experimental/packages/otlp-transformer/test/trace.test.ts @@ -443,6 +443,29 @@ describe('Trace', () => { ); }); }); + + it('serializes a span with resource schema URL', () => { + const resourceWithSchema = resourceFromAttributes( + { 'resource-attribute': 'resource attribute value' }, + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } + ); + + const spanWithSchema: ReadableSpan = { + ...span, + resource: resourceWithSchema, + }; + + const exportRequest = createExportTraceServiceRequest([spanWithSchema], { + useHex: true, + }); + + assert.ok(exportRequest); + assert.strictEqual(exportRequest.resourceSpans?.length, 1); + assert.strictEqual( + exportRequest.resourceSpans?.[0].schemaUrl, + 'https://opentelemetry.test/schemas/1.2.3' + ); + }); }); describe('ProtobufTracesSerializer', function () { From 11584ba4eaa4349877ad8dfccfd8848569b89b5e Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 11:32:31 +0700 Subject: [PATCH 06/17] delete duplicate test --- .../test/Resource.test.ts | 69 +++++++------------ 1 file changed, 25 insertions(+), 44 deletions(-) diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 708a13f81c9..09151f5e2fc 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -289,7 +289,7 @@ describe('Resource', () => { describe('schema URL support', () => { it('should create resource with schema URL', () => { - const schemaUrl = 'https://example.com/schema'; + const schemaUrl = 'https://example.test/schemas/1.2.3'; const resource = resourceFromAttributes({ attr: 'value' }, { schemaUrl }); assert.strictEqual(resource.getSchemaUrl?.(), schemaUrl); @@ -301,24 +301,8 @@ describe('Resource', () => { assert.strictEqual(resource.getSchemaUrl?.(), undefined); }); - it('should handle schema URL conflicts by returning undefined', () => { - const resource1 = resourceFromAttributes( - { attr1: 'value1' }, - { schemaUrl: 'https://schema1.com' } - ); - const resource2 = resourceFromAttributes( - { attr2: 'value2' }, - { schemaUrl: 'https://schema2.com' } - ); - - const mergedResource = resource1.merge(resource2); - - // When both resources have different non-empty schema URLs, return undefined - assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); - }); - it('should retain schema URL from base resource when other has no schema URL', () => { - const schemaUrl = 'https://example.com/schema'; + const schemaUrl = 'https://opentelemetry.test/schemas/1.2.3'; const resource1 = resourceFromAttributes( { attr1: 'value1' }, { schemaUrl } @@ -331,54 +315,48 @@ describe('Resource', () => { }); it('should retain schema URL from the resource that has it when merging', () => { - const resource1 = resourceFromAttributes( - { attr1: 'value1' }, - { schemaUrl: '' } - ); + const resource1 = resourceFromAttributes({ attr1: 'value1' }); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - { schemaUrl: 'https://example.com/schema' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); const mergedResource = resource1.merge(resource2); assert.strictEqual( mergedResource.getSchemaUrl?.(), - 'https://example.com/schema' + 'https://opentelemetry.test/schemas/1.2.3' ); }); - it('should have empty schema URL when merging resources with no schema URL', () => { + it('should retain schema URL from the resource that has it when merging', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, - { schemaUrl: '' } - ); - const resource2 = resourceFromAttributes( - { attr2: 'value2' }, - { schemaUrl: '' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); + const resource2 = resourceFromAttributes({ attr2: 'value2' }); const mergedResource = resource1.merge(resource2); - assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); + assert.strictEqual( + mergedResource.getSchemaUrl?.(), + 'https://opentelemetry.test/schemas/1.2.3' + ); }); - it('should handle merging with empty string schema URLs', () => { + it('should have empty schema URL when merging resources with no schema URL', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, { schemaUrl: '' } ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - { schemaUrl: 'https://valid.schema' } + { schemaUrl: '' } ); const mergedResource = resource1.merge(resource2); - assert.strictEqual( - mergedResource.getSchemaUrl?.(), - 'https://valid.schema' - ); + assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); }); it('should maintain backward compatibility - getSchemaUrl is optional', () => { @@ -397,7 +375,7 @@ describe('Resource', () => { setTimeout(() => resolve('fromasync'), 1) ), }, - { schemaUrl: 'https://async.schema' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); await resource.waitForAsyncAttributes?.(); @@ -406,24 +384,27 @@ describe('Resource', () => { sync: 'fromsync', async: 'fromasync', }); - assert.strictEqual(resource.getSchemaUrl?.(), 'https://async.schema'); + assert.strictEqual( + resource.getSchemaUrl?.(), + 'https://opentelemetry.test/schemas/1.2.3' + ); }); it('should merge schema URLs according to OpenTelemetry spec - same URLs', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, - { schemaUrl: 'https://same.schema' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - { schemaUrl: 'https://same.schema' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); const mergedResource = resource1.merge(resource2); assert.strictEqual( mergedResource.getSchemaUrl?.(), - 'https://same.schema' + 'https://opentelemetry.test/schemas/1.2.3' ); }); @@ -432,11 +413,11 @@ describe('Resource', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, - { schemaUrl: 'https://schema1.com' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); const resource2 = resourceFromAttributes( { attr2: 'value2' }, - { schemaUrl: 'https://schema2.com' } + { schemaUrl: 'https://opentelemetry.test/schemas/1.2.4' } ); const mergedResource = resource1.merge(resource2); From 9d3219140acfc9a6cac00c8e07d52af2b9a636a7 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 12:29:28 +0700 Subject: [PATCH 07/17] improve tests --- .../otlp-transformer/test/metrics.test.ts | 37 +++++++++---------- .../otlp-transformer/test/trace.test.ts | 26 +++++++------ 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/experimental/packages/otlp-transformer/test/metrics.test.ts b/experimental/packages/otlp-transformer/test/metrics.test.ts index 725d1e3ad2e..ffd100febb3 100644 --- a/experimental/packages/otlp-transformer/test/metrics.test.ts +++ b/experimental/packages/otlp-transformer/test/metrics.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { ValueType } from '@opentelemetry/api'; -import { resourceFromAttributes } from '@opentelemetry/resources'; +import { Resource, resourceFromAttributes } from '@opentelemetry/resources'; import { AggregationTemporality, DataPointType, @@ -301,10 +301,16 @@ describe('Metrics', () => { }; } - function createResourceMetrics(metricData: MetricData[]): ResourceMetrics { - const resource = resourceFromAttributes({ - 'resource-attribute': 'resource attribute value', - }); + function createResourceMetrics( + metricData: MetricData[], + customResource?: Resource + ): ResourceMetrics { + const resource = + customResource || + resourceFromAttributes({ + 'resource-attribute': 'resource attribute value', + }); + return { resource: resource, scopeMetrics: [ @@ -774,28 +780,19 @@ describe('Metrics', () => { }); }); - it('serializes a metric record with resource schema URL', () => { + it('supports schema URL on resource', () => { const resourceWithSchema = resourceFromAttributes( {}, { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - const resourceMetricsWithSchema: ResourceMetrics = { - resource: resourceWithSchema, - scopeMetrics: [ - { - scope: { - name: 'mylib', - version: '0.1.0', - schemaUrl: expectedSchemaUrl, - }, - metrics: [createCounterData(10, AggregationTemporality.DELTA)], - }, - ], - }; + const resourceMetrics = createResourceMetrics( + [createCounterData(10, AggregationTemporality.DELTA)], + resourceWithSchema + ); const exportRequest = createExportMetricsServiceRequest([ - resourceMetricsWithSchema, + resourceMetrics, ]); assert.ok(exportRequest); diff --git a/experimental/packages/otlp-transformer/test/trace.test.ts b/experimental/packages/otlp-transformer/test/trace.test.ts index c885dc540b7..b65e34fe556 100644 --- a/experimental/packages/otlp-transformer/test/trace.test.ts +++ b/experimental/packages/otlp-transformer/test/trace.test.ts @@ -232,11 +232,8 @@ describe('Trace', () => { let resource: Resource; let span: ReadableSpan; - beforeEach(() => { - resource = resourceFromAttributes({ - 'resource-attribute': 'resource attribute value', - }); - span = { + function createSpanWithResource(spanResource: Resource): ReadableSpan { + return { spanContext: () => ({ spanId: '0000000000000002', traceFlags: TraceFlags.SAMPLED, @@ -283,7 +280,7 @@ describe('Trace', () => { }, ], name: 'span-name', - resource, + resource: spanResource, startTime: [1640715557, 342725388], status: { code: SpanStatusCode.OK, @@ -292,6 +289,13 @@ describe('Trace', () => { droppedEventsCount: 0, droppedLinksCount: 0, }; + } + + beforeEach(() => { + resource = resourceFromAttributes({ + 'resource-attribute': 'resource attribute value', + }); + span = createSpanWithResource(resource); }); describe('createExportTraceServiceRequest', () => { @@ -444,18 +448,16 @@ describe('Trace', () => { }); }); - it('serializes a span with resource schema URL', () => { + it('supports schema URL on resource', () => { const resourceWithSchema = resourceFromAttributes( { 'resource-attribute': 'resource attribute value' }, { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - const spanWithSchema: ReadableSpan = { - ...span, - resource: resourceWithSchema, - }; + // Create span as if it came from SDK with this resource + const spanFromSDK = createSpanWithResource(resourceWithSchema); - const exportRequest = createExportTraceServiceRequest([spanWithSchema], { + const exportRequest = createExportTraceServiceRequest([spanFromSDK], { useHex: true, }); From 5540ba048f0aab3967adb5268d2781db8efd087f Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 12:52:05 +0700 Subject: [PATCH 08/17] make log test more useful --- .../otlp-transformer/test/logs.test.ts | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index c1ee79fd448..bdfea2836b1 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -166,22 +166,12 @@ describe('Logs', () => { // using `resource_2`, `scope_1`, `log_fragment_1` let log_2_1_1: ReadableLogRecord; - beforeEach(() => { - resource_1 = resourceFromAttributes({ - 'resource-attribute': 'some attribute value', - }); - resource_2 = resourceFromAttributes({ - 'resource-attribute': 'another attribute value', - }); - scope_1 = { - name: 'scope_name_1', - version: '0.1.0', - schemaUrl: 'http://url.to.schema', - }; - scope_2 = { - name: 'scope_name_2', - }; - const log_fragment_1 = { + function createLogWithResource( + resource: Resource, + scope: InstrumentationScope, + logFragment?: Partial + ): ReadableLogRecord { + const defaultFragment = { hrTime: [1680253513, 123241635] as HrTime, hrTimeObserved: [1683526948, 965142784] as HrTime, attributes: { @@ -198,6 +188,31 @@ describe('Logs', () => { traceId: '00000000000000000000000000000001', }, }; + + return { + ...defaultFragment, + ...logFragment, + resource: resource, + instrumentationScope: scope, + }; + } + + beforeEach(() => { + resource_1 = resourceFromAttributes({ + 'resource-attribute': 'some attribute value', + }); + resource_2 = resourceFromAttributes({ + 'resource-attribute': 'another attribute value', + }); + scope_1 = { + name: 'scope_name_1', + version: '0.1.0', + schemaUrl: 'http://url.to.schema', + }; + scope_2 = { + name: 'scope_name_2', + }; + const log_fragment_2 = { hrTime: [1680253797, 687038506] as HrTime, hrTimeObserved: [1680253797, 687038506] as HrTime, @@ -206,26 +221,12 @@ describe('Logs', () => { }, droppedAttributesCount: 0, }; - log_1_1_1 = { - ...log_fragment_1, - resource: resource_1, - instrumentationScope: scope_1, - }; - log_1_1_2 = { - ...log_fragment_2, - resource: resource_1, - instrumentationScope: scope_1, - }; - log_1_2_1 = { - ...log_fragment_1, - resource: resource_1, - instrumentationScope: scope_2, - }; - log_2_1_1 = { - ...log_fragment_1, - resource: resource_2, - instrumentationScope: scope_1, - }; + + // Use helper function for log creation (log_fragment_1 is the default) + log_1_1_1 = createLogWithResource(resource_1, scope_1); + log_1_1_2 = createLogWithResource(resource_1, scope_1, log_fragment_2); + log_1_2_1 = createLogWithResource(resource_1, scope_2); + log_2_1_1 = createLogWithResource(resource_2, scope_1); }); describe('createExportLogsServiceRequest', () => { @@ -295,14 +296,11 @@ describe('Logs', () => { it('supports schema URL on resource', () => { const resourceWithSchema = resourceFromAttributes( - {}, + { 'resource-attribute': 'some attribute value' }, { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - const logWithSchema: ReadableLogRecord = { - ...log_1_1_1, - resource: resourceWithSchema, - }; + const logWithSchema = createLogWithResource(resourceWithSchema, scope_1); const exportRequest = createExportLogsServiceRequest([logWithSchema], { useHex: true, From 38af840bea8e658bea0ee4a0fa540ef54bbae8cd Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 13:03:59 +0700 Subject: [PATCH 09/17] better log test --- .../otlp-transformer/test/logs.test.ts | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index bdfea2836b1..fd7b0b93810 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -144,6 +144,27 @@ function createExpectedLogProtobuf(): IExportLogsServiceRequest { }; } +const DEFAULT_LOG_FRAGMENT: Omit< + ReadableLogRecord, + 'resource' | 'instrumentationScope' +> = { + hrTime: [1680253513, 123241635] as HrTime, + hrTimeObserved: [1683526948, 965142784] as HrTime, + attributes: { + 'some-attribute': 'some attribute value', + }, + droppedAttributesCount: 0, + severityNumber: SeverityNumber.ERROR, + severityText: 'error', + body: 'some_log_body', + eventName: 'some.event.name', + spanContext: { + spanId: '0000000000000002', + traceFlags: TraceFlags.SAMPLED, + traceId: '00000000000000000000000000000001', + }, +} as const; + describe('Logs', () => { let resource_1: Resource; let resource_2: Resource; @@ -169,32 +190,13 @@ describe('Logs', () => { function createLogWithResource( resource: Resource, scope: InstrumentationScope, - logFragment?: Partial + logFragment: Omit ): ReadableLogRecord { - const defaultFragment = { - hrTime: [1680253513, 123241635] as HrTime, - hrTimeObserved: [1683526948, 965142784] as HrTime, - attributes: { - 'some-attribute': 'some attribute value', - }, - droppedAttributesCount: 0, - severityNumber: SeverityNumber.ERROR, - severityText: 'error', - body: 'some_log_body', - eventName: 'some.event.name', - spanContext: { - spanId: '0000000000000002', - traceFlags: TraceFlags.SAMPLED, - traceId: '00000000000000000000000000000001', - }, - }; - return { - ...defaultFragment, ...logFragment, resource: resource, instrumentationScope: scope, - }; + } as ReadableLogRecord; } beforeEach(() => { @@ -213,6 +215,7 @@ describe('Logs', () => { name: 'scope_name_2', }; + const log_fragment_1 = DEFAULT_LOG_FRAGMENT; const log_fragment_2 = { hrTime: [1680253797, 687038506] as HrTime, hrTimeObserved: [1680253797, 687038506] as HrTime, @@ -222,11 +225,11 @@ describe('Logs', () => { droppedAttributesCount: 0, }; - // Use helper function for log creation (log_fragment_1 is the default) - log_1_1_1 = createLogWithResource(resource_1, scope_1); + // Use helper function for log creation + log_1_1_1 = createLogWithResource(resource_1, scope_1, log_fragment_1); log_1_1_2 = createLogWithResource(resource_1, scope_1, log_fragment_2); - log_1_2_1 = createLogWithResource(resource_1, scope_2); - log_2_1_1 = createLogWithResource(resource_2, scope_1); + log_1_2_1 = createLogWithResource(resource_1, scope_2, log_fragment_1); + log_2_1_1 = createLogWithResource(resource_2, scope_1, log_fragment_1); }); describe('createExportLogsServiceRequest', () => { @@ -300,7 +303,11 @@ describe('Logs', () => { { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - const logWithSchema = createLogWithResource(resourceWithSchema, scope_1); + const logWithSchema = createLogWithResource( + resourceWithSchema, + scope_1, + DEFAULT_LOG_FRAGMENT + ); const exportRequest = createExportLogsServiceRequest([logWithSchema], { useHex: true, From ddf719f76182f6833e08cf33f10eb93d8c1a29b9 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 13:11:27 +0700 Subject: [PATCH 10/17] remove some comments that feel unnecessary --- experimental/packages/otlp-transformer/test/logs.test.ts | 3 +-- experimental/packages/otlp-transformer/test/trace.test.ts | 1 - packages/opentelemetry-resources/test/Resource.test.ts | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index fd7b0b93810..b82065893f7 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -225,7 +225,6 @@ describe('Logs', () => { droppedAttributesCount: 0, }; - // Use helper function for log creation log_1_1_1 = createLogWithResource(resource_1, scope_1, log_fragment_1); log_1_1_2 = createLogWithResource(resource_1, scope_1, log_fragment_2); log_1_2_1 = createLogWithResource(resource_1, scope_2, log_fragment_1); @@ -299,7 +298,7 @@ describe('Logs', () => { it('supports schema URL on resource', () => { const resourceWithSchema = resourceFromAttributes( - { 'resource-attribute': 'some attribute value' }, + {}, { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); diff --git a/experimental/packages/otlp-transformer/test/trace.test.ts b/experimental/packages/otlp-transformer/test/trace.test.ts index b65e34fe556..f76ea753f24 100644 --- a/experimental/packages/otlp-transformer/test/trace.test.ts +++ b/experimental/packages/otlp-transformer/test/trace.test.ts @@ -454,7 +454,6 @@ describe('Trace', () => { { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - // Create span as if it came from SDK with this resource const spanFromSDK = createSpanWithResource(resourceWithSchema); const exportRequest = createExportTraceServiceRequest([spanFromSDK], { diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 09151f5e2fc..7014c5c8045 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -362,7 +362,6 @@ describe('Resource', () => { it('should maintain backward compatibility - getSchemaUrl is optional', () => { const resource = emptyResource(); - // This should not throw even if getSchemaUrl is not implemented const schemaUrl = resource.getSchemaUrl?.(); assert.strictEqual(schemaUrl, undefined); }); @@ -408,7 +407,7 @@ describe('Resource', () => { ); }); - it('should merge schema URLs according to OpenTelemetry spec - conflict case', () => { + it('should merge schema URLs according to OpenTelemetry spec - conflict case (undefined behavior)', () => { const warnStub = sinon.spy(diag, 'warn'); const resource1 = resourceFromAttributes( From df8e255f2014a55afa898e220772674c2f7ee5b2 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 13:23:43 +0700 Subject: [PATCH 11/17] rename to `createReadableLogRecord` --- .../packages/otlp-transformer/test/logs.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/experimental/packages/otlp-transformer/test/logs.test.ts b/experimental/packages/otlp-transformer/test/logs.test.ts index b82065893f7..4ddd4ff3b45 100644 --- a/experimental/packages/otlp-transformer/test/logs.test.ts +++ b/experimental/packages/otlp-transformer/test/logs.test.ts @@ -187,7 +187,7 @@ describe('Logs', () => { // using `resource_2`, `scope_1`, `log_fragment_1` let log_2_1_1: ReadableLogRecord; - function createLogWithResource( + function createReadableLogRecord( resource: Resource, scope: InstrumentationScope, logFragment: Omit @@ -225,10 +225,10 @@ describe('Logs', () => { droppedAttributesCount: 0, }; - log_1_1_1 = createLogWithResource(resource_1, scope_1, log_fragment_1); - log_1_1_2 = createLogWithResource(resource_1, scope_1, log_fragment_2); - log_1_2_1 = createLogWithResource(resource_1, scope_2, log_fragment_1); - log_2_1_1 = createLogWithResource(resource_2, scope_1, log_fragment_1); + log_1_1_1 = createReadableLogRecord(resource_1, scope_1, log_fragment_1); + log_1_1_2 = createReadableLogRecord(resource_1, scope_1, log_fragment_2); + log_1_2_1 = createReadableLogRecord(resource_1, scope_2, log_fragment_1); + log_2_1_1 = createReadableLogRecord(resource_2, scope_1, log_fragment_1); }); describe('createExportLogsServiceRequest', () => { @@ -302,7 +302,7 @@ describe('Logs', () => { { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } ); - const logWithSchema = createLogWithResource( + const logWithSchema = createReadableLogRecord( resourceWithSchema, scope_1, DEFAULT_LOG_FRAGMENT From 9046e4d36b345b1f31e0394403d93f8e156fba18 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 16 Jun 2025 13:40:01 +0700 Subject: [PATCH 12/17] don't need this comment either --- packages/opentelemetry-resources/test/Resource.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 7014c5c8045..389fd22aa9d 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -424,7 +424,7 @@ describe('Resource', () => { // Implementation-specific: we return undefined to indicate error state // This aligns with Go, Java, and PHP SDKs which return null/empty for conflicts assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); - // Should log a warning about the conflict + assert.ok(warnStub.calledWithMatch('Schema URL merge conflict')); warnStub.restore(); From c8f4262c4657d35639485e48cf7b1ea350c1c714 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 23 Jun 2025 12:26:37 +0700 Subject: [PATCH 13/17] remove duplicate test case, consistent naming --- .../test/Resource.test.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 389fd22aa9d..4e11a07b6af 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -314,7 +314,7 @@ describe('Resource', () => { assert.strictEqual(mergedResource.getSchemaUrl?.(), schemaUrl); }); - it('should retain schema URL from the resource that has it when merging', () => { + it('should retain schema URL from other resource when base has no schema URL', () => { const resource1 = resourceFromAttributes({ attr1: 'value1' }); const resource2 = resourceFromAttributes( { attr2: 'value2' }, @@ -329,21 +329,6 @@ describe('Resource', () => { ); }); - it('should retain schema URL from the resource that has it when merging', () => { - const resource1 = resourceFromAttributes( - { attr1: 'value1' }, - { schemaUrl: 'https://opentelemetry.test/schemas/1.2.3' } - ); - const resource2 = resourceFromAttributes({ attr2: 'value2' }); - - const mergedResource = resource1.merge(resource2); - - assert.strictEqual( - mergedResource.getSchemaUrl?.(), - 'https://opentelemetry.test/schemas/1.2.3' - ); - }); - it('should have empty schema URL when merging resources with no schema URL', () => { const resource1 = resourceFromAttributes( { attr1: 'value1' }, From b082bfb115c590ab52fc9818076dad519c6febf6 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 23 Jun 2025 13:18:27 +0700 Subject: [PATCH 14/17] add schema url validation --- .../src/ResourceImpl.ts | 28 ++++++++- .../test/Resource.test.ts | 62 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 035dd637f47..00ac2f2eb9d 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -71,7 +71,7 @@ class ResourceImpl implements Resource { }); this._rawAttributes = guardedRawAttributes(this._rawAttributes); - this._schemaUrl = options?.schemaUrl; + this._schemaUrl = validateSchemaUrl(options?.schemaUrl); } public get asyncAttributesPending(): boolean { @@ -194,6 +194,32 @@ function guardedRawAttributes( }); } +function validateSchemaUrl(schemaUrl?: string): string | undefined { + if (schemaUrl === undefined || schemaUrl === '') { + return schemaUrl; + } + + try { + const url = new URL(schemaUrl); + + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + diag.warn( + 'Invalid schema URL format: "%s". Schema URL must be a valid URI with http:// or https://. Schema URL will be ignored.', + schemaUrl + ); + return undefined; + } + + return schemaUrl; + } catch { + diag.warn( + 'Invalid schema URL format: "%s". Schema URL must be a valid URI with http:// or https://. Schema URL will be ignored.', + schemaUrl + ); + return undefined; + } +} + function mergeSchemaUrl( old: Resource, updating: Resource | null diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 4e11a07b6af..8f3ed1857a6 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -414,6 +414,68 @@ describe('Resource', () => { warnStub.restore(); }); + + it('should accept valid schema URL formats', () => { + const validSchemaUrls = [ + 'https://opentelemetry.test/schemas/1.2.3', + 'http://example.test/schema', + 'https://schemas.opentelemetry.test/path/to/schema/1.21.0', + 'https://example.test:8080/path/to/schema', + ]; + + validSchemaUrls.forEach(validUrl => { + const resource = resourceFromAttributes( + { attr: 'value' }, + { schemaUrl: validUrl } + ); + + assert.strictEqual( + resource.getSchemaUrl?.(), + validUrl, + `Expected valid schema URL to be preserved: ${validUrl}` + ); + }); + }); + + it('should handle invalid schema URL formats gracefully', () => { + const warnStub = sinon.spy(diag, 'warn'); + + const invalidSchemaUrls = [ + 'not-a-url', + 'relative/path', + 'http://', + 'https://', + '://missing-scheme.test', + 'http://[invalid-ipv6', + 'https://invalid space.test', + 'ht tp://space-in-scheme.test', + 'http://example.test:invalid-port', + 'ftp://example.test/schema', + 'custom://example.test/schema', + 'file:///path/to/schema', + 'ws://example.test/schema', + ]; + + invalidSchemaUrls.forEach(invalidUrl => { + const resource = resourceFromAttributes( + { attr: 'value' }, + { schemaUrl: invalidUrl } + ); + + // Invalid schema URLs should be ignored (set to undefined) + assert.strictEqual( + resource.getSchemaUrl?.(), + undefined, + `Expected undefined for invalid schema URL: ${invalidUrl}` + ); + }); + + // Should have logged warnings for each invalid URL + assert.strictEqual(warnStub.callCount, invalidSchemaUrls.length); + assert.ok(warnStub.alwaysCalledWithMatch('Invalid schema URL format')); + + warnStub.restore(); + }); }); describeNode('.default()', () => { From a62f67e06928276b05b8f9afe11a7bea184eb33e Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Mon, 23 Jun 2025 13:29:44 +0700 Subject: [PATCH 15/17] make schemaURL readonly, use a getter in ResourceImpl to access --- .../otlp-transformer/src/common/internal.ts | 2 +- .../opentelemetry-resources/src/Resource.ts | 12 ++++------ .../src/ResourceImpl.ts | 6 ++--- .../test/Resource.test.ts | 24 +++++++++---------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/experimental/packages/otlp-transformer/src/common/internal.ts b/experimental/packages/otlp-transformer/src/common/internal.ts index ded6a527af0..cc9351a61fa 100644 --- a/experimental/packages/otlp-transformer/src/common/internal.ts +++ b/experimental/packages/otlp-transformer/src/common/internal.ts @@ -29,7 +29,7 @@ export function createResource(resource: ISdkResource): Resource { droppedAttributesCount: 0, }; - const schemaUrl = resource.getSchemaUrl?.(); + const schemaUrl = resource.schemaUrl; if (schemaUrl && schemaUrl !== '') result.schemaUrl = schemaUrl; return result; diff --git a/packages/opentelemetry-resources/src/Resource.ts b/packages/opentelemetry-resources/src/Resource.ts index 9acb4cfac35..c8e25477490 100644 --- a/packages/opentelemetry-resources/src/Resource.ts +++ b/packages/opentelemetry-resources/src/Resource.ts @@ -41,6 +41,11 @@ export interface Resource { */ readonly attributes: Attributes; + /** + * @returns the Resource's schema URL or undefined if not set. + */ + readonly schemaUrl?: string; + /** * Returns a promise that will never be rejected. Resolves when all async attributes have finished being added to * this Resource's attributes. This is useful in exporters to block until resource detection @@ -59,11 +64,4 @@ export interface Resource { merge(other: Resource | null): Resource; getRawAttributes(): RawResourceAttribute[]; - - /** - * Get the schema URL for this resource. - * - * @returns the schema URL or undefined if not set - */ - getSchemaUrl?(): string | undefined; } diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 00ac2f2eb9d..fb863f10774 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -125,7 +125,7 @@ class ResourceImpl implements Resource { return this._rawAttributes; } - public getSchemaUrl(): string | undefined { + public get schemaUrl(): string | undefined { return this._schemaUrl; } @@ -224,8 +224,8 @@ function mergeSchemaUrl( old: Resource, updating: Resource | null ): string | undefined { - const oldSchemaUrl = old?.getSchemaUrl?.(); - const updatingSchemaUrl = updating?.getSchemaUrl?.(); + const oldSchemaUrl = old?.schemaUrl; + const updatingSchemaUrl = updating?.schemaUrl; const isOldEmpty = oldSchemaUrl === undefined || oldSchemaUrl === ''; const isUpdatingEmpty = diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index 8f3ed1857a6..816b0dae354 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -292,13 +292,13 @@ describe('Resource', () => { const schemaUrl = 'https://example.test/schemas/1.2.3'; const resource = resourceFromAttributes({ attr: 'value' }, { schemaUrl }); - assert.strictEqual(resource.getSchemaUrl?.(), schemaUrl); + assert.strictEqual(resource.schemaUrl, schemaUrl); }); it('should create resource without schema URL', () => { const resource = resourceFromAttributes({ attr: 'value' }); - assert.strictEqual(resource.getSchemaUrl?.(), undefined); + assert.strictEqual(resource.schemaUrl, undefined); }); it('should retain schema URL from base resource when other has no schema URL', () => { @@ -311,7 +311,7 @@ describe('Resource', () => { const mergedResource = resource1.merge(resource2); - assert.strictEqual(mergedResource.getSchemaUrl?.(), schemaUrl); + assert.strictEqual(mergedResource.schemaUrl, schemaUrl); }); it('should retain schema URL from other resource when base has no schema URL', () => { @@ -324,7 +324,7 @@ describe('Resource', () => { const mergedResource = resource1.merge(resource2); assert.strictEqual( - mergedResource.getSchemaUrl?.(), + mergedResource.schemaUrl, 'https://opentelemetry.test/schemas/1.2.3' ); }); @@ -341,13 +341,13 @@ describe('Resource', () => { const mergedResource = resource1.merge(resource2); - assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); + assert.strictEqual(mergedResource.schemaUrl, undefined); }); - it('should maintain backward compatibility - getSchemaUrl is optional', () => { + it('should maintain backward compatibility - schemaUrl is optional', () => { const resource = emptyResource(); - const schemaUrl = resource.getSchemaUrl?.(); + const schemaUrl = resource.schemaUrl; assert.strictEqual(schemaUrl, undefined); }); @@ -369,7 +369,7 @@ describe('Resource', () => { async: 'fromasync', }); assert.strictEqual( - resource.getSchemaUrl?.(), + resource.schemaUrl, 'https://opentelemetry.test/schemas/1.2.3' ); }); @@ -387,7 +387,7 @@ describe('Resource', () => { const mergedResource = resource1.merge(resource2); assert.strictEqual( - mergedResource.getSchemaUrl?.(), + mergedResource.schemaUrl, 'https://opentelemetry.test/schemas/1.2.3' ); }); @@ -408,7 +408,7 @@ describe('Resource', () => { // Implementation-specific: we return undefined to indicate error state // This aligns with Go, Java, and PHP SDKs which return null/empty for conflicts - assert.strictEqual(mergedResource.getSchemaUrl?.(), undefined); + assert.strictEqual(mergedResource.schemaUrl, undefined); assert.ok(warnStub.calledWithMatch('Schema URL merge conflict')); @@ -430,7 +430,7 @@ describe('Resource', () => { ); assert.strictEqual( - resource.getSchemaUrl?.(), + resource.schemaUrl, validUrl, `Expected valid schema URL to be preserved: ${validUrl}` ); @@ -464,7 +464,7 @@ describe('Resource', () => { // Invalid schema URLs should be ignored (set to undefined) assert.strictEqual( - resource.getSchemaUrl?.(), + resource.schemaUrl, undefined, `Expected undefined for invalid schema URL: ${invalidUrl}` ); From 5190bccf7ed1fc64e5961003cd177b2e29189279 Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Wed, 23 Jul 2025 11:19:56 +0700 Subject: [PATCH 16/17] Explicitly reject URLs with spaces --- .../src/ResourceImpl.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index fb863f10774..28e264c8b77 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -33,6 +33,9 @@ import { } from './types'; import { isPromiseLike } from './utils'; +const INVALID_SCHEMA_URL_MESSAGE = + 'Invalid schema URL format: "%s". Schema URL must be a valid URI with http:// or https://. Schema URL will be ignored.'; + class ResourceImpl implements Resource { private _rawAttributes: RawResourceAttribute[]; private _asyncAttributesPending = false; @@ -199,23 +202,26 @@ function validateSchemaUrl(schemaUrl?: string): string | undefined { return schemaUrl; } + if ( + schemaUrl.includes(' ') || + schemaUrl.includes('\t') || + schemaUrl.includes('\n') + ) { + diag.warn(INVALID_SCHEMA_URL_MESSAGE, schemaUrl); + return undefined; + } + try { const url = new URL(schemaUrl); if (url.protocol !== 'http:' && url.protocol !== 'https:') { - diag.warn( - 'Invalid schema URL format: "%s". Schema URL must be a valid URI with http:// or https://. Schema URL will be ignored.', - schemaUrl - ); + diag.warn(INVALID_SCHEMA_URL_MESSAGE, schemaUrl); return undefined; } return schemaUrl; } catch { - diag.warn( - 'Invalid schema URL format: "%s". Schema URL must be a valid URI with http:// or https://. Schema URL will be ignored.', - schemaUrl - ); + diag.warn(INVALID_SCHEMA_URL_MESSAGE, schemaUrl); return undefined; } } From 659b02c3552159221bb62a4fb6cc7177634f666d Mon Sep 17 00:00:00 2001 From: c-ehrlich Date: Wed, 23 Jul 2025 11:31:30 +0700 Subject: [PATCH 17/17] check for more control characters --- packages/opentelemetry-resources/src/ResourceImpl.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-resources/src/ResourceImpl.ts b/packages/opentelemetry-resources/src/ResourceImpl.ts index 28e264c8b77..8c381b2f983 100644 --- a/packages/opentelemetry-resources/src/ResourceImpl.ts +++ b/packages/opentelemetry-resources/src/ResourceImpl.ts @@ -202,11 +202,10 @@ function validateSchemaUrl(schemaUrl?: string): string | undefined { return schemaUrl; } - if ( - schemaUrl.includes(' ') || - schemaUrl.includes('\t') || - schemaUrl.includes('\n') - ) { + // Reject any ASCII C0 control (0x00–0x1F), SPACE (0x20) or DEL (0x7F) + // This ensures consistent behavior across Node.js and browser environments + // eslint-disable-next-line no-control-regex + if (/[\x00-\x20\x7F]/.test(schemaUrl)) { diag.warn(INVALID_SCHEMA_URL_MESSAGE, schemaUrl); return undefined; }