fix(configuration): percent-decode keys and values in resource.attributes_list

experimental/CHANGELOG.md

@@ -12,6 +12,10 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :bug: Bug Fixes
 
+* fix(configuration): percent-decode keys and values in `resource.attributes_list` per spec [#6787](https://github.com/open-telemetry/opentelemetry-js/pull/6787) @MikeGoldsmith
+* fix(sdk-node): apply spec-defined `schedule_delay: 1000` default for BatchLogRecordProcessor from declarative config (SDK defaults to 5000) [#6788](https://github.com/open-telemetry/opentelemetry-js/pull/6788) @MikeGoldsmith
+* fix(configuration): default `log_level` to `info` in env-based config initialization for consistency with file-based config [#6788](https://github.com/open-telemetry/opentelemetry-js/pull/6788) @MikeGoldsmith
+
 ### :books: Documentation
 
 ### :house: Internal
@@ -33,8 +37,6 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :bug: Bug Fixes
 
-* fix(sdk-node): apply spec-defined `schedule_delay: 1000` default for BatchLogRecordProcessor from declarative config (SDK defaults to 5000) [#6788](https://github.com/open-telemetry/opentelemetry-js/pull/6788) @MikeGoldsmith
-* fix(configuration): default `log_level` to `info` in env-based config initialization for consistency with file-based config [#6788](https://github.com/open-telemetry/opentelemetry-js/pull/6788) @MikeGoldsmith
 * fix(sdk-node): pass all config properties to log record exporters in declarative config [#6708](https://github.com/open-telemetry/opentelemetry-js/pull/6708) @MikeGoldsmith
 * fix(sdk-node): warn and ignore zero exporter timeout in declarative config [#6711](https://github.com/open-telemetry/opentelemetry-js/pull/6711) @MikeGoldsmith
 * fix(sdk-node): pass gRPC credentials and headers to span exporter in declarative config [#6705](https://github.com/open-telemetry/opentelemetry-js/pull/6705) @MikeGoldsmith

experimental/packages/configuration/src/FileConfigFactory.ts

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { diag } from '@opentelemetry/api';
 import { getStringFromEnv } from '@opentelemetry/core';
 import * as fs from 'fs';
 import * as yaml from 'yaml';
@@ -115,12 +116,53 @@ function applyOtlpHttpEncodingDefaults(data: ConfigurationModel): void {
 /**
  * Merge resource.attributes_list (comma-separated key=value pairs) into
  * resource.attributes, with entries already in attributes taking precedence.
+ *
+ * Per the spec, `,` and `=` in keys and values MUST be percent-encoded, and
+ * other characters MAY be percent-encoded. On any parse or decode error, the
+ * entire attributes_list is discarded and a warning is emitted.
+ * See https://opentelemetry.io/docs/specs/otel/resource/sdk/#specifying-resource-information-via-an-environment-variable
  */
 function mergeAttributesList(data: ConfigurationModel): void {
   const resource = data.resource;
   const list = resource?.attributes_list;
   if (typeof list !== 'string' || !list.trim()) return;
 
+  const decoded: Array<{ key: string; value: string }> = [];
+  for (const pair of list.split(',')) {
+    if (pair.trim() === '') continue;
+
+    // Per spec, `=` must be percent-encoded in keys/values, so a valid entry
+    // splits into exactly two parts.
+    const parts = pair.split('=');
+    if (parts.length !== 2) {
+      diag.warn(
+        `Invalid format for resource.attributes_list entry "${pair}": expected key=value with '=' percent-encoded in keys/values. Discarding all entries.`
+      );
+      return;
+    }
+
+    const rawKey = parts[0].trim();
+    const rawValue = parts[1].trim();
+    if (rawKey === '') {
+      diag.warn(
+        `Empty attribute key in resource.attributes_list entry "${pair}". Discarding all entries.`
+      );
+      return;
+    }
+
+    try {
+      decoded.push({
+        key: decodeURIComponent(rawKey),
+        value: decodeURIComponent(rawValue),
+      });
+    } catch (e) {
+      diag.warn(
+        `Failed to percent-decode resource.attributes_list entry "${pair}", discarding all entries: ${e}`
+      );
+      return;
+    }
+  }
+
   if (resource!.attributes == null) {
     resource!.attributes = [];
   }
@@ -129,14 +171,9 @@ function mergeAttributesList(data: ConfigurationModel): void {
     resource!.attributes.map((a: { name: string }) => a.name)
   );
 
-  for (const pair of list.split(',')) {
-    const eqIdx = pair.indexOf('=');
-    if (eqIdx > 0) {
-      const key = pair.slice(0, eqIdx).trim();
-      const value = pair.slice(eqIdx + 1).trim();
-      if (key && !existingKeys.has(key)) {
-        resource!.attributes.push({ name: key, value, type: 'string' });
-      }
+  for (const { key, value } of decoded) {
+    if (!existingKeys.has(key)) {
+      resource!.attributes.push({ name: key, value, type: 'string' });
     }
   }
 }

experimental/packages/configuration/test/FileConfigFactory.test.ts

@@ -5,6 +5,7 @@
 
 import * as assert from 'assert';
 import * as Sinon from 'sinon';
+import { diag } from '@opentelemetry/api';
 import type { ConfigurationModel } from '../src';
 import { createConfigFactory } from '../src/ConfigFactory';
 import { parseConfigFile } from '../src/FileConfigFactory';
@@ -1152,6 +1153,45 @@ describe('FileConfigFactory', function () {
     assert.deepStrictEqual(configFactory.getConfigModel(), expectedConfig);
   });
 
+  it('decodes percent-encoded keys and values in attributes_list', function () {
+    process.env.OTEL_CONFIG_FILE =
+      'test/fixtures/attributes-list-percent-encoded.yaml';
+    const configFactory = createConfigFactory();
+    const config = configFactory.getConfigModel();
+    assert.deepStrictEqual(config.resource?.attributes, [
+      { name: 'my,key', value: 'value=with=equals', type: 'string' },
+      { name: 'unicode', value: 'café', type: 'string' },
+    ]);
+  });
+
+  it('discards all entries when attributes_list has invalid percent-encoding', function () {
+    const warnStub = Sinon.stub(diag, 'warn');
+    process.env.OTEL_CONFIG_FILE =
+      'test/fixtures/attributes-list-invalid-encoding.yaml';
+    const configFactory = createConfigFactory();
+    const config = configFactory.getConfigModel();
+    assert.strictEqual(config.resource?.attributes, undefined);
+    assert.ok(
+      warnStub.args.some(args =>
+        String(args[0]).includes('Failed to percent-decode')
+      )
+    );
+    warnStub.restore();
+  });
+
+  it('discards all entries when attributes_list has unencoded `=` in value', function () {
+    const warnStub = Sinon.stub(diag, 'warn');
+    process.env.OTEL_CONFIG_FILE =
+      'test/fixtures/attributes-list-unencoded-equals.yaml';
+    const configFactory = createConfigFactory();
+    const config = configFactory.getConfigModel();
+    assert.strictEqual(config.resource?.attributes, undefined);
+    assert.ok(
+      warnStub.args.some(args => String(args[0]).includes('Invalid format'))
+    );
+    warnStub.restore();
+  });
+
   it('leaves attribute type undefined when omitted in YAML', function () {
     // The spec says "if omitted, string is used" for attribute type, but we intentionally
     // do NOT apply this default in the config parser. The consumer (SDK init code) is

experimental/packages/configuration/test/fixtures/attributes-list-invalid-encoding.yaml

@@ -0,0 +1,5 @@
+file_format: "1.0"
+resource:
+  # `%ZZ` is not a valid percent-encoded sequence — should cause the entire
+  # attributes_list to be discarded per spec.
+  attributes_list: "good=value,bad=%ZZ"

experimental/packages/configuration/test/fixtures/attributes-list-percent-encoded.yaml

@@ -0,0 +1,5 @@
+file_format: "1.0"
+resource:
+  # Values include percent-encoded `,` (%2C) and `=` (%3D) in keys and values,
+  # plus a non-ASCII value encoded as UTF-8 (%C3%A9 = é).
+  attributes_list: "my%2Ckey=value%3Dwith%3Dequals,unicode=caf%C3%A9"

experimental/packages/configuration/test/fixtures/attributes-list-unencoded-equals.yaml

@@ -0,0 +1,5 @@
+file_format: "1.0"
+resource:
+  # Unencoded `=` in value violates the spec format (must be %3D). Entire
+  # attributes_list should be discarded.
+  attributes_list: "key=val=ue,other=ok"