Skip to content

feat(opentelemetry-resources): add schema url #5753

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

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

### :rocket: Features

* feat(opentelemetry-resources): add schema url [#5070](https://github.com/open-telemetry/opentelemetry-js/pull/5753) @c-ehrlich

### :bug: Bug Fixes

### :books: Documentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export interface Resource {

/** Resource droppedAttributesCount */
droppedAttributesCount: number;

/** Resource schemaUrl */
schemaUrl?: string;
}

/** Properties of an InstrumentationScope. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 7 additions & 0 deletions packages/opentelemetry-resources/src/Resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +62 to +68
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer a readonly property, then use a getter in the ResourceImpl to enforce it, like we do with the other properties to keep it consistent.

cc @dyladan do you have a preference? (since you re-did resources in the latest major version)

}
54 changes: 43 additions & 11 deletions packages/opentelemetry-resources/src/ResourceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AttributeValue | undefined>][]
attributes: [string, MaybePromise<AttributeValue | undefined>][],
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;
Expand All @@ -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]) => {
Expand All @@ -67,6 +70,7 @@ class ResourceImpl implements Resource {
});

this._rawAttributes = guardedRawAttributes(this._rawAttributes);
this._schemaUrl = schemaUrl;
}

public get asyncAttributesPending(): boolean {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
110 changes: 110 additions & 0 deletions packages/opentelemetry-resources/test/Resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you might have a duplicated test case between this and the next case.

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');
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to add a negative test to ensure invalid schema URL formats are handled gracefully.


describeNode('.default()', () => {
it('should return a default resource', () => {
const resource = defaultResource();
Expand Down