implement service instance id by default with env var

use an experimental environment variable to set the default value of `service.instance.id` as default
open-telemetry · Apr 9, 2024 · f6b4645 · f6b4645
1 parent e96cfed
commit f6b4645
Show file tree

Hide file tree

Showing 12 changed files with 68 additions and 97 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :rocket: (Enhancement)
 
-feat(sdk-node): set value for `service.instance.id` as random UUID as default. The value can still be overwritten by environment variable or cloud provider.
+feat(sdk-node): set value for `service.instance.id` as random UUID as default if environment variable `OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID` is set to true. The value can still be overwritten by the value of `service.instance.id` on the environment variable `OTEL_RESOURCE_ATTRIBUTES` or bt the cloud provider.
 
 ### :bug: (Bug Fix)
 

diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
@@ -797,8 +797,9 @@ describe('Node SDK', () => {
       sdk.shutdown();
     });
 
-    it('should configure service instance id with random UUID', async () => {
+    it('should configure service instance id with random UUID with OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID env var', async () => {
       process.env.OTEL_RESOURCE_ATTRIBUTES = 'service.name=my-service';
+      process.env.OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID = 'true';
       const sdk = new NodeSDK();
 
       sdk.start();
@@ -810,8 +811,44 @@ describe('Node SDK', () => {
         36
       );
       delete process.env.OTEL_RESOURCE_ATTRIBUTES;
+      delete process.env.OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID;
       await sdk.shutdown();
     });
+
+    it('should configure service instance id via OTEL_RESOURCE_ATTRIBUTES env var even with OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID env var', async () => {
+      process.env.OTEL_RESOURCE_ATTRIBUTES =
+        'service.instance.id=627cc493,service.name=my-service';
+      process.env.OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID = 'true';
+      const sdk = new NodeSDK();
+
+      sdk.start();
+      const resource = sdk['_resource'];
+      await resource.waitForAsyncAttributes?.();
+
+      assertServiceResource(resource, {
+        name: 'my-service',
+        instanceId: '627cc493',
+      });
+      delete process.env.OTEL_RESOURCE_ATTRIBUTES;
+      delete process.env.OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID;
+      sdk.shutdown();
+    });
+
+    it('should not configure service instance id with no value for it on OTEL_RESOURCE_ATTRIBUTES env var and  OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID env var as false', async () => {
+      process.env.OTEL_RESOURCE_ATTRIBUTES = 'service.name=my-service';
+      process.env.OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID = 'false';
+      const sdk = new NodeSDK();
+
+      sdk.start();
+      const resource = sdk['_resource'];
+      await resource.waitForAsyncAttributes?.();
+
+      assertServiceResource(resource, {
+        name: 'my-service',
+      });
+      delete process.env.OTEL_RESOURCE_ATTRIBUTES;
+      sdk.shutdown();
+    });
   });
 
   describe('A disabled SDK should be no-op', () => {

diff --git a/experimental/packages/sdk-logs/package.json b/experimental/packages/sdk-logs/package.json
@@ -102,7 +102,6 @@
   },
   "dependencies": {
     "@opentelemetry/core": "1.23.0",
-    "@opentelemetry/resources": "1.23.0",
-    "@opentelemetry/semantic-conventions": "1.23.0"
+    "@opentelemetry/resources": "1.23.0"
   }
 }
diff --git a/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts b/experimental/packages/sdk-logs/test/common/LoggerProvider.test.ts
@@ -24,7 +24,6 @@ import { loadDefaultConfig } from '../../src/config';
 import { DEFAULT_LOGGER_NAME } from './../../src/LoggerProvider';
 import { MultiLogRecordProcessor } from '../../src/MultiLogRecordProcessor';
 import { Logger } from '../../src/Logger';
-import { assertServiceResource } from './utils';
 
 describe('LoggerProvider', () => {
   let envSource: Record<string, any>;
@@ -61,14 +60,14 @@ describe('LoggerProvider', () => {
       it('should have default resource if not pass', () => {
         const provider = new LoggerProvider();
         const { resource } = provider['_sharedState'];
-        assertServiceResource(resource, Resource.default());
+        assert.deepStrictEqual(resource, Resource.default());
       });
 
       it('should fallback to default resource attrs', () => {
         const passedInResource = new Resource({ foo: 'bar' });
         const provider = new LoggerProvider({ resource: passedInResource });
         const { resource } = provider['_sharedState'];
-        assertServiceResource(
+        assert.deepStrictEqual(
           resource,
           Resource.default().merge(passedInResource)
         );

diff --git a/experimental/packages/sdk-logs/test/common/utils.ts b/experimental/packages/sdk-logs/test/common/utils.ts
@@ -14,10 +14,6 @@
  * limitations under the License.
  */
 
-import * as assert from 'assert';
-import { Resource } from '@opentelemetry/resources';
-import { SEMRESATTRS_SERVICE_INSTANCE_ID } from '@opentelemetry/semantic-conventions';
-
 export const validAttributes = {
   string: 'string',
   number: 0,
@@ -36,32 +32,3 @@ export const invalidAttributes = {
   // This empty length attribute should not be set
   '': 'empty-key',
 };
-
-/**
- * Compare two Service Resource values. Since the value for service.instance.id can be a randomUUID, the function checks if the size of the value matches the size of a randomUUID and removes it from the object, otherwise leave for comparison.
- * @param serviceResourceA
- * @param serviceResourceB
- */
-export const assertServiceResource = (
-  serviceResourceA: Resource,
-  serviceResourceB: Resource
-) => {
-  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;
-  const instanceIDAisUUID =
-    serviceResourceA.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] != null &&
-    UUID_REGEX.test(
-      serviceResourceA.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID].toString()
-    );
-  const instanceIDBisUUID =
-    serviceResourceB.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] != null &&
-    UUID_REGEX.test(
-      serviceResourceB.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID].toString()
-    );
-  if (instanceIDAisUUID && instanceIDBisUUID) {
-    delete serviceResourceA.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID];
-    delete serviceResourceB.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID];
-  }
-
-  assert.deepStrictEqual(serviceResourceA, serviceResourceB);
-};
diff --git a/experimental/packages/sdk-logs/tsconfig.esm.json b/experimental/packages/sdk-logs/tsconfig.esm.json
@@ -18,9 +18,6 @@
     {
       "path": "../../../packages/opentelemetry-resources"
     },
-    {
-      "path": "../../../packages/opentelemetry-semantic-conventions"
-    },
     {
       "path": "../api-logs"
     }

diff --git a/experimental/packages/sdk-logs/tsconfig.esnext.json b/experimental/packages/sdk-logs/tsconfig.esnext.json
@@ -18,9 +18,6 @@
     {
       "path": "../../../packages/opentelemetry-resources"
     },
-    {
-      "path": "../../../packages/opentelemetry-semantic-conventions"
-    },
     {
       "path": "../api-logs"
     }

diff --git a/experimental/packages/sdk-logs/tsconfig.json b/experimental/packages/sdk-logs/tsconfig.json
@@ -18,9 +18,6 @@
     {
       "path": "../../../packages/opentelemetry-resources"
     },
-    {
-      "path": "../../../packages/opentelemetry-semantic-conventions"
-    },
     {
       "path": "../api-logs"
     }

diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts
@@ -24,7 +24,10 @@ const DEFAULT_LIST_SEPARATOR = ',';
  * Environment interface to define all names
  */
 
-const ENVIRONMENT_BOOLEAN_KEYS = ['OTEL_SDK_DISABLED'] as const;
+const ENVIRONMENT_BOOLEAN_KEYS = [
+  'OTEL_SDK_DISABLED',
+  'OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID',
+] as const;
 
 type ENVIRONMENT_BOOLEANS = {
   [K in (typeof ENVIRONMENT_BOOLEAN_KEYS)[number]]?: boolean;
@@ -236,6 +239,7 @@ export const DEFAULT_ENVIRONMENT: Required<ENVIRONMENT> = {
   OTEL_EXPORTER_OTLP_METRICS_PROTOCOL: 'http/protobuf',
   OTEL_EXPORTER_OTLP_LOGS_PROTOCOL: 'http/protobuf',
   OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: 'cumulative',
+  OTEL_NODE_JS_EXPERIMENTAL_DEFAULT_SERVICE_INSTANCE_ID: false,
 };
 
 /**

diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts
@@ -23,11 +23,7 @@ import {
 import { Resource } from '@opentelemetry/resources';
 import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
-import {
-  SEMRESATTRS_SERVICE_INSTANCE_ID,
-  SEMRESATTRS_SERVICE_NAME,
-  SEMRESATTRS_TELEMETRY_SDK_LANGUAGE,
-} from '@opentelemetry/semantic-conventions';
+import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
 import {
   defaultStatusCodeTagName,
   defaultStatusErrorTagName,
@@ -39,15 +35,16 @@ import * as zipkinTypes from '../../src/types';
 const tracer = new BasicTracerProvider({
   resource: Resource.default().merge(
     new Resource({
-      [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
+      [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
       cost: '112.12',
       service: 'ui',
       version: '1',
     })
   ),
 }).getTracer('default');
 
-const language = tracer.resource.attributes[SEMRESATTRS_TELEMETRY_SDK_LANGUAGE];
+const language =
+  tracer.resource.attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE];
 
 const parentId = '5c1c63257de34c67';
 const spanContext: api.SpanContext = {
@@ -71,8 +68,6 @@ describe('transform', () => {
         key1: 'value1',
         key2: 'value2',
       });
-      span.resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] =
-        'faabc844-c78f-4d87-952d-cbb4ca4cbf6b';
       span.addEvent('my-event', { key3: 'value3' });
       span.end();
 
@@ -102,9 +97,7 @@ describe('transform', () => {
         tags: {
           key1: 'value1',
           key2: 'value2',
-          [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-          [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-            'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+          [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
           cost: '112.12',
           service: 'ui',
           version: '1',
@@ -124,8 +117,6 @@ describe('transform', () => {
         spanContext,
         api.SpanKind.SERVER
       );
-      span.resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] =
-        'faabc844-c78f-4d87-952d-cbb4ca4cbf6b';
       span.end();
 
       const zipkinSpan = toZipkinSpan(
@@ -147,9 +138,7 @@ describe('transform', () => {
         name: span.name,
         parentId: undefined,
         tags: {
-          [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-          [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-            'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+          [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
           cost: '112.12',
           service: 'ui',
           version: '1',
@@ -179,8 +168,6 @@ describe('transform', () => {
           spanContext,
           item.ot
         );
-        span.resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] =
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b';
         span.end();
 
         const zipkinSpan = toZipkinSpan(
@@ -202,9 +189,7 @@ describe('transform', () => {
           name: span.name,
           parentId: undefined,
           tags: {
-            [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-            [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-              'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+            [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
             cost: '112.12',
             service: 'ui',
             version: '1',
@@ -233,8 +218,6 @@ describe('transform', () => {
         key1: 'value1',
         key2: 'value2',
       });
-      span.resource.attributes[SEMRESATTRS_SERVICE_INSTANCE_ID] =
-        'faabc844-c78f-4d87-952d-cbb4ca4cbf6b';
       const tags: zipkinTypes.Tags = _toZipkinTags(
         span,
         defaultStatusCodeTagName,
@@ -244,9 +227,7 @@ describe('transform', () => {
       assert.deepStrictEqual(tags, {
         key1: 'value1',
         key2: 'value2',
-        [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+        [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
         'telemetry.sdk.language': language,
         'telemetry.sdk.name': 'opentelemetry',
         'telemetry.sdk.version': VERSION,
@@ -269,8 +250,6 @@ describe('transform', () => {
         {
           key1: 'value1',
           key2: 'value2',
-          [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-            'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
         }
       );
       const tags: zipkinTypes.Tags = _toZipkinTags(
@@ -282,9 +261,7 @@ describe('transform', () => {
       assert.deepStrictEqual(tags, {
         key1: 'value1',
         key2: 'value2',
-        [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+        [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
         'telemetry.sdk.language': language,
         'telemetry.sdk.name': 'opentelemetry',
         'telemetry.sdk.version': VERSION,
@@ -309,8 +286,6 @@ describe('transform', () => {
       span.setAttributes({
         key1: 'value1',
         key2: 'value2',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
       });
       const tags: zipkinTypes.Tags = _toZipkinTags(
         span,
@@ -322,9 +297,7 @@ describe('transform', () => {
         key1: 'value1',
         key2: 'value2',
         [defaultStatusCodeTagName]: 'ERROR',
-        [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+        [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
         'telemetry.sdk.language': language,
         'telemetry.sdk.name': 'opentelemetry',
         'telemetry.sdk.version': VERSION,
@@ -350,8 +323,6 @@ describe('transform', () => {
       span.setAttributes({
         key1: 'value1',
         key2: 'value2',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
       });
       const tags: zipkinTypes.Tags = _toZipkinTags(
         span,
@@ -364,9 +335,7 @@ describe('transform', () => {
         key2: 'value2',
         [defaultStatusCodeTagName]: 'ERROR',
         [defaultStatusErrorTagName]: status.message,
-        [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test',
-        [SEMRESATTRS_SERVICE_INSTANCE_ID]:
-          'faabc844-c78f-4d87-952d-cbb4ca4cbf6b',
+        [SemanticResourceAttributes.SERVICE_NAME]: 'zipkin-test',
         'telemetry.sdk.language': language,
         'telemetry.sdk.name': 'opentelemetry',
         'telemetry.sdk.version': VERSION,