Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved logging mechanism for operations on ended spans, making it easier to trace issues in both Node.js and Browser environments #5113 #5184

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
14 changes: 10 additions & 4 deletions packages/opentelemetry-sdk-trace-base/src/Span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,18 @@ export class Span implements APISpan, ReadableSpan {

private _isSpanEnded(): boolean {
if (this._ended) {
diag.warn(
`Can not execute the operation on ended Span {traceId: ${this._spanContext.traceId}, spanId: ${this._spanContext.spanId}}`
);
const error = new Error(`Operation attempted on ended Span {traceId: ${this._spanContext.traceId}, spanId: ${this._spanContext.spanId}}`);

diag.warn(
`Cannot execute the operation on ended Span {traceId: ${this._spanContext.traceId}, spanId: ${this._spanContext.spanId}}. Change log level to debug for stack trace.`,
error // Pass the error object as the second argument
);
Comment on lines +368 to +371
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we remove the additional debug log, we can also remove the message telling people to enable it.

Suggested change
diag.warn(
`Cannot execute the operation on ended Span {traceId: ${this._spanContext.traceId}, spanId: ${this._spanContext.spanId}}. Change log level to debug for stack trace.`,
error // Pass the error object as the second argument
);
diag.warn(
`Cannot execute the operation on ended Span {traceId: ${this._spanContext.traceId}, spanId: ${this._spanContext.spanId}}.`,
error // Pass the error object as the second argument
);


// Optionally, you can still log the stack trace for additional context
diag.debug(`Stack trace for ended span operation: ${error.stack}`);
Comment on lines +373 to +374
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one now does not provide extra value since the error is passed to the warn, we can safely remove it :)

}
return this._ended;
}
}

// Utility function to truncate given value within size
// for value type of string, will truncate to given limit
Expand Down
26 changes: 26 additions & 0 deletions packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,32 @@
assert.ok(span.isRecording() === false);
});
});
it('should log a warning and a debug message when span is ended', () => {
const span = new Span(
tracer,
ROOT_CONTEXT,
name,
spanContext,
SpanKind.CLIENT
);
span.end(); // End the span to set it to ended state

const warnStub = sinon.spy(diag, 'warn');
const debugStub = sinon.spy(diag, 'debug');

// Call the method that checks if the span is ended
span['_isSpanEnded']();

// Assert that the warning log was called with the expected message and an Error object
sinon.assert.calledOnce(warnStub);
sinon.assert.calledWith(warnStub, sinon.match(/Cannot execute the operation on ended Span/), sinon.match.instanceOf(Error));

// Assert that the debug log was called and contains a stack trace
sinon.assert.calledOnce(debugStub);
const debugMessage = debugStub.getCall(0).args[0];
assert.ok(debugMessage.startsWith('Stack trace for ended span operation:'));
});
});

describe('setAttribute', () => {
describe('when default options set', () => {
Expand Down Expand Up @@ -1346,4 +1372,4 @@
});
});
});
});

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-windows-tests

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-windows-tests

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (14)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (14)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (16)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (16)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (18)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (18)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (20)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (20)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (22)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / node-tests (22)

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / browser-tests

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / browser-tests

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / webworker-tests

Declaration or statement expected.

Check failure on line 1375 in packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts

View workflow job for this annotation

GitHub Actions / webworker-tests

Declaration or statement expected.
Loading