-
Notifications
You must be signed in to change notification settings - Fork 896
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
ccb2347
72c7d8d
35b4305
e69c81d
eae14ba
39bc4fe
11584ba
1f5393d
9d32191
5540ba0
38af840
ddf719f
df8e255
9046e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 cc @dyladan do you have a preference? (since you re-did resources in the latest major version) |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' | ||
); | ||
}); | ||
pichlermarc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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' | ||
pichlermarc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
|
||
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'); | ||
}); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
Uh oh!
There was an error while loading. Please reload this page.