Skip to content

Commit

Permalink
Revert "feat(api): add attributes argument to recordException API (#4071
Browse files Browse the repository at this point in the history
)" (#4137)
  • Loading branch information
pichlermarc authored Sep 11, 2023
1 parent 5fcd8cf commit 27897d6
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 105 deletions.
5 changes: 5 additions & 0 deletions api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.

## Unreleased

### :bug: (Bug Fix)

* Revert "feat(api): add attributes argument to recordException API [#4071](https://github.com/open-telemetry/opentelemetry-js/pull/4071)"
* This feature was an unintentional breaking change introduced with 1.5.0

## 1.5.0

### :rocket: (Enhancement)
Expand Down
6 changes: 1 addition & 5 deletions api/src/trace/NonRecordingSpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,5 @@ export class NonRecordingSpan implements Span {
}

// By default does nothing
recordException(
_exception: Exception,
_attributesOrStartTime?: SpanAttributes | TimeInput,
_time?: TimeInput
): void {}
recordException(_exception: Exception, _time?: TimeInput): void {}
}
13 changes: 0 additions & 13 deletions api/src/trace/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,4 @@ export interface Span {
* use the current time.
*/
recordException(exception: Exception, time?: TimeInput): void;

/**
* Sets exception as a span event
* @param exception the exception the only accepted values are string or Error
* @param [attributes] the attributes that will be added to the error event.
* @param [time] the time to set as Span's event time. If not provided,
* use the current time.
*/
recordException(
exception: Exception,
attributes?: SpanAttributes,
time?: TimeInput
): void;
}
18 changes: 2 additions & 16 deletions packages/opentelemetry-sdk-trace-base/src/Span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,7 @@ export class Span implements APISpan, ReadableSpan {
return this._ended === false;
}

recordException(
exception: Exception,
attributesOrStartTime?: SpanAttributes | TimeInput,
timeStamp?: TimeInput
): void {
if (isTimeInput(attributesOrStartTime)) {
if (!isTimeInput(timeStamp)) {
timeStamp = attributesOrStartTime;
}
attributesOrStartTime = undefined;
}

recordException(exception: Exception, time?: TimeInput): void {
const attributes: SpanAttributes = {};
if (typeof exception === 'string') {
attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
Expand All @@ -301,16 +290,13 @@ export class Span implements APISpan, ReadableSpan {
attributes[SemanticAttributes.EXCEPTION_STACKTRACE] = exception.stack;
}
}
if (attributesOrStartTime) {
Object.assign(attributes, sanitizeAttributes(attributesOrStartTime));
}

// these are minimum requirements from spec
if (
attributes[SemanticAttributes.EXCEPTION_TYPE] ||
attributes[SemanticAttributes.EXCEPTION_MESSAGE]
) {
this.addEvent(ExceptionEventName, attributes, timeStamp);
this.addEvent(ExceptionEventName, attributes, time);
} else {
diag.warn(`Failed to record an exception ${exception}`);
}
Expand Down
71 changes: 0 additions & 71 deletions packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1203,77 +1203,6 @@ describe('Span', () => {
const event = span.events[0];
assert.deepStrictEqual(event.time, [0, 123]);
});

it('should record an exception with provided time as a 3rd arg', () => {
const span = new Span(
tracer,
ROOT_CONTEXT,
name,
spanContext,
SpanKind.CLIENT
);
// @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
span['_performanceOffset'] = 0;
assert.strictEqual(span.events.length, 0);
span.recordException('boom', undefined, [0, 123]);
const event = span.events[0];
assert.deepStrictEqual(event.time, [0, 123]);
});
});

describe('when attributes are provided', () => {
it('should sanitized and merge attributes when provided', () => {
const span = new Span(
tracer,
ROOT_CONTEXT,
name,
spanContext,
SpanKind.CLIENT
);
// @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
span['_performanceOffset'] = 0;
assert.strictEqual(span.events.length, 0);
const exception = { code: 'Error', message: 'boom', stack: 'bar' };
span.recordException(exception, {
...validAttributes,
...invalidAttributes,
} as unknown as SpanAttributes);
const event = span.events[0];
assert.deepStrictEqual(event.attributes, {
[SemanticAttributes.EXCEPTION_TYPE]: 'Error',
[SemanticAttributes.EXCEPTION_MESSAGE]: 'boom',
[SemanticAttributes.EXCEPTION_STACKTRACE]: 'bar',
...validAttributes,
});
});

it('should prioritize the provided attributes over generated', () => {
const span = new Span(
tracer,
ROOT_CONTEXT,
name,
spanContext,
SpanKind.CLIENT
);
// @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
span['_performanceOffset'] = 0;
assert.strictEqual(span.events.length, 0);
const exception = { code: 'Error', message: 'boom', stack: 'bar' };
span.recordException(exception, {
[SemanticAttributes.EXCEPTION_TYPE]: 'OverrideError',
[SemanticAttributes.EXCEPTION_MESSAGE]: 'override-boom',
[SemanticAttributes.EXCEPTION_STACKTRACE]: 'override-bar',
...validAttributes,
...invalidAttributes,
} as unknown as SpanAttributes);
const event = span.events[0];
assert.deepStrictEqual(event.attributes, {
...validAttributes,
[SemanticAttributes.EXCEPTION_TYPE]: 'OverrideError',
[SemanticAttributes.EXCEPTION_MESSAGE]: 'override-boom',
[SemanticAttributes.EXCEPTION_STACKTRACE]: 'override-bar',
});
});
});

describe('when exception code is numeric', () => {
Expand Down

0 comments on commit 27897d6

Please sign in to comment.