Merge branch 'main' into dluna-ci-pr-speed-and-coverage

Author: david-luna

.github/component_owners.yml

@@ -11,7 +11,9 @@ components:
     - abhee11
   packages/resource-detector-gcp:
     - aabmass
-    - punya
+    - psx95
+    - DylanRussell
+    - dashpole
   packages/resource-detector-github: []
     # Unmaintained
   packages/resource-detector-instana:

packages/resource-detector-aws/src/detectors/AwsEcsDetector.ts

@@ -63,6 +63,7 @@ interface AwsLogOptions {
  */
 export class AwsEcsDetector implements ResourceDetector {
   static readonly CONTAINER_ID_LENGTH = 64;
+  static readonly CONTAINER_ID_LENGTH_MIN = 32;
   static readonly DEFAULT_CGROUP_PATH = '/proc/self/cgroup';
 
   private static readFileAsync = util.promisify(fs.readFile);
@@ -155,25 +156,76 @@ export class AwsEcsDetector implements ResourceDetector {
    * and then return undefined.
    */
   private async _getContainerId(): Promise<string | undefined> {
-    let containerId = undefined;
     try {
       const rawData = await AwsEcsDetector.readFileAsync(
         AwsEcsDetector.DEFAULT_CGROUP_PATH,
         'utf8'
       );
-      const splitData = rawData.trim().split('\n');
-      for (const str of splitData) {
-        if (str.length > AwsEcsDetector.CONTAINER_ID_LENGTH) {
-          containerId = str.substring(
-            str.length - AwsEcsDetector.CONTAINER_ID_LENGTH
-          );
-          break;
-        }
+      const lines = rawData
+        .split('\n')
+        .map(s => s.trim())
+        .filter(Boolean);
+
+      // Pass 1: Prefer primary ECS pattern across all lines
+      for (const line of lines) {
+        const id = this._extractPrimaryEcsContainerId(line);
+        if (id) return id;
+      }
+
+      // Pass 2: Fallback to last-segment with strict allowed chars (hex + hyphen)
+      for (const line of lines) {
+        const id = this._extractLastSegmentContainerId(line);
+        if (id) return id;
+      }
+
+      // Pass 3: Legacy fallback to last 64 chars (Docker-style), keep existing behavior
+      for (const line of lines) {
+        const id = this._extractLegacyContainerId(line);
+        if (id) return id;
       }
     } catch (e) {
       diag.debug('AwsEcsDetector failed to read container ID', e);
     }
-    return containerId;
+    return undefined;
+  }
+
+  // Prefer primary ECS format extraction
+  private _extractPrimaryEcsContainerId(line: string): string | undefined {
+    const ecsPattern = /\/ecs\/[a-fA-F0-9-]+\/([a-fA-F0-9-]+)$/;
+    const match = line.match(ecsPattern);
+    if (
+      match &&
+      match[1] &&
+      match[1].length >= AwsEcsDetector.CONTAINER_ID_LENGTH_MIN &&
+      match[1].length <= AwsEcsDetector.CONTAINER_ID_LENGTH
+    ) {
+      return match[1];
+    }
+    return undefined;
+  }
+
+  // Fallback: accept last path segment if it looks like a container id (hex + '-')
+  private _extractLastSegmentContainerId(line: string): string | undefined {
+    const parts = line.split('/');
+    if (parts.length <= 1) return undefined;
+    const last = parts[parts.length - 1];
+    if (
+      last &&
+      last.length >= AwsEcsDetector.CONTAINER_ID_LENGTH_MIN &&
+      last.length <= AwsEcsDetector.CONTAINER_ID_LENGTH &&
+      /^[a-fA-F0-9-]+$/.test(last)
+    ) {
+      return last;
+    }
+    return undefined;
+  }
+
+  // Legacy fallback: keep existing behavior to avoid breaking users/tests
+  private _extractLegacyContainerId(line: string): string | undefined {
+    if (line.length > AwsEcsDetector.CONTAINER_ID_LENGTH) {
+      return line.substring(line.length - AwsEcsDetector.CONTAINER_ID_LENGTH);
+    }
+    return undefined;
   }
 
   /**

packages/resource-detector-aws/test/detectors/AwsEcsDetector.test.ts

@@ -452,4 +452,141 @@ describe('AwsEcsResourceDetector', () => {
       });
     });
   });
+
+  describe('Container ID extraction', () => {
+    const testMetadataUri = 'http://169.254.170.2/v4/test';
+    const testHostname = 'test-hostname';
+    let readStub: sinon.SinonStub;
+
+    beforeEach(() => {
+      process.env.ECS_CONTAINER_METADATA_URI_V4 = testMetadataUri;
+    });
+
+    afterEach(() => {
+      sinon.restore();
+    });
+
+    function setupMocks(cgroupData: string) {
+      sinon.stub(os, 'hostname').returns(testHostname);
+      readStub = sinon
+        .stub(AwsEcsDetector, 'readFileAsync' as any)
+        .resolves(cgroupData);
+    }
+
+    function setupMetadataNock() {
+      return nock('http://169.254.170.2:80')
+        .persist(false)
+        .get('/v4/test')
+        .reply(200, {
+          ContainerARN: 'arn:aws:ecs:us-west-2:111122223333:container/test',
+        })
+        .get('/v4/test/task')
+        .reply(200, {
+          TaskARN: 'arn:aws:ecs:us-west-2:111122223333:task/default/test',
+          Family: 'test-family',
+          Revision: '1',
+          Cluster: 'test-cluster',
+          LaunchType: 'FARGATE',
+        });
+    }
+
+    it('should extract full container ID from new ECS Fargate format', async () => {
+      const taskId = 'c23e5f76c09d438aa1824ca4058bdcab';
+      const containerId = '1234567890abcdef';
+      const cgroupData = `/ecs/${taskId}/${taskId}-${containerId}`;
+
+      setupMocks(cgroupData);
+      const nockScope = setupMetadataNock();
+
+      const resource = detectResources({ detectors: [awsEcsDetector] });
+      await resource.waitForAsyncAttributes?.();
+
+      sinon.assert.calledOnce(readStub);
+      assert.ok(resource);
+      assertEcsResource(resource, {});
+      assertContainerResource(resource, {
+        name: testHostname,
+        id: `${taskId}-${containerId}`,
+      });
+
+      nockScope.done();
+    });
+
+    it('should handle backward compatibility with legacy format', async () => {
+      const legacyContainerId =
+        'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklm';
+
+      setupMocks(legacyContainerId);
+      const nockScope = setupMetadataNock();
+
+      const resource = detectResources({ detectors: [awsEcsDetector] });
+      await resource.waitForAsyncAttributes?.();
+
+      sinon.assert.calledOnce(readStub);
+      assert.ok(resource);
+      assertEcsResource(resource, {});
+      assertContainerResource(resource, {
+        name: testHostname,
+        id: 'bcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklm',
+      });
+
+      nockScope.done();
+    });
+
+    it('should extract container ID from Docker format cgroup', async () => {
+      const dockerContainerId =
+        'a4d00c9dd675d67f866c786181419e1b44832d4696780152e61afd44a3e02856';
+      const cgroupData = `1:blkio:/docker/${dockerContainerId}
+2:cpu:/docker/${dockerContainerId}
+3:cpuacct:/docker/${dockerContainerId}`;
+
+      setupMocks(cgroupData);
+      const nockScope = setupMetadataNock();
+
+      const resource = detectResources({ detectors: [awsEcsDetector] });
+      await resource.waitForAsyncAttributes?.();
+
+      sinon.assert.calledOnce(readStub);
+      assert.ok(resource);
+      assertEcsResource(resource, {});
+      assertContainerResource(resource, {
+        name: testHostname,
+        id: dockerContainerId,
+      });
+
+      nockScope.done();
+    });
+
+    it('should extract container ID from mixed ECS and Docker format cgroup', async () => {
+      const taskId = '447438d8540d49dca93b4f0a488ebe90';
+      const containerId = `${taskId}-1364044452`;
+      const cgroupData = `11:memory:/ecs/${taskId}/${containerId}
+10:devices:/ecs/${taskId}/${containerId}
+9:freezer:/ecs/${taskId}/${containerId}
+8:blkio:/ecs/${taskId}/${containerId}
+7:perf_event:/ecs/${taskId}/${containerId}
+6:net_cls,net_prio:/ecs/${taskId}/${containerId}
+5:cpuset:/ecs/${taskId}/${containerId}
+4:pids:/ecs/${taskId}/${containerId}
+3:hugetlb:/ecs/${taskId}/${containerId}
+2:cpu,cpuacct:/ecs/${taskId}/${containerId}
+1:name=systemd:/ecs/${taskId}/${containerId}`;
+
+      setupMocks(cgroupData);
+      const nockScope = setupMetadataNock();
+
+      const resource = detectResources({ detectors: [awsEcsDetector] });
+      await resource.waitForAsyncAttributes?.();
+
+      sinon.assert.calledOnce(readStub);
+      assert.ok(resource);
+      assertEcsResource(resource, {});
+      assertContainerResource(resource, {
+        name: testHostname,
+        id: containerId,
+      });
+
+      nockScope.done();
+    });
+  });
 });

packages/resource-detector-gcp/test/detectors/GcpDetectorIntegration.test.ts

@@ -14,64 +14,32 @@
  * limitations under the License.
  */
 
-// NOTE: in other test the modules have already been required and cached (`GcpDetector.test.ts`)
-// so SDK cannot instrument them. We make sure we load fresh copies so RITM hook does its work.
-// we remove all the require chain: detector -> gcp-metadata -> gaxios -> node-fetch
-// but kepp other cache entries to not slow down the test more than necessary
-const modules = [
-  'opentelemetry-resource-detector-gcp',
-  'gcp-metadata',
-  'gaxios',
-  'node-fetch',
-];
-Object.keys(require.cache)
-  .filter(path => modules.some(m => path.includes(m)))
-  .forEach(key => {
-    delete require.cache[key];
-  });
-
-import * as assert from 'assert';
-
-import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
 import {
-  InMemorySpanExporter,
-  SimpleSpanProcessor,
-} from '@opentelemetry/sdk-trace-base';
-import { NodeSDK } from '@opentelemetry/sdk-node';
-import { detectResources } from '@opentelemetry/resources';
+  runTestFixture,
+  TestCollector,
+} from '@opentelemetry/contrib-test-utils';
+import * as assert from 'assert';
 
-describe('[Integration] GcpDetector', () => {
+describe('[Integration] GcpDetector', async () => {
   it('should not start spans for detector requests', async () => {
-    const memoryExporter = new InMemorySpanExporter();
-    const sdk = new NodeSDK({
-      instrumentations: [new HttpInstrumentation()],
-      spanProcessors: [new SimpleSpanProcessor(memoryExporter)],
+    await runTestFixture({
+      cwd: __dirname,
+      argv: ['fixtures/detect-with-http-instrumentation.mjs'],
+      env: {
+        NODE_OPTIONS:
+          '--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
+        METADATA_SERVER_DETECTION: 'assume-present',
+      },
+      checkResult: (err, stdout, stderr) => {
+        assert.ifError(err);
+      },
+      checkCollector: (collector: TestCollector) => {
+        assert.equal(
+          collector.spans.length,
+          0,
+          'no spans exported for GcpDetector'
+        );
+      },
     });
-
-    sdk.start();
-
-    process.env.METADATA_SERVER_DETECTION = 'assume-present';
-
-    // NOTE: detectors implementing the `DetectorSync` interface and starting
-    // HTTP requests within the `detect` method will produce Noop Spans since
-    // the SDK resolves the trace provider after resource detectors are triggered.
-    // Ref: https://github.com/open-telemetry/opentelemetry-js/blob/38f6689480d28dcbdafcb7b5ba4b14025328ffda/experimental/packages/opentelemetry-sdk-node/src/sdk.ts#L210-L240
-    //
-    // So having the detector in the config would result in no spans for Azure requests
-    // being exported which is what we want. Although we may think we're safe of sending
-    // internal tracing any change that delays these request will result in internal
-    // tracing being exported. We do the detection outside the SDK constructor to have such
-    // scenario.
-    const { gcpDetector } = require('../../build/src/detectors/GcpDetector');
-    const resource = detectResources({ detectors: [gcpDetector] });
-    await resource.waitForAsyncAttributes?.();
-
-    // Wait for the next loop to let the span close properly
-    await new Promise(r => setTimeout(r, 0));
-    const spans = memoryExporter.getFinishedSpans();
-
-    assert.equal(spans.length, 0, 'no spans exported for GcpDetector');
-
-    await sdk.shutdown();
   }).timeout(15000);
 });

packages/resource-detector-gcp/test/detectors/fixtures/detect-with-http-instrumentation.mjs

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
+import { NodeSDK } from '@opentelemetry/sdk-node';
+import { detectResources } from '@opentelemetry/resources';
+
+const sdk = new NodeSDK({
+  instrumentations: [new HttpInstrumentation()],
+});
+sdk.start();
+
+// Defer import until after instrumentation is added
+const { gcpDetector } = await import(
+  '../../../build/src/detectors/GcpDetector.js'
+);
+const resource = detectResources({ detectors: [gcpDetector] });
+await resource.waitForAsyncAttributes?.();
+
+console.log(resource);
+
+await sdk.shutdown();