diff --git a/CHANGELOG.md b/CHANGELOG.md index 345fe48283..1d9b3f9363 100644 --- 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 index 2ef82468de..dfddda8ae1 100644 --- a/packages/opentelemetry-resources/src/Resource.ts +++ b/packages/opentelemetry-resources/src/Resource.ts @@ -27,8 +27,9 @@ import { IResource } from './IResource'; */ export class Resource implements IResource { static readonly EMPTY = new Resource({}); - private _syncAttributes: ResourceAttributes; - private _asyncAttributesPromise: Promise | undefined; + private _syncAttributes?: ResourceAttributes; + private _asyncAttributesPromise?: Promise; + private _attributes?: ResourceAttributes; /** * Check if async attributes have resolved. This is useful to avoid awaiting @@ -36,7 +37,7 @@ export class Resource implements IResource { * * @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 ) { + 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,7 +94,7 @@ export class Resource implements IResource { ); } - return this._attributes; + return this._attributes ?? {}; } /** @@ -100,7 +102,7 @@ export class Resource implements IResource { * this Resource's attributes. This is useful in exporters to block until resource detection * has finished. */ - async waitForAsyncAttributes(): Promise { + async waitForAsyncAttributes?(): Promise { if (this.asyncAttributesPending) { await this._asyncAttributesPromise; } diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index df9430af75..b355542288 100644 --- 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 new file mode 100644 index 0000000000..56c996744f --- /dev/null +++ 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 { + return Resource.empty(); + } +} + +describe('Regression Test @opentelemetry/resources@1.9.1', () => { + 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 index 673e631c8d..8351b4b6ed 100644 --- 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(); }