fix(configuration): combine resource attributes and attributes list correctly (#6166)

maryliag · web-flow · commit 6cca46ecce0c · 2025-11-28T21:01:46.000Z
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -28,6 +28,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 * fix(instrumentation-grpc): attach correct name to diag message [#6097](https://github.com/open-telemetry/opentelemetry-js/pull/6043) @pichlermarc
 * fix(opentelemetry-sdk-node): default to otlp if OTEL_METRICS_EXPORTER is empty [#6092](https://github.com/open-telemetry/opentelemetry-js/pull/6092) @jeengbe
 * fix(configuration): merge service name from OTEL_SERVICE_NAME instead of replacing all resource attributes [#6162](https://github.com/open-telemetry/opentelemetry-js/pull/6162) @maryliag
+* fix(configuration): combine resource attributes and attributes list correctly [#6166](https://github.com/open-telemetry/opentelemetry-js/pull/6166) @maryliag
 
 ### :books: Documentation
 
diff --git a/experimental/packages/configuration/src/EnvironmentConfigFactory.ts b/experimental/packages/configuration/src/EnvironmentConfigFactory.ts
@@ -87,23 +87,8 @@ export function setResources(config: ConfigurationModel): void {
   const resourceAttrList = getStringFromEnv('OTEL_RESOURCE_ATTRIBUTES');
   const list = getStringListFromEnv('OTEL_RESOURCE_ATTRIBUTES');
   const serviceName = getStringFromEnv('OTEL_SERVICE_NAME');
-  if (list && list.length > 0) {
-    config.resource.attributes_list = resourceAttrList;
-    config.resource.attributes = [];
-    for (let i = 0; i < list.length; i++) {
-      const element = list[i].split('=');
-      config.resource.attributes.push({
-        name: element[0],
-        value:
-          serviceName && element[0] === 'service.name'
-            ? serviceName
-            : element[1],
-        type: 'string',
-      });
-    }
-  }
 
-  if (serviceName && config.resource.attributes == null) {
+  if (serviceName) {
     config.resource.attributes = [
       {
         name: 'service.name',
@@ -112,6 +97,26 @@ export function setResources(config: ConfigurationModel): void {
       },
     ];
   }
+  if (list && list.length > 0) {
+    config.resource.attributes_list = resourceAttrList;
+    if (config.resource.attributes == null) {
+      config.resource.attributes = [];
+    }
+
+    for (let i = 0; i < list.length; i++) {
+      const element = list[i].split('=');
+      if (
+        element[0] !== 'service.name' ||
+        (element[0] === 'service.name' && serviceName === undefined)
+      ) {
+        config.resource.attributes.push({
+          name: element[0],
+          value: element[1],
+          type: 'string',
+        });
+      }
+    }
+  }
 }
 
 export function setAttributeLimits(config: ConfigurationModel): void {
diff --git a/experimental/packages/configuration/src/FileConfigFactory.ts b/experimental/packages/configuration/src/FileConfigFactory.ts
@@ -121,31 +121,6 @@ export function parseConfigFile(config: ConfigurationModel) {
       if (config.resource == null) {
         config.resource = {};
       }
-      const attrList = getStringFromConfigFile(
-        parsedContent['resource']?.['attributes_list']
-      );
-      if (attrList) {
-        config.resource.attributes_list = attrList;
-        const list = getStringListFromConfigFile(
-          parsedContent['resource']?.['attributes_list']
-        );
-        if (
-          list &&
-          list.length > 0 &&
-          parsedContent['resource']?.['attributes'] == null
-        ) {
-          config.resource.attributes = [];
-          for (let i = 0; i < list.length; i++) {
-            const element = list[i].split('=');
-            config.resource.attributes.push({
-              name: element[0],
-              value: element[1],
-              type: 'string',
-            });
-          }
-        }
-      }
-
       const schemaUrl = getStringFromConfigFile(
         parsedContent['resource']?.['schema_url']
       );
@@ -154,7 +129,11 @@ export function parseConfigFile(config: ConfigurationModel) {
       }
     }
 
-    setResourceAttributes(config, parsedContent['resource']?.['attributes']);
+    setResourceAttributes(
+      config,
+      parsedContent['resource']?.['attributes'],
+      parsedContent['resource']?.['attributes_list']
+    );
     setAttributeLimits(config, parsedContent['attribute_limits']);
     setPropagator(config, parsedContent['propagator']);
     setTracerProvider(config, parsedContent['tracer_provider']);
@@ -169,44 +148,71 @@ export function parseConfigFile(config: ConfigurationModel) {
 
 export function setResourceAttributes(
   config: ConfigurationModel,
-  attributes: AttributeNameValue[]
+  attributes: AttributeNameValue[],
+  attributeList: string
 ) {
-  if (attributes) {
+  if (attributes || attributeList) {
+    const addedKeys = [];
     if (config.resource == null) {
       config.resource = {};
     }
-    config.resource.attributes = [];
-    for (let i = 0; i < attributes.length; i++) {
-      const att = attributes[i];
-      let value = att['value'];
-      switch (att['type']) {
-        case 'bool':
-          value = getBooleanFromConfigFile(value);
-          break;
-        case 'bool_array':
-          value = getBooleanListFromConfigFile(value);
-          break;
-        case 'int':
-        case 'double':
-          value = getNumberFromConfigFile(value);
-          break;
-        case 'int_array':
-        case 'double_array':
-          value = getNumberListFromConfigFile(value);
-          break;
-        case 'string_array':
-          value = getStringListFromConfigFile(value);
-          break;
-        default:
-          value = getStringFromConfigFile(value);
-          break;
+    if (getStringFromConfigFile(attributeList)) {
+      config.resource.attributes_list = getStringFromConfigFile(attributeList);
+    }
+
+    const list = getStringListFromConfigFile(attributeList);
+    if ((list && list.length > 0) || (attributes && attributes.length > 0)) {
+      config.resource.attributes = [];
+
+      if (attributes) {
+        for (let i = 0; i < attributes.length; i++) {
+          const att = attributes[i];
+          let value = att['value'];
+          switch (att['type']) {
+            case 'bool':
+              value = getBooleanFromConfigFile(value);
+              break;
+            case 'bool_array':
+              value = getBooleanListFromConfigFile(value);
+              break;
+            case 'int':
+            case 'double':
+              value = getNumberFromConfigFile(value);
+              break;
+            case 'int_array':
+            case 'double_array':
+              value = getNumberListFromConfigFile(value);
+              break;
+            case 'string_array':
+              value = getStringListFromConfigFile(value);
+              break;
+            default:
+              value = getStringFromConfigFile(value);
+              break;
+          }
+
+          const key = getStringFromConfigFile(att['name']) ?? '';
+          config.resource.attributes.push({
+            name: key,
+            value: value,
+            type: att['type'] ?? 'string',
+          });
+          addedKeys.push(key);
+        }
       }
 
-      config.resource.attributes.push({
-        name: getStringFromConfigFile(att['name']) ?? '',
-        value: value,
-        type: att['type'] ?? 'string',
-      });
+      if (list) {
+        for (let i = 0; i < list.length; i++) {
+          const element = list[i].split('=');
+          if (!addedKeys.includes(element[0])) {
+            config.resource.attributes.push({
+              name: element[0],
+              value: element[1],
+              type: 'string',
+            });
+          }
+        }
+      }
     }
   }
 }
diff --git a/experimental/packages/configuration/test/ConfigFactory.test.ts b/experimental/packages/configuration/test/ConfigFactory.test.ts
@@ -129,6 +129,16 @@ const configFromFile: ConfigurationModel = {
         value: [1.1, 2.2],
         type: 'double_array',
       },
+      {
+        name: 'service.namespace',
+        type: 'string',
+        value: 'my-namespace',
+      },
+      {
+        name: 'service.version',
+        type: 'string',
+        value: '1.0.0',
+      },
     ],
   },
   attribute_limits: {
@@ -836,6 +846,32 @@ describe('ConfigFactory', function () {
       assert.deepStrictEqual(configFactory.getConfigModel(), expectedConfig);
     });
 
+    it('should configure service name via OTEL_SERVICE_NAME env var', function () {
+      process.env.OTEL_SERVICE_NAME = 'name-from-service-name';
+      process.env.OTEL_RESOURCE_ATTRIBUTES =
+        'service.instance.id=my-instance-id';
+      const expectedConfig: ConfigurationModel = {
+        ...defaultConfig,
+        resource: {
+          attributes: [
+            {
+              name: 'service.name',
+              value: 'name-from-service-name',
+              type: 'string',
+            },
+            {
+              name: 'service.instance.id',
+              value: 'my-instance-id',
+              type: 'string',
+            },
+          ],
+          attributes_list: 'service.instance.id=my-instance-id',
+        },
+      };
+      const configFactory = createConfigFactory();
+      assert.deepStrictEqual(configFactory.getConfigModel(), expectedConfig);
+    });
+
     it('should return config with custom attribute_limits', function () {
       process.env.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '100';
       process.env.OTEL_ATTRIBUTE_COUNT_LIMIT = '200';
@@ -1892,7 +1928,7 @@ describe('ConfigFactory', function () {
       process.env.OTEL_SDK_DISABLED = 'false';
       process.env.OTEL_LOG_LEVEL = 'debug';
       process.env.OTEL_SERVICE_NAME = 'custom-name';
-      process.env.OTEL_RESOURCE_ATTRIBUTES = 'attributes';
+      process.env.OTEL_RESOURCE_ATTRIBUTES = 'att=1';
       process.env.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '23';
       process.env.OTEL_ATTRIBUTE_COUNT_LIMIT = '7';
       process.env.OTEL_PROPAGATORS = 'prop';
@@ -1950,13 +1986,18 @@ describe('ConfigFactory', function () {
       const expectedConfig: ConfigurationModel = {
         ...defaultConfigFromFileWithEnvVariables,
         resource: {
-          attributes_list: 'attributes',
+          attributes_list: 'att=1',
           attributes: [
             {
               name: 'service.name',
               value: 'custom-name',
               type: 'string',
             },
+            {
+              name: 'att',
+              value: '1',
+              type: 'string',
+            },
           ],
         },
         attribute_limits: {
@@ -2101,6 +2142,78 @@ describe('ConfigFactory', function () {
       );
     });
 
+    it('check resources priority', function () {
+      process.env.OTEL_EXPERIMENTAL_CONFIG_FILE =
+        'test/fixtures/resources.yaml';
+      const configFactory = createConfigFactory();
+      const expectedConfig: ConfigurationModel = {
+        ...defaultConfig,
+        resource: {
+          schema_url: 'https://opentelemetry.io/schemas/1.16.0',
+          attributes_list:
+            'service.namespace=config-namespace,service.version=1.0.0',
+          attributes: [
+            {
+              name: 'service.name',
+              value: 'config-name',
+              type: 'string',
+            },
+            {
+              name: 'service.namespace',
+              value: 'priority-config-namespace',
+              type: 'string',
+            },
+            {
+              name: 'string_key',
+              value: 'value',
+              type: 'string',
+            },
+            {
+              name: 'bool_key',
+              value: true,
+              type: 'bool',
+            },
+            {
+              name: 'int_key',
+              value: 1,
+              type: 'int',
+            },
+            {
+              name: 'double_key',
+              value: 1.1,
+              type: 'double',
+            },
+            {
+              name: 'string_array_key',
+              value: ['value1', 'value2'],
+              type: 'string_array',
+            },
+            {
+              name: 'bool_array_key',
+              value: [true, false],
+              type: 'bool_array',
+            },
+            {
+              name: 'int_array_key',
+              value: [1, 2],
+              type: 'int_array',
+            },
+            {
+              name: 'double_array_key',
+              value: [1.1, 2.2],
+              type: 'double_array',
+            },
+            {
+              name: 'service.version',
+              value: '1.0.0',
+              type: 'string',
+            },
+          ],
+        },
+      };
+      assert.deepStrictEqual(configFactory.getConfigModel(), expectedConfig);
+    });
+
     it('checks to keep good code coverage', function () {
       process.env.OTEL_EXPERIMENTAL_CONFIG_FILE =
         'test/fixtures/test-for-coverage.yaml';
@@ -2110,8 +2223,8 @@ describe('ConfigFactory', function () {
       assert.deepStrictEqual(config, { resource: {} });
 
       config = {};
-      setResourceAttributes(config, []);
-      assert.deepStrictEqual(config, { resource: { attributes: [] } });
+      setResourceAttributes(config, [], '');
+      assert.deepStrictEqual(config, { resource: {} });
 
       config = {};
       setFileAttributeLimits(config, { attribute_count_limit: 128 });
diff --git a/experimental/packages/configuration/test/fixtures/resources.yaml b/experimental/packages/configuration/test/fixtures/resources.yaml
