feat: allow Resource to be created with some attributes resolving asy…

…nchronously
open-telemetry · Jun 17, 2022 · 944e392 · 944e392
1 parent 3cc40d7
commit 944e392
Show file tree

Hide file tree

Showing 9 changed files with 256 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file.
 
 ### :rocket: (Enhancement)
 
+* feat: allow Resource to be created with some attributes resolving asynchronously [#2933](https://github.com/open-telemetry/opentelemetry-js/pull/2933) @aabmass
+
 ### :bug: (Bug Fix)
 
 * fix(resources): fix browser compatibility for host and os detectors [#3004](https://github.com/open-telemetry/opentelemetry-js/pull/3004) @legendecas

diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts
@@ -24,7 +24,7 @@ import {
 } from '@opentelemetry/instrumentation';
 import { NodeTracerConfig, NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
 import {
-  detectResources,
+  detectResourcesSync,
   envDetector,
   processDetector,
   Resource,
@@ -113,13 +113,13 @@ export class NodeSDK {
   }
 
   /** Detect resource attributes */
-  public async detectResources(config?: ResourceDetectionConfig): Promise<void> {
+  public detectResources(config?: ResourceDetectionConfig): void {
     const internalConfig: ResourceDetectionConfig = {
       detectors: [ envDetector, processDetector],
       ...config,
     };
 
-    this.addResource(await detectResources(internalConfig));
+    this.addResource(detectResourcesSync(internalConfig));
   }
 
   /** Manually add a resource */
@@ -130,9 +130,9 @@ export class NodeSDK {
   /**
    * Once the SDK has been configured, call this method to construct SDK components and register them with the OpenTelemetry API.
    */
-  public async start(): Promise<void> {
+  public start(): void {
     if (this._autoDetectResources) {
-      await this.detectResources();
+      this.detectResources();
     }
 
     if (this._tracerProviderConfig) {

diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
@@ -70,7 +70,7 @@ describe('Node SDK', () => {
         autoDetectResources: false,
       });
 
-      await sdk.start();
+      sdk.start();
 
       assert.strictEqual(context['_getContextManager'](), ctxManager, 'context manager should not change');
       assert.strictEqual(propagation['_getGlobalPropagator'](), propagator, 'propagator should not change');
@@ -85,7 +85,7 @@ describe('Node SDK', () => {
         autoDetectResources: false,
       });
 
-      await sdk.start();
+      sdk.start();
 
       assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider);
 
@@ -108,7 +108,7 @@ describe('Node SDK', () => {
         autoDetectResources: false,
       });
 
-      await sdk.start();
+      sdk.start();
 
       assert.ok(metrics.getMeterProvider() instanceof NoopMeterProvider);
 
@@ -135,7 +135,7 @@ describe('Node SDK', () => {
         autoDetectResources: false,
       });
 
-      await sdk.start();
+      sdk.start();
 
       assert.strictEqual(context['_getContextManager'](), ctxManager, 'context manager should not change');
       assert.strictEqual(propagation['_getGlobalPropagator'](), propagator, 'propagator should not change');
@@ -162,7 +162,7 @@ describe('Node SDK', () => {
         const sdk = new NodeSDK({
           autoDetectResources: true,
         });
-        await sdk.detectResources({
+        sdk.detectResources({
           detectors: [ processDetector, {
             detect() {
               throw new Error('Buggy detector');
@@ -171,6 +171,7 @@ describe('Node SDK', () => {
           envDetector ]
         });
         const resource = sdk['_resource'];
+        await resource.waitForAsyncAttributes();
 
         assertServiceResource(resource, {
           instanceId: '627cc493',
@@ -217,7 +218,8 @@ describe('Node SDK', () => {
           DiagLogLevel.VERBOSE
         );
 
-        await sdk.detectResources();
+        sdk.detectResources();
+        await sdk['_resource'].waitForAsyncAttributes().catch(() => {});
 
         // Test that the Env Detector successfully found its resource and populated it with the right values.
         assert.ok(
@@ -249,7 +251,7 @@ describe('Node SDK', () => {
             DiagLogLevel.DEBUG
           );
 
-          await sdk.detectResources();
+          sdk.detectResources();
 
           assert.ok(
             callArgsContains(

diff --git a/packages/opentelemetry-resources/src/Resource.ts b/packages/opentelemetry-resources/src/Resource.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { diag } from '@opentelemetry/api';
 import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
 import { SDK_INFO } from '@opentelemetry/core';
 import { ResourceAttributes } from './types';
@@ -25,6 +26,10 @@ import { defaultServiceName } from './platform';
  */
 export class Resource {
   static readonly EMPTY = new Resource({});
+  private _attributes: ResourceAttributes;
+  private asyncAttributesPromise: Promise<ResourceAttributes> | undefined;
+
+  private _asyncAttributesHaveResolved: boolean;
 
   /**
    * Returns an empty Resource
@@ -34,7 +39,7 @@ export class Resource {
   }
 
   /**
-   * Returns a Resource that indentifies the SDK in use.
+   *  Returns a Resource that indentifies the SDK in use.
    */
   static default(): Resource {
     return new Resource({
@@ -54,8 +59,50 @@ export class Resource {
      * information about the entity as numbers, strings or booleans
      * TODO: Consider to add check/validation on attributes.
      */
-    readonly attributes: ResourceAttributes
-  ) {}
+    attributes: ResourceAttributes,
+    asyncAttributesPromise?: Promise<ResourceAttributes>,
+  ) {
+    this._attributes = attributes;
+    this._asyncAttributesHaveResolved = asyncAttributesPromise === undefined;
+    this.asyncAttributesPromise = asyncAttributesPromise?.then(
+      asyncAttributes => {
+        this._attributes = Object.assign({}, this._attributes, asyncAttributes);
+        this._asyncAttributesHaveResolved = true;
+        return asyncAttributes;
+      },
+      err => {
+        diag.debug("The resource's async promise rejected: %s", err);
+        this._asyncAttributesHaveResolved = true;
+        return {};
+      }
+    );
+  }
+
+  get attributes(): ResourceAttributes {
+    return this._attributes;
+  }
+
+  /**
+   * Check if async attributes have resolved. This is useful to avoid awaiting
+   * waitForAsyncAttributes (which will introduce asynchronous behavior) when not necessary.
+   *
+   * @returns true if no async attributes promise was provided or if the promise has resolved
+   * and been merged together with the sync attributes.
+   */
+  asyncAttributesHaveResolved(): boolean {
+    return this._asyncAttributesHaveResolved;
+  }
+
+  /**
+   * Returns a promise that resolves when all async attributes have finished being added to
+   * this Resource's attributes. This is useful in exporters to block until resource detection
+   * has finished.
+   */
+  async waitForAsyncAttributes(): Promise<void> {
+    if (!this._asyncAttributesHaveResolved) {
+      await this.asyncAttributesPromise;
+    }
+  }
 
   /**
    * Returns a new, merged {@link Resource} by merging the current Resource
@@ -66,14 +113,27 @@ export class Resource {
    * @returns the newly merged Resource.
    */
   merge(other: Resource | null): Resource {
-    if (!other || !Object.keys(other.attributes).length) return this;
+    if (!other) return this;
 
     // SpanAttributes from resource overwrite attributes from other resource.
     const mergedAttributes = Object.assign(
       {},
       this.attributes,
       other.attributes
     );
-    return new Resource(mergedAttributes);
+
+    let mergedAsyncAttributesPromise: Promise<ResourceAttributes> | undefined;
+    if (this.asyncAttributesPromise && other.asyncAttributesPromise) {
+      mergedAsyncAttributesPromise =  Promise.all([this.asyncAttributesPromise.catch(() => ({})), other.asyncAttributesPromise.catch(() => ({}))]).then(
+        ([thisAttributes, otherAttributes]) => {
+          return Object.assign({}, thisAttributes, otherAttributes);
+        }
+      );
+    } else {
+      mergedAsyncAttributesPromise = this.asyncAttributesPromise ?? other.asyncAttributesPromise;
+    }
+
+
+    return new Resource(mergedAttributes, mergedAsyncAttributesPromise);
   }
 }
diff --git a/packages/opentelemetry-resources/src/platform/node/detect-resources.ts b/packages/opentelemetry-resources/src/platform/node/detect-resources.ts
@@ -20,9 +20,11 @@ import { diag } from '@opentelemetry/api';
 import * as util from 'util';
 
 /**
- * Runs all resource detectors and returns the results merged into a single
- * Resource.
+ * Runs all resource detectors and returns the results merged into a single Resource. Promise
+ * does not resolve until all of the underlying detectors have resolved, unlike
+ * detectResourcesSync.
  *
+ * @deprecated use detectResourceSync() instead.
  * @param config Configuration for resource detection
  */
 export const detectResources = async (
@@ -53,6 +55,58 @@ export const detectResources = async (
 };
 
 
+/**
+ * Runs all resource detectors synchronously, merging their results. Any asynchronous
+ * attributes will be merged together in-order after they resolve.
+ *
+ * @param config Configuration for resource detection
+ */
+export const detectResourcesSync = (
+  config: ResourceDetectionConfig = {}
+): Resource => {
+  const internalConfig: ResourceDetectionConfig = Object.assign(config);
+
+  const resources: Resource[] = (internalConfig.detectors ?? []).map(d => {
+    try {
+      const resourceOrPromise = d.detect(internalConfig);
+      let resource: Resource;
+      if (resourceOrPromise instanceof Promise) {
+        diag.info('Resource detector %s should return a Resource directly instead of a promise.', d.constructor.name);
+        const createPromise = async () => {
+          const resolved = await resourceOrPromise;
+          await resolved.waitForAsyncAttributes();
+          return resolved.attributes;
+        };
+        resource = new Resource({}, createPromise());
+      } else {
+        resource = resourceOrPromise;
+      }
+
+      resource.waitForAsyncAttributes().then(() => {
+        diag.debug(`${d.constructor.name} found resource.`, resource);
+      }).catch(e => {
+        diag.debug(`${d.constructor.name} failed: ${e.message}`);
+      });
+
+      return resource;
+    } catch (e) {
+      diag.debug(`${d.constructor.name} failed: ${e.message}`);
+      return Resource.empty();
+    }
+  });
+
+
+  const mergedResources = resources.reduce(
+    (acc, resource) => acc.merge(resource),
+    Resource.empty()
+  );
+  void mergedResources.waitForAsyncAttributes().then(() => {
+    // Future check if verbose logging is enabled issue #1903
+    logResources(resources);
+  });
+  return mergedResources;
+};
+
 /**
  * Writes debug information about the detected resources to the logger defined in the resource detection config, if one is provided.
  *

diff --git a/packages/opentelemetry-resources/src/types.ts b/packages/opentelemetry-resources/src/types.ts
@@ -26,9 +26,10 @@ import { SpanAttributes } from '@opentelemetry/api';
 export type ResourceAttributes = SpanAttributes;
 
 /**
- * Interface for a Resource Detector. In order to detect resources in parallel
- * a detector returns a Promise containing a Resource.
+ * Interface for a Resource Detector. In order to detect resources asynchronously, a detector
+ * can pass a Promise as the second parameter to the Resource constructor. Returning a
+ * Promise<Resource> is deprecated in favor of this approach.
  */
 export interface Detector {
-  detect(config?: ResourceDetectionConfig): Promise<Resource>;
+  detect(config?: ResourceDetectionConfig): Promise<Resource> | Resource;
 }