refactor(detector-aws): change implementation to DetectorSync interfa…

…ce for ECS detector
david-luna · Jul 12, 2024 · fa218b0 · fa218b0
1 parent 5f2c160
commit fa218b0
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 43 deletions.
diff --git a/detectors/node/opentelemetry-resource-detector-aws/src/detectors/AwsEcsDetector.ts b/detectors/node/opentelemetry-resource-detector-aws/src/detectors/AwsEcsDetector.ts
@@ -16,7 +16,8 @@
 
 import { diag } from '@opentelemetry/api';
 import {
-  Detector,
+  DetectorSync,
+  IResource,
   Resource,
   ResourceAttributes,
 } from '@opentelemetry/resources';
@@ -62,43 +63,51 @@ interface AwsLogOptions {
  * ECS and return a {@link Resource} populated with data about the ECS
  * plugins of AWS X-Ray. Returns an empty Resource if detection fails.
  */
-export class AwsEcsDetector implements Detector {
+export class AwsEcsDetector implements DetectorSync {
   static readonly CONTAINER_ID_LENGTH = 64;
   static readonly DEFAULT_CGROUP_PATH = '/proc/self/cgroup';
 
   private static readFileAsync = util.promisify(fs.readFile);
 
-  async detect(): Promise<Resource> {
+  detect(): IResource {
+    return new Resource({}, this._getAttributes());
+  }
+
+  private async _getAttributes(): Promise<ResourceAttributes> {
     const env = getEnv();
     if (!env.ECS_CONTAINER_METADATA_URI_V4 && !env.ECS_CONTAINER_METADATA_URI) {
       diag.debug('AwsEcsDetector failed: Process is not on ECS');
-      return Resource.empty();
+      return {};
     }
 
-    let resource = new Resource({
-      [SEMRESATTRS_CLOUD_PROVIDER]: CLOUDPROVIDERVALUES_AWS,
-      [SEMRESATTRS_CLOUD_PLATFORM]: CLOUDPLATFORMVALUES_AWS_ECS,
-    }).merge(await AwsEcsDetector._getContainerIdAndHostnameResource());
-
-    const metadataUrl = getEnv().ECS_CONTAINER_METADATA_URI_V4;
-    if (metadataUrl) {
-      const [containerMetadata, taskMetadata] = await Promise.all([
-        AwsEcsDetector._getUrlAsJson(metadataUrl),
-        AwsEcsDetector._getUrlAsJson(`${metadataUrl}/task`),
-      ]);
-
-      const metadatav4Resource = await AwsEcsDetector._getMetadataV4Resource(
-        containerMetadata,
-        taskMetadata
-      );
-      const logsResource = await AwsEcsDetector._getLogResource(
-        containerMetadata
-      );
-
-      resource = resource.merge(metadatav4Resource).merge(logsResource);
+    try {
+      let resource = new Resource({
+        [SEMRESATTRS_CLOUD_PROVIDER]: CLOUDPROVIDERVALUES_AWS,
+        [SEMRESATTRS_CLOUD_PLATFORM]: CLOUDPLATFORMVALUES_AWS_ECS,
+      }).merge(await AwsEcsDetector._getContainerIdAndHostnameResource());
+
+      const metadataUrl = getEnv().ECS_CONTAINER_METADATA_URI_V4;
+      if (metadataUrl) {
+        const [containerMetadata, taskMetadata] = await Promise.all([
+          AwsEcsDetector._getUrlAsJson(metadataUrl),
+          AwsEcsDetector._getUrlAsJson(`${metadataUrl}/task`),
+        ]);
+
+        const metadatav4Resource = await AwsEcsDetector._getMetadataV4Resource(
+          containerMetadata,
+          taskMetadata
+        );
+        const logsResource = await AwsEcsDetector._getLogResource(
+          containerMetadata
+        );
+
+        resource = resource.merge(metadatav4Resource).merge(logsResource);
+      }
+
+      return resource.attributes;
+    } catch {
+      return {};
     }
-
-    return resource;
   }
 
   /**

diff --git a/detectors/node/opentelemetry-resource-detector-aws/test/detectors/AwsEcsDetector.test.ts b/detectors/node/opentelemetry-resource-detector-aws/test/detectors/AwsEcsDetector.test.ts
@@ -165,7 +165,8 @@ describe('AwsEcsResourceDetector', () => {
       .stub(AwsEcsDetector, 'readFileAsync' as any)
       .resolves(noisyCgroupData);
 
-    const resource = await awsEcsDetector.detect();
+    const resource = awsEcsDetector.detect();
+    await resource.waitForAsyncAttributes?.();
 
     sinon.assert.calledOnce(readStub);
     assert.ok(resource);
@@ -183,7 +184,8 @@ describe('AwsEcsResourceDetector', () => {
       .stub(AwsEcsDetector, 'readFileAsync' as any)
       .resolves(multiValidCgroupData);
 
-    const resource = await awsEcsDetector.detect();
+    const resource = awsEcsDetector.detect();
+    await resource.waitForAsyncAttributes?.();
 
     sinon.assert.calledOnce(readStub);
     assert.ok(resource);
@@ -200,7 +202,8 @@ describe('AwsEcsResourceDetector', () => {
       .stub(AwsEcsDetector, 'readFileAsync' as any)
       .resolves(correctCgroupData);
 
-    const resource = await awsEcsDetector.detect();
+    const resource = awsEcsDetector.detect();
+    await resource.waitForAsyncAttributes?.();
 
     sinon.assert.notCalled(readStub);
     assert.ok(resource);
@@ -259,7 +262,8 @@ describe('AwsEcsResourceDetector', () => {
             .stub(AwsEcsDetector, 'readFileAsync' as any)
             .resolves(correctCgroupData);
 
-          const resource = await awsEcsDetector.detect();
+          const resource = awsEcsDetector.detect();
+          await resource.waitForAsyncAttributes?.();
 
           sinon.assert.calledOnce(readStub);
           assert.ok(resource);
@@ -276,7 +280,8 @@ describe('AwsEcsResourceDetector', () => {
             .stub(AwsEcsDetector, 'readFileAsync' as any)
             .rejects(errorMsg.fileNotFoundError);
 
-          const resource = await awsEcsDetector.detect();
+          const resource = awsEcsDetector.detect();
+          await resource.waitForAsyncAttributes?.();
 
           sinon.assert.calledOnce(readStub);
           assert.ok(resource);
@@ -292,7 +297,8 @@ describe('AwsEcsResourceDetector', () => {
             .stub(AwsEcsDetector, 'readFileAsync' as any)
             .resolves('');
 
-          const resource = await awsEcsDetector.detect();
+          const resource = awsEcsDetector.detect();
+          await resource.waitForAsyncAttributes?.();
 
           sinon.assert.calledOnce(readStub);
           assert.ok(resource);
@@ -308,7 +314,8 @@ describe('AwsEcsResourceDetector', () => {
             .stub(AwsEcsDetector, 'readFileAsync' as any)
             .resolves(correctCgroupData);
 
-          const resource = await awsEcsDetector.detect();
+          const resource = awsEcsDetector.detect();
+          await resource.waitForAsyncAttributes?.();
 
           sinon.assert.calledOnce(readStub);
           assert.ok(resource);
@@ -324,7 +331,8 @@ describe('AwsEcsResourceDetector', () => {
             .stub(AwsEcsDetector, 'readFileAsync' as any)
             .rejects(errorMsg.fileNotFoundError);
 
-          const resource = await awsEcsDetector.detect();
+          const resource = awsEcsDetector.detect();
+          await resource.waitForAsyncAttributes?.();
 
           sinon.assert.calledOnce(readStub);
           assert.ok(resource);
@@ -409,13 +417,11 @@ describe('AwsEcsResourceDetector', () => {
         sinon.stub(AwsEcsDetector, '_getUrlAsJson' as any).rejects(error);
       });
 
-      it('should reject with an error', async () => {
-        try {
-          await awsEcsDetector.detect();
-          throw new Error('Should not have reached here');
-        } catch (err) {
-          assert.strictEqual(err, error);
-        }
+      it('should return empty resource if when there is an error', async () => {
+        const resource = awsEcsDetector.detect();
+        await resource.waitForAsyncAttributes?.();
+
+        assert.deepStrictEqual(resource.attributes, {});
       });
     });
   });

diff --git a/metapackages/auto-instrumentations-node/src/utils.ts b/metapackages/auto-instrumentations-node/src/utils.ts
@@ -235,7 +235,11 @@ function getDisabledInstrumentationsFromEnv() {
 export function getResourceDetectorsFromEnv(): Array<Detector | DetectorSync> {
   const resourceDetectors = new Map<
     string,
-    Detector | DetectorSync | Detector[] | DetectorSync[]
+    | Detector
+    | DetectorSync
+    | Detector[]
+    | DetectorSync[]
+    | Array<Detector | DetectorSync>
   >([
     [RESOURCE_DETECTOR_CONTAINER, containerDetector],
     [RESOURCE_DETECTOR_ENVIRONMENT, envDetectorSync],