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

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
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