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.

Sign up for GitHub

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

fix: Remove unknown_error transaction status #2440

Merged
merged 3 commits into from
Feb 19, 2020
Merged

fix: Remove unknown_error transaction status #2440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1669c84
ref: Mark transaction as failed on error
HazAT Feb 19, 2020
5e90412
ref: Remove transaction status
HazAT Feb 19, 2020
d8cf0dc
fix: Don't set status if UnknownError
HazAT Feb 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions packages/apm/src/integrations/tracing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,16 @@ export class Tracing implements Integration {
return event;
}

if (
Tracing.options.maxTransactionDuration !== 0 &&
Tracing._isEnabled() &&
event.type === 'transaction' &&
event.timestamp &&
event.start_timestamp &&
(event.timestamp - event.start_timestamp > Tracing.options.maxTransactionDuration ||
event.timestamp - event.start_timestamp < 0)
) {
return null;
if (Tracing._isEnabled()) {
const isOutdatedTransaction =
event.timestamp &&
event.start_timestamp &&
(event.timestamp - event.start_timestamp > Tracing.options.maxTransactionDuration ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd extract this to something more descriptive like isOutdatedTransaction or something, as its easier to understand what these 2 timing checks do

event.timestamp - event.start_timestamp < 0);

if (Tracing.options.maxTransactionDuration !== 0 && event.type === 'transaction' && isOutdatedTransaction) {
return null;
}
}

return event;
Expand Down
5 changes: 4 additions & 1 deletion packages/apm/src/span.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ export class Span implements SpanInterface, SpanContext {
*/
public setHttpStatus(httpStatus: number): this {
this.setTag('http.status_code', String(httpStatus));
this.setStatus(SpanStatus.fromHttpCode(httpStatus));
const spanStatus = SpanStatus.fromHttpCode(httpStatus);
if (spanStatus !== SpanStatus.UnknownError) {
this.setStatus(spanStatus);
}
return this;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/span.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export enum SpanStatus {
/** The operation completed successfully. */
Ok = 'ok',
/** Deadline expired before operation could complete. */
DealineExceeded = 'deadline_exceeded',
Copy link
Member

Choose a reason for hiding this comment

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

good typo :D

DeadlineExceeded = 'deadline_exceeded',
/** 401 Unauthorized (actually does mean unauthenticated according to RFC 7235) */
Unauthenticated = 'unauthenticated',
/** 403 Forbidden */
Expand Down Expand Up @@ -164,7 +164,7 @@ export namespace SpanStatus {
case 503:
return SpanStatus.Unavailable;
case 504:
return SpanStatus.DealineExceeded;
return SpanStatus.DeadlineExceeded;
default:
return SpanStatus.InternalError;
}
Expand Down