feat(sdk-node): add serviceInstanceIdDetector to NodeSDK (#4626)

* feat(sdk-node): add serviceInstanceIDDetector to NodeSDK Follow up from #4608 Adds the resource detector ServiceInstanceIDDetector on the NodeSDK constructor. It only gets added by default on any of those conditions: - the value `serviceinstance` is part of the list `OTEL_NODE_RESOURCE_DETECTORS` - `OTEL_NODE_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID` is set to `true` * remove OTEL_NODE_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID Signed-off-by: maryliag <[email protected]> * update readme on how to use `OTEL_NODE_RESOURCE_DETECTORS` * feedback from review * Update experimental/packages/opentelemetry-sdk-node/README.md Co-authored-by: Marc Pichler <[email protected]> * feedback from review --------- Signed-off-by: maryliag <[email protected]> Co-authored-by: Marc Pichler <[email protected]>
open-telemetry · Apr 17, 2024 · 73fddf9 · 73fddf9
1 parent 0d6c456
commit 73fddf9
Show file tree

Hide file tree

Showing 10 changed files with 121 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 * feat(sdk-trace-base): log resource attributes in ConsoleSpanExporter [#4605](https://github.com/open-telemetry/opentelemetry-js/pull/4605) @pichlermarc
 * feat(propagator-aws-xray): moved AWS Xray propagator from contrib [4603](https://github.com/open-telemetry/opentelemetry-js/pull/4603) @martinkuba
-* feat(resources): new experimental detector ServiceInstanceIdDetectorSync that sets the value for `service.instance.id` as random UUID.
+* feat(resources): new experimental detector ServiceInstanceIdDetectorSync that sets the value for `service.instance.id` as random UUID. [#4608](https://github.com/open-telemetry/opentelemetry-js/pull/4608) @maryliag
 
 ### :bug: (Bug Fix)
 

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -18,6 +18,11 @@ All notable changes to experimental packages in this project will be documented
 * refactor(instrumentation-grpc): move to use SEMATTRS [#4633](https://github.com/open-telemetry/opentelemetry-js/pull/4633)
 * feat(otlp-transformer): consolidate scope/resource creation in transformer [#4600](https://github.com/open-telemetry/opentelemetry-js/pull/4600)
 * feat(sdk-logs): print message when attributes are dropped due to attribute count limit [#4614](https://github.com/open-telemetry/opentelemetry-js/pull/4614) @HyunnoH
+* feat(sdk-node): add usage for the detector ServiceInstanceIdDetectorSync. [#4626](https://github.com/open-telemetry/opentelemetry-js/pull/4626) @maryliag
+  * The resource detector can be added to default resource detector list by adding the value `serviceinstance` to the list of resource detectors on the environment variable `OTEL_NODE_RESOURCE_DETECTORS`, e.g `OTEL_NODE_RESOURCE_DETECTORS=env,host,os,serviceinstance`
+  * The value can be overwritten by
+    * merging a resource containing the `service.instance.id` attribute
+    * using another resource detector which writes `service.instance.id`
 
 ### :bug: (Bug Fix)
 

diff --git a/experimental/packages/opentelemetry-sdk-node/README.md b/experimental/packages/opentelemetry-sdk-node/README.md
@@ -115,9 +115,26 @@ Configure a resource. Resources may also be detected by using the `autoDetectRes
 
 ### resourceDetectors
 
-Configure resource detectors. By default, the resource detectors are [envDetector, processDetector].
+Configure resource detectors. By default, the resource detectors are [envDetector, processDetector, hostDetector].
 NOTE: In order to enable the detection, the parameter `autoDetectResources` has to be `true`.
 
+If `resourceDetectors` was not set, you can also use the environment variable `OTEL_NODE_RESOURCE_DETECTORS` to enable only certain detectors, or completely disable them:
+
+- `env`
+- `host`
+- `os`
+- `process`
+- `serviceinstance` (experimental)
+- `all` - enable all resource detectors above
+  - **NOTE:** future versions of `@opentelemetry/sdk-node` may include additional detectors that will be covered by this scope.
+- `none` - disable resource detection
+
+For example, to enable only the `env`, `host` detectors:
+
+```shell
+export OTEL_NODE_RESOURCE_DETECTORS="env,host"
+```
+
 ### sampler
 
 Configure a custom sampler. By default, all traces will be sampled.

diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts
@@ -51,7 +51,10 @@ import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
 import { NodeSDKConfiguration } from './types';
 import { TracerProviderWithEnvExporters } from './TracerProviderWithEnvExporter';
 import { getEnv, getEnvWithoutDefaults } from '@opentelemetry/core';
-import { parseInstrumentationOptions } from './utils';
+import {
+  getResourceDetectorsFromEnv,
+  parseInstrumentationOptions,
+} from './utils';
 
 /** This class represents everything needed to register a fully configured OpenTelemetry Node.js SDK */
 
@@ -121,11 +124,15 @@ export class NodeSDK {
     this._configuration = configuration;
 
     this._resource = configuration.resource ?? new Resource({});
-    this._resourceDetectors = configuration.resourceDetectors ?? [
-      envDetector,
-      processDetector,
-      hostDetector,
-    ];
+    let defaultDetectors: (Detector | DetectorSync)[] = [];
+    if (process.env.OTEL_NODE_RESOURCE_DETECTORS != null) {
+      defaultDetectors = getResourceDetectorsFromEnv();
+    } else {
+      defaultDetectors = [envDetector, processDetector, hostDetector];
+    }
+
+    this._resourceDetectors =
+      configuration.resourceDetectors ?? defaultDetectors;
 
     this._serviceName = configuration.serviceName;
 
@@ -157,6 +164,7 @@ export class NodeSDK {
 
       const spanProcessor =
         configuration.spanProcessor ??
+        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
         new BatchSpanProcessor(configuration.traceExporter!);
 
       const spanProcessors = configuration.spanProcessors ?? [spanProcessor];

diff --git a/experimental/packages/opentelemetry-sdk-node/src/utils.ts b/experimental/packages/opentelemetry-sdk-node/src/utils.ts
@@ -14,10 +14,19 @@
  * limitations under the License.
  */
 
+import { diag } from '@opentelemetry/api';
 import {
   Instrumentation,
   InstrumentationOption,
 } from '@opentelemetry/instrumentation';
+import {
+  DetectorSync,
+  envDetectorSync,
+  hostDetectorSync,
+  osDetectorSync,
+  processDetectorSync,
+  serviceInstanceIdDetectorSync,
+} from '@opentelemetry/resources';
 
 // TODO: This part of a workaround to fix https://github.com/open-telemetry/opentelemetry-js/issues/3609
 // If the MeterProvider is not yet registered when instrumentations are registered, all metrics are dropped.
@@ -41,3 +50,41 @@ export function parseInstrumentationOptions(
 
   return instrumentations;
 }
+
+const RESOURCE_DETECTOR_ENVIRONMENT = 'env';
+const RESOURCE_DETECTOR_HOST = 'host';
+const RESOURCE_DETECTOR_OS = 'os';
+const RESOURCE_DETECTOR_PROCESS = 'process';
+const RESOURCE_DETECTOR_SERVICE_INSTANCE_ID = 'serviceinstance';
+
+export function getResourceDetectorsFromEnv(): Array<DetectorSync> {
+  // 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 resourceDetectorsFromEnv =
+    process.env.OTEL_NODE_RESOURCE_DETECTORS?.split(',') ?? ['all'];
+
+  if (resourceDetectorsFromEnv.includes('all')) {
+    return [...resourceDetectors.values()].flat();
+  }
+
+  if (resourceDetectorsFromEnv.includes('none')) {
+    return [];
+  }
+
+  return resourceDetectorsFromEnv.flatMap(detector => {
+    const resourceDetector = resourceDetectors.get(detector);
+    if (!resourceDetector) {
+      diag.error(
+        `Invalid resource detector "${detector}" specified in the environment variable OTEL_NODE_RESOURCE_DETECTORS`
+      );
+    }
+    return resourceDetector || [];
+  });
+}
diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
@@ -39,7 +39,10 @@ import {
   View,
 } from '@opentelemetry/sdk-metrics';
 import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
-import { assertServiceResource } from './util/resource-assertions';
+import {
+  assertServiceInstanceIdIsUUID,
+  assertServiceResource,
+} from './util/resource-assertions';
 import {
   ConsoleSpanExporter,
   SimpleSpanProcessor,
@@ -71,7 +74,6 @@ import {
 import {
   SEMRESATTRS_HOST_NAME,
   SEMRESATTRS_PROCESS_PID,
-  SEMRESATTRS_SERVICE_INSTANCE_ID,
 } from '@opentelemetry/semantic-conventions';
 
 const DefaultContextManager = semver.gte(process.version, '14.8.0')
@@ -682,7 +684,7 @@ describe('Node SDK', () => {
   describe('configureServiceInstanceId', async () => {
     it('should configure service instance id via OTEL_RESOURCE_ATTRIBUTES env var', async () => {
       process.env.OTEL_RESOURCE_ATTRIBUTES =
-        'service.instance.id=627cc493,service.name=my-service';
+        'service.instance.id=627cc493,service.name=my-service,service.namespace';
       const sdk = new NodeSDK();
 
       sdk.start();
@@ -694,7 +696,20 @@ describe('Node SDK', () => {
         instanceId: '627cc493',
       });
       delete process.env.OTEL_RESOURCE_ATTRIBUTES;
-      sdk.shutdown();
+      await sdk.shutdown();
+    });
+
+    it('should configure service instance id via OTEL_NODE_RESOURCE_DETECTORS env var', async () => {
+      process.env.OTEL_NODE_RESOURCE_DETECTORS = 'env,host,os,serviceinstance';
+      const sdk = new NodeSDK();
+
+      sdk.start();
+      const resource = sdk['_resource'];
+      await resource.waitForAsyncAttributes?.();
+
+      assertServiceInstanceIdIsUUID(resource);
+      delete process.env.OTEL_NODE_RESOURCE_DETECTORS;
+      await sdk.shutdown();
     });
 
     it('should configure service instance id with random UUID', async () => {
@@ -712,14 +727,7 @@ describe('Node SDK', () => {
       const resource = sdk['_resource'];
       await resource.waitForAsyncAttributes?.();
 
-      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(
-        UUID_REGEX.test(
-          resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID]?.toString() || ''
-        ),
-        true
-      );
+      assertServiceInstanceIdIsUUID(resource);
       await sdk.shutdown();
     });
   });

diff --git a/experimental/packages/opentelemetry-sdk-node/test/util/resource-assertions.ts b/experimental/packages/opentelemetry-sdk-node/test/util/resource-assertions.ts
@@ -18,6 +18,7 @@ import { SDK_INFO } from '@opentelemetry/core';
 import * as assert from 'assert';
 import { IResource, Resource } from '@opentelemetry/resources';
 import {
+  SEMRESATTRS_SERVICE_INSTANCE_ID,
   SEMRESATTRS_TELEMETRY_SDK_LANGUAGE,
   SEMRESATTRS_TELEMETRY_SDK_NAME,
   SEMRESATTRS_TELEMETRY_SDK_VERSION,
@@ -336,3 +337,14 @@ const assertHasOneLabel = (prefix: string, resource: Resource): void => {
       JSON.stringify(Object.keys(SemanticResourceAttributes))
   );
 };
+
+export const assertServiceInstanceIdIsUUID = (resource: Resource): 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(
+    UUID_REGEX.test(
+      resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID]?.toString() || ''
+    ),
+    true
+  );
+};
diff --git a/packages/opentelemetry-resources/README.md b/packages/opentelemetry-resources/README.md
@@ -16,11 +16,11 @@ npm install --save @opentelemetry/resources
 ## Usage
 
 ```typescript
-import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
+import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions';
 import { Resource } from '@opentelemetry/resources';
 
 const resource = new Resource({
-    [SemanticResourceAttributes.SERVICE_NAME]: 'api-service',
+    [SEMRESATTRS_SERVICE_NAME]: 'api-service',
 });
 
 const anotherResource = new Resource({

diff --git a/packages/opentelemetry-resources/src/platform/browser/index.ts b/packages/opentelemetry-resources/src/platform/browser/index.ts
@@ -16,8 +16,8 @@
 
 export * from './default-service-name';
 export * from './HostDetector';
-export * from './OSDetector';
 export * from './HostDetectorSync';
+export * from './OSDetector';
 export * from './OSDetectorSync';
 export * from './ProcessDetector';
 export * from './ProcessDetectorSync';

diff --git a/packages/opentelemetry-resources/src/platform/node/index.ts b/packages/opentelemetry-resources/src/platform/node/index.ts
@@ -16,8 +16,8 @@
 
 export * from './default-service-name';
 export * from './HostDetector';
-export * from './OSDetector';
 export * from './HostDetectorSync';
+export * from './OSDetector';
 export * from './OSDetectorSync';
 export * from './ProcessDetector';
 export * from './ProcessDetectorSync';