Skip to content
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

Converge sync and async resources #5350

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0ac450e
WIP sync and async resource merge
dyladan Jan 16, 2025
49ef5ed
Fix isPromiseLike
dyladan Jan 16, 2025
29ccd0f
Fix tests - TODO: attribute fallback order
dyladan Jan 17, 2025
767947c
Fix resource usages for 2.x
dyladan Jan 21, 2025
9aba287
Fix remaining tests
dyladan Jan 22, 2025
9f231aa
Unskip tests
dyladan Jan 22, 2025
13f2064
Remove deep export
dyladan Jan 22, 2025
2fa42c8
Review comments
dyladan Jan 22, 2025
ff5af1a
Remove noop detector export
dyladan Jan 22, 2025
8e513e4
Merge remote-tracking branch 'origin/main' into resource-async-sync
dyladan Jan 22, 2025
8236217
Lint and cleanup
dyladan Jan 22, 2025
36d6b9c
Changelog
dyladan Jan 22, 2025
c1f6d24
Changelog
dyladan Jan 22, 2025
1de27ba
Use shared empty resource
dyladan Jan 23, 2025
20d297d
Make asyncAttributesPending readonly
dyladan Jan 23, 2025
6d1f3a0
Add explicit return type to resource merge
dyladan Jan 23, 2025
f13ba2c
Fix types and lint
dyladan Jan 23, 2025
15f54f0
Create new resource from list
dyladan Jan 23, 2025
e6dce6b
Explict return types
dyladan Jan 27, 2025
7a93c59
revert package lock changes
dyladan Jan 27, 2025
0467bc8
Merge remote-tracking branch 'origin/main' into resource-async-sync
dyladan Jan 27, 2025
fd4bab8
Revert package lock changes
dyladan Jan 27, 2025
9c74732
Use strict equal
dyladan Jan 27, 2025
9e55991
Fix test compilation
dyladan Jan 28, 2025
eba4a63
Do not await non-promise
dyladan Jan 29, 2025
cd8a8f1
Review comment
dyladan Jan 29, 2025
dde176a
Add comment about merge order
dyladan Jan 31, 2025
ee3e732
Merge remote-tracking branch 'origin/main' into resource-async-sync
dyladan Jan 31, 2025
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* (user-facing): deprecated `AlwaysOffSampler` has moved to `@opentelemetry/sdk-trace-base`
* (user-facing): deprecated `TraceIdRatioSampler` has moved to `@opentelemetry/sdk-trace-base`
* (user-facing): deprecated `TraceIdRatioSampler` has moved to `@opentelemetry/sdk-trace-base`
* feat(resource): Merge sync and async resource interfaces into a single interface [#5350](https://github.com/open-telemetry/opentelemetry-js/pull/5350) @dyladan
* Resource constructor now takes a single argument which contains an optional `attributes` object
* Detected resource attribute values may be a promise or a synchronous value
* Resources are now merged by the order in which their detectors are configured instead of async attributes being last
* Resource detectors now return `DetectedResource` plain objects instead of `new Resource()`

### :rocket: (Enhancement)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@

import { Attributes, diag } from '@opentelemetry/api';
import {
Detector,
IResource,
ResourceDetector,
Resource,
ResourceDetectionConfig,
} from '@opentelemetry/resources';
import { BROWSER_ATTRIBUTES, UserAgentData } from './types';
import { DetectedResource } from '@opentelemetry/resources';

/**
* BrowserDetector will be used to detect the resources related to browser.
*/
class BrowserDetector implements Detector {
async detect(config?: ResourceDetectionConfig): Promise<IResource> {
class BrowserDetector implements ResourceDetector {
detect(config?: ResourceDetectionConfig): DetectedResource {

Check warning on line 30 in experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts

View check run for this annotation

Codecov / codecov/patch

experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts#L30

Added line #L30 was not covered by tests
const isBrowser = typeof navigator !== 'undefined';
if (!isBrowser) {
return Resource.empty();
return Resource.EMPTY;

Check warning on line 33 in experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts

View check run for this annotation

Codecov / codecov/patch

experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts#L33

Added line #L33 was not covered by tests
}
const browserResource: Attributes = getBrowserAttributes();
return this._getResourceAttributes(browserResource, config);
Expand All @@ -45,17 +45,17 @@
private _getResourceAttributes(
browserResource: Attributes,
_config?: ResourceDetectionConfig
) {
): DetectedResource {
if (
!browserResource[BROWSER_ATTRIBUTES.USER_AGENT] &&
!browserResource[BROWSER_ATTRIBUTES.PLATFORM]
) {
diag.debug(
'BrowserDetector failed: Unable to find required browser resources. '
);
return Resource.empty();
return Resource.EMPTY;

Check warning on line 56 in experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts

View check run for this annotation

Codecov / codecov/patch

experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts#L56

Added line #L56 was not covered by tests
} else {
return new Resource(browserResource);
return { attributes: browserResource };

Check warning on line 58 in experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts

View check run for this annotation

Codecov / codecov/patch

experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts#L58

Added line #L58 was not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
* limitations under the License.
*/
import * as sinon from 'sinon';
import { IResource } from '@opentelemetry/resources';
import { browserDetector } from '../src/BrowserDetector';
import { describeBrowser, assertResource, assertEmptyResource } from './util';
import { assertEmptyResource, assertResource, describeBrowser } from './util';

describeBrowser('browserDetector()', () => {
afterEach(() => {
Expand Down Expand Up @@ -47,7 +46,7 @@ describeBrowser('browserDetector()', () => {
},
});

const resource: IResource = await browserDetector.detect();
const resource = browserDetector.detect();
assertResource(resource, {
platform: 'platform',
brands: ['Chromium 106', 'Google Chrome 106', 'Not;A=Brand 99'],
Expand All @@ -63,7 +62,7 @@ describeBrowser('browserDetector()', () => {
userAgentData: undefined,
});

const resource: IResource = await browserDetector.detect();
const resource = browserDetector.detect();
assertResource(resource, {
language: 'en-US',
user_agent: 'dddd',
Expand All @@ -74,7 +73,7 @@ describeBrowser('browserDetector()', () => {
sinon.stub(globalThis, 'navigator').value({
userAgent: '',
});
const resource: IResource = await browserDetector.detect();
const resource = browserDetector.detect();
assertEmptyResource(resource);
});
});
20 changes: 10 additions & 10 deletions experimental/packages/opentelemetry-browser-detector/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { Suite } from 'mocha';
import * as assert from 'assert';
import { BROWSER_ATTRIBUTES } from '../src/types';
import { IResource } from '@opentelemetry/resources';
import { DetectedResource } from '@opentelemetry/resources';

export function describeBrowser(title: string, fn: (this: Suite) => void) {
title = `Browser: ${title}`;
Expand All @@ -27,7 +27,7 @@ export function describeBrowser(title: string, fn: (this: Suite) => void) {
}

export const assertResource = (
resource: IResource,
resource: DetectedResource,
validations: {
platform?: string;
brands?: string[];
Expand All @@ -38,32 +38,32 @@ export const assertResource = (
) => {
if (validations.platform) {
assert.strictEqual(
resource.attributes[BROWSER_ATTRIBUTES.PLATFORM],
resource.attributes?.[BROWSER_ATTRIBUTES.PLATFORM],
validations.platform
);
}
if (validations.brands) {
assert.ok(Array.isArray(resource.attributes[BROWSER_ATTRIBUTES.BRANDS]));
assert.ok(Array.isArray(resource.attributes?.[BROWSER_ATTRIBUTES.BRANDS]));
assert.deepStrictEqual(
resource.attributes[BROWSER_ATTRIBUTES.BRANDS] as string[],
resource.attributes?.[BROWSER_ATTRIBUTES.BRANDS] as string[],
validations.brands
);
}
if (validations.mobile) {
assert.strictEqual(
resource.attributes[BROWSER_ATTRIBUTES.MOBILE],
resource.attributes?.[BROWSER_ATTRIBUTES.MOBILE],
validations.mobile
);
}
if (validations.language) {
assert.strictEqual(
resource.attributes[BROWSER_ATTRIBUTES.LANGUAGE],
resource.attributes?.[BROWSER_ATTRIBUTES.LANGUAGE],
validations.language
);
}
if (validations.user_agent) {
assert.strictEqual(
resource.attributes[BROWSER_ATTRIBUTES.USER_AGENT],
resource.attributes?.[BROWSER_ATTRIBUTES.USER_AGENT],
validations.user_agent
);
}
Expand All @@ -74,6 +74,6 @@ export const assertResource = (
*
* @param resource the Resource to validate
*/
export const assertEmptyResource = (resource: IResource) => {
assert.strictEqual(Object.keys(resource.attributes).length, 0);
export const assertEmptyResource = (resource: DetectedResource) => {
assert.strictEqual(Object.keys(resource.attributes ?? {}).length, 0);
};
Original file line number Diff line number Diff line change
Expand Up @@ -703,11 +703,13 @@ describe('PrometheusSerializer', () => {
const serializer = new PrometheusSerializer(undefined, true);
const result = serializer['_serializeResource'](
new Resource({
env: 'prod',
hostname: 'myhost',
datacenter: 'sdc',
region: 'europe',
owner: 'frontend',
attributes: {
env: 'prod',
hostname: 'myhost',
datacenter: 'sdc',
region: 'europe',
owner: 'frontend',
},
})
);

Expand Down
15 changes: 7 additions & 8 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ import {
registerInstrumentations,
} from '@opentelemetry/instrumentation';
import {
Detector,
DetectorSync,
detectResourcesSync,
detectResources,
envDetector,
hostDetector,
IResource,
processDetector,
Resource,
ResourceDetectionConfig,
ResourceDetector,
} from '@opentelemetry/resources';
import {
LogRecordProcessor,
Expand Down Expand Up @@ -209,7 +208,7 @@ export class NodeSDK {
private _instrumentations: Instrumentation[];

private _resource: IResource;
private _resourceDetectors: Array<Detector | DetectorSync>;
private _resourceDetectors: Array<ResourceDetector>;

private _autoDetectResources: boolean;

Expand Down Expand Up @@ -345,17 +344,17 @@ export class NodeSDK {
detectors: this._resourceDetectors,
};

this._resource = this._resource.merge(
detectResourcesSync(internalConfig)
);
this._resource = this._resource.merge(detectResources(internalConfig));
}

this._resource =
this._serviceName === undefined
? this._resource
: this._resource.merge(
new Resource({
[ATTR_SERVICE_NAME]: this._serviceName,
attributes: {
[ATTR_SERVICE_NAME]: this._serviceName,
},
})
);

Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { ContextManager } from '@opentelemetry/api';
import { TextMapPropagator } from '@opentelemetry/api';
import { Instrumentation } from '@opentelemetry/instrumentation';
import { Detector, DetectorSync, IResource } from '@opentelemetry/resources';
import { IResource, ResourceDetector } from '@opentelemetry/resources';
import { LogRecordProcessor } from '@opentelemetry/sdk-logs';
import { MetricReader, ViewOptions } from '@opentelemetry/sdk-metrics';
import {
Expand All @@ -39,7 +39,7 @@ export interface NodeSDKConfiguration {
views: ViewOptions[];
instrumentations: (Instrumentation | Instrumentation[])[];
resource: IResource;
resourceDetectors: Array<Detector | DetectorSync>;
resourceDetectors: Array<ResourceDetector>;
sampler: Sampler;
serviceName?: string;
/** @deprecated use spanProcessors instead*/
Expand Down
26 changes: 13 additions & 13 deletions experimental/packages/opentelemetry-sdk-node/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import { OTLPTraceExporter as OTLPHttpTraceExporter } from '@opentelemetry/expor
import { OTLPTraceExporter as OTLPGrpcTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
import { ZipkinExporter } from '@opentelemetry/exporter-zipkin';
import {
DetectorSync,
envDetectorSync,
hostDetectorSync,
osDetectorSync,
processDetectorSync,
serviceInstanceIdDetectorSync,
envDetector,
hostDetector,
osDetector,
processDetector,
ResourceDetector,
serviceInstanceIdDetector,
} from '@opentelemetry/resources';
import {
BatchSpanProcessor,
Expand All @@ -42,14 +42,14 @@ const RESOURCE_DETECTOR_OS = 'os';
const RESOURCE_DETECTOR_PROCESS = 'process';
const RESOURCE_DETECTOR_SERVICE_INSTANCE_ID = 'serviceinstance';

export function getResourceDetectorsFromEnv(): Array<DetectorSync> {
export function getResourceDetectorsFromEnv(): Array<ResourceDetector> {
// When updating this list, make sure to also update the section `resourceDetectors` on README.
const resourceDetectors = new Map<string, DetectorSync>([
[RESOURCE_DETECTOR_ENVIRONMENT, envDetectorSync],
[RESOURCE_DETECTOR_HOST, hostDetectorSync],
[RESOURCE_DETECTOR_OS, osDetectorSync],
[RESOURCE_DETECTOR_SERVICE_INSTANCE_ID, serviceInstanceIdDetectorSync],
[RESOURCE_DETECTOR_PROCESS, processDetectorSync],
const resourceDetectors = new Map<string, ResourceDetector>([
[RESOURCE_DETECTOR_ENVIRONMENT, envDetector],
[RESOURCE_DETECTOR_HOST, hostDetector],
[RESOURCE_DETECTOR_OS, osDetector],
[RESOURCE_DETECTOR_SERVICE_INSTANCE_ID, serviceInstanceIdDetector],
[RESOURCE_DETECTOR_PROCESS, processDetector],
]);

const resourceDetectorsFromEnv =
Expand Down
20 changes: 12 additions & 8 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ import { NodeSDK } from '../src';
import { env } from 'process';
import {
envDetector,
envDetectorSync,
processDetector,
hostDetector,
Resource,
serviceInstanceIdDetectorSync,
serviceInstanceIdDetector,
DetectedResource,
} from '@opentelemetry/resources';
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
import { logs, ProxyLoggerProvider } from '@opentelemetry/api-logs';
Expand Down Expand Up @@ -525,8 +525,10 @@ describe('Node SDK', () => {
resourceDetectors: [
processDetector,
{
async detect(): Promise<Resource> {
return new Resource({ customAttr: 'someValue' });
detect(): DetectedResource {
return {
attributes: { customAttr: 'someValue' },
};
},
},
envDetector,
Expand Down Expand Up @@ -690,7 +692,7 @@ describe('Node SDK', () => {
await sdk1.shutdown();

const sdk2 = new NodeSDK({
resourceDetectors: [envDetectorSync],
resourceDetectors: [envDetector],
});
sdk2.start();
await sdk2.shutdown();
Expand Down Expand Up @@ -855,7 +857,7 @@ describe('Node SDK', () => {
processDetector,
envDetector,
hostDetector,
serviceInstanceIdDetectorSync,
serviceInstanceIdDetector,
],
});

Expand Down Expand Up @@ -925,8 +927,10 @@ describe('Node SDK', () => {
resourceDetectors: [
processDetector,
{
async detect(): Promise<Resource> {
return new Resource({ customAttr: 'someValue' });
detect(): DetectedResource {
return {
attributes: { customAttr: 'someValue' },
};
},
},
envDetector,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ const assertHasOneLabel = (prefix: string, resource: Resource): void => {
);
};

export const assertServiceInstanceIdIsUUID = (resource: Resource): void => {
export const assertServiceInstanceIdIsUUID = (resource: IResource): void => {
const UUID_REGEX =
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
assert.equal(
Expand Down
8 changes: 6 additions & 2 deletions experimental/packages/otlp-transformer/test/logs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,14 @@ describe('Logs', () => {

beforeEach(() => {
resource_1 = new Resource({
'resource-attribute': 'some attribute value',
attributes: {
'resource-attribute': 'some attribute value',
},
});
resource_2 = new Resource({
'resource-attribute': 'another attribute value',
attributes: {
'resource-attribute': 'another attribute value',
},
});
scope_1 = {
name: 'scope_name_1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ describe('Metrics', () => {

function createResourceMetrics(metricData: MetricData[]): ResourceMetrics {
const resource = new Resource({
'resource-attribute': 'resource attribute value',
attributes: {
'resource-attribute': 'resource attribute value',
},
});
return {
resource: resource,
Expand Down
4 changes: 3 additions & 1 deletion experimental/packages/otlp-transformer/test/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ describe('Trace', () => {

beforeEach(() => {
resource = new Resource({
'resource-attribute': 'resource attribute value',
attributes: {
'resource-attribute': 'resource attribute value',
},
});
span = {
spanContext: () => ({
Expand Down
Loading
Loading