fix(resource): make properties for async resource resolution optional (…

…#3677) * fix(resource): make properties for async resolution optional * fix(changelog): add changelog * fix(resources): add regression test * fix(resources): remove ts-ignore in tests --------- Co-authored-by: Daniel Dyla <[email protected]>
open-telemetry · Mar 16, 2023 · 9a37faf · 9a37faf
1 parent 3653f5c
commit 9a37faf
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :bug: (Bug Fix)
 
+* fix(resource): make properties for async resource resolution optional [#3677](https://github.com/open-telemetry/opentelemetry-js/pull/3677) @pichlermarc
+
 ### :books: (Refine Doc)
 
 ### :house: (Internal)

diff --git a/packages/opentelemetry-resources/src/Resource.ts b/packages/opentelemetry-resources/src/Resource.ts
@@ -27,16 +27,17 @@ import { IResource } from './IResource';
  */
 export class Resource implements IResource {
   static readonly EMPTY = new Resource({});
-  private _syncAttributes: ResourceAttributes;
-  private _asyncAttributesPromise: Promise<ResourceAttributes> | undefined;
+  private _syncAttributes?: ResourceAttributes;
+  private _asyncAttributesPromise?: Promise<ResourceAttributes>;
+  private _attributes?: ResourceAttributes;
 
   /**
    * Check if async attributes have resolved. This is useful to avoid awaiting
    * waitForAsyncAttributes (which will introduce asynchronous behavior) when not necessary.
    *
    * @returns true if the resource "attributes" property is not yet settled to its final value
    */
-  public asyncAttributesPending: boolean;
+  public asyncAttributesPending?: boolean;
 
   /**
    * Returns an empty Resource
@@ -66,11 +67,12 @@ export class Resource implements IResource {
      * information about the entity as numbers, strings or booleans
      * TODO: Consider to add check/validation on attributes.
      */
-    private _attributes: ResourceAttributes,
+    attributes: ResourceAttributes,
     asyncAttributesPromise?: Promise<ResourceAttributes>
   ) {
+    this._attributes = attributes;
     this.asyncAttributesPending = asyncAttributesPromise != null;
-    this._syncAttributes = _attributes;
+    this._syncAttributes = this._attributes ?? {};
     this._asyncAttributesPromise = asyncAttributesPromise?.then(
       asyncAttributes => {
         this._attributes = Object.assign({}, this._attributes, asyncAttributes);
@@ -92,15 +94,15 @@ export class Resource implements IResource {
       );
     }
 
-    return this._attributes;
+    return this._attributes ?? {};
   }
 
   /**
    * Returns a promise that will never be rejected. 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> {
+  async waitForAsyncAttributes?(): Promise<void> {
     if (this.asyncAttributesPending) {
       await this._asyncAttributesPromise;
     }

diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts
@@ -155,7 +155,7 @@ describe('Resource', () => {
       for (const resource of [resourceResolve, resourceReject]) {
         assert.ok(resource.asyncAttributesPending);
         await clock.nextAsync();
-        await resource.waitForAsyncAttributes();
+        await resource.waitForAsyncAttributes?.();
         assert.ok(!resource.asyncAttributesPending);
       }
     });
@@ -174,7 +174,7 @@ describe('Resource', () => {
         asyncAttributes
       );
 
-      await resource.waitForAsyncAttributes();
+      await resource.waitForAsyncAttributes?.();
       assert.deepStrictEqual(resource.attributes, {
         sync: 'fromsync',
         // async takes precedence
@@ -266,7 +266,7 @@ describe('Resource', () => {
       const debugStub = sinon.spy(diag, 'debug');
 
       const resource = new Resource({}, Promise.reject(new Error('rejected')));
-      await resource.waitForAsyncAttributes();
+      await resource.waitForAsyncAttributes?.();
 
       assert.ok(
         debugStub.calledWithMatch(
@@ -325,10 +325,6 @@ describe('Resource', () => {
       const resource = Resource.EMPTY;
       const oldResource = new Resource190({ fromold: 'fromold' });
 
-      //TODO: find a solution for ts-ignore
-
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore
       const mergedResource = resource.merge(oldResource);
 
       assert.strictEqual(mergedResource.attributes['fromold'], 'fromold');
@@ -339,19 +335,12 @@ describe('Resource', () => {
         {},
         Promise.resolve({ fromnew: 'fromnew' })
       );
-
       const oldResource = new Resource190({ fromold: 'fromold' });
 
-      //TODO: find a solution for ts-ignore
-
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore
       const mergedResource = resource.merge(oldResource);
-
       assert.strictEqual(mergedResource.attributes['fromold'], 'fromold');
 
       await mergedResource.waitForAsyncAttributes?.();
-
       assert.strictEqual(mergedResource.attributes['fromnew'], 'fromnew');
     });
   });

diff --git a/packages/opentelemetry-resources/test/regression/existing-detectors-1-9-1.test.ts b/packages/opentelemetry-resources/test/regression/existing-detectors-1-9-1.test.ts
@@ -0,0 +1,31 @@
+/*
+ * 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 { Resource, Detector, ResourceDetectionConfig } from '../../src';
+import * as assert from 'assert';
+
+// DO NOT MODIFY THIS DETECTOR: Previous detectors used Resource as IResource did not yet exist.
+// If compilation fails at this point then the changes made are breaking.
+class RegressionTestResourceDetector_1_9_1 implements Detector {
+  async detect(_config?: ResourceDetectionConfig): Promise<Resource> {
+    return Resource.empty();
+  }
+}
+
+describe('Regression Test @opentelemetry/[email protected]', () => {
+  it('constructor', () => {
+    assert.ok(new RegressionTestResourceDetector_1_9_1());
+  });
+});
diff --git a/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts
@@ -79,17 +79,21 @@ export class SimpleSpanProcessor implements SpanProcessor {
     // Avoid scheduling a promise to make the behavior more predictable and easier to test
     if (span.resource.asyncAttributesPending) {
       const exportPromise = (span.resource as Resource)
-        .waitForAsyncAttributes()
+        .waitForAsyncAttributes?.()
         .then(
           () => {
-            this._unresolvedExports.delete(exportPromise);
+            if (exportPromise != null) {
+              this._unresolvedExports.delete(exportPromise);
+            }
             return doExport();
           },
           err => globalErrorHandler(err)
         );
 
       // store the unresolved exports
-      this._unresolvedExports.add(exportPromise);
+      if (exportPromise != null) {
+        this._unresolvedExports.add(exportPromise);
+      }
     } else {
       void doExport();
     }