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

ref(types): deprecate span status enum #4299

Merged
merged 7 commits into from
Dec 16, 2021
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
ref(types): deprecate transactionmethod enum (#4314)
* ref(types): deprecate transactionmethod enum

* fix(types): drop transactionsamplingmethod

* ref(types): deprecate outcome enum (#4315)

* ref(types): deprecate outcome enum

* fix(types): drop transportoutcome

* ref(types): deprecate request status enum (#4316)

* ref(types): deprecate request status

* ref(types): deprecate session status

* ref(types): remove unused logLevel (#4317) (#4320)
  • Loading branch information
JonasBa authored Dec 16, 2021

Verified

This commit was signed with the committer’s verified signature.
ronnnnn Seiya Kokushi
commit d305f6215b59ae481834a595dcbfa42f350c585d
8 changes: 4 additions & 4 deletions packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core';
import { Event, Outcome, Response, SentryRequest, Session, TransportOptions } from '@sentry/types';
import { Event, Response, SentryRequest, Session, TransportOptions } from '@sentry/types';
import { SentryError, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';
@@ -37,7 +37,7 @@ export class FetchTransport extends BaseTransport {
*/
private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
if (this._isRateLimited(sentryRequest.type)) {
this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type);
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);

return Promise.reject({
event: originalPayload,
@@ -89,9 +89,9 @@ export class FetchTransport extends BaseTransport {
.then(undefined, reason => {
// It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError.
if (reason instanceof SentryError) {
this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type);
this.recordLostEvent('queue_overflow', sentryRequest.type);
} else {
this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
this.recordLostEvent('network_error', sentryRequest.type);
}
throw reason;
});
8 changes: 4 additions & 4 deletions packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core';
import { Event, Outcome, Response, SentryRequest, Session } from '@sentry/types';
import { Event, Response, SentryRequest, Session } from '@sentry/types';
import { SentryError, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';
@@ -26,7 +26,7 @@ export class XHRTransport extends BaseTransport {
*/
private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
if (this._isRateLimited(sentryRequest.type)) {
this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type);
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);

return Promise.reject({
event: originalPayload,
@@ -66,9 +66,9 @@ export class XHRTransport extends BaseTransport {
.then(undefined, reason => {
// It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError.
if (reason instanceof SentryError) {
this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type);
this.recordLostEvent('queue_overflow', sentryRequest.type);
} else {
this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
this.recordLostEvent('network_error', sentryRequest.type);
}
throw reason;
});
32 changes: 15 additions & 17 deletions packages/browser/test/unit/transports/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { Outcome } from '@sentry/types';

import { BaseTransport } from '../../../src/transports/base';

const testDsn = 'https://[email protected]/42';
@@ -44,12 +42,12 @@ describe('BaseTransport', () => {
it('sends beacon request when there are outcomes captured and visibility changed to `hidden`', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [{ reason: Outcome.BeforeSend, category: 'error', quantity: 1 }];
const outcomes = [{ reason: 'before_send', category: 'error', quantity: 1 }];

expect(sendBeaconSpy).toHaveBeenCalledWith(
envelopeEndpoint,
@@ -59,7 +57,7 @@ describe('BaseTransport', () => {

it('doesnt send beacon request when there are outcomes captured, but visibility state did not change to `hidden`', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });
transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'visible';
document.dispatchEvent(new Event('visibilitychange'));
@@ -70,21 +68,21 @@ describe('BaseTransport', () => {
it('correctly serializes request with different categories/reasons pairs', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent(Outcome.SampleRate, 'transaction');
transport.recordLostEvent(Outcome.NetworkError, 'session');
transport.recordLostEvent(Outcome.NetworkError, 'session');
transport.recordLostEvent(Outcome.RateLimitBackoff, 'event');
transport.recordLostEvent('before_send', 'event');
transport.recordLostEvent('before_send', 'event');
transport.recordLostEvent('sample_rate', 'transaction');
transport.recordLostEvent('network_error', 'session');
transport.recordLostEvent('network_error', 'session');
transport.recordLostEvent('ratelimit_backoff', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [
{ reason: Outcome.BeforeSend, category: 'error', quantity: 2 },
{ reason: Outcome.SampleRate, category: 'transaction', quantity: 1 },
{ reason: Outcome.NetworkError, category: 'session', quantity: 2 },
{ reason: Outcome.RateLimitBackoff, category: 'error', quantity: 1 },
{ reason: 'before_send', category: 'error', quantity: 2 },
{ reason: 'sample_rate', category: 'transaction', quantity: 1 },
{ reason: 'network_error', category: 'session', quantity: 2 },
{ reason: 'ratelimit_backoff', category: 'error', quantity: 1 },
];

expect(sendBeaconSpy).toHaveBeenCalledWith(
@@ -97,12 +95,12 @@ describe('BaseTransport', () => {
const tunnel = 'https://hello.com/world';
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true, tunnel });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [{ reason: Outcome.BeforeSend, category: 'error', quantity: 1 }];
const outcomes = [{ reason: 'before_send', category: 'error', quantity: 1 }];

expect(sendBeaconSpy).toHaveBeenCalledWith(
tunnel,
9 changes: 4 additions & 5 deletions packages/browser/test/unit/transports/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Outcome } from '@sentry/types';
import { SentryError } from '@sentry/utils';

import { Event, Response, Transports } from '../../../src';
@@ -117,7 +116,7 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
expect(spy).toHaveBeenCalledWith('network_error', 'event');
}
});

@@ -129,7 +128,7 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction');
expect(spy).toHaveBeenCalledWith('queue_overflow', 'transaction');
}
});

@@ -490,13 +489,13 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'event');
}

try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
}
});
});
9 changes: 4 additions & 5 deletions packages/browser/test/unit/transports/xhr.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Outcome } from '@sentry/types';
import { SentryError } from '@sentry/utils';
import { fakeServer, SinonFakeServer } from 'sinon';

@@ -79,7 +78,7 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
expect(spy).toHaveBeenCalledWith('network_error', 'event');
}
});

@@ -91,7 +90,7 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction');
expect(spy).toHaveBeenCalledWith('queue_overflow', 'transaction');
}
});

@@ -416,13 +415,13 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'event');
}

try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
}
});
});
12 changes: 5 additions & 7 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
@@ -7,8 +7,6 @@ import {
Integration,
IntegrationClass,
Options,
Outcome,
SessionStatus,
SeverityLevel,
Transport,
} from '@sentry/types';
@@ -268,12 +266,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
// A session is updated and that session update is sent in only one of the two following scenarios:
// 1. Session with non terminal status and 0 errors + an error occurred -> Will set error count to 1 and send update
// 2. Session with non terminal status and 1 error + a crash occurred -> Will set status crashed and send update
const sessionNonTerminal = session.status === SessionStatus.Ok;
const sessionNonTerminal = session.status === 'ok';
const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed);

if (shouldUpdateAndSend) {
session.update({
...(crashed && { status: SessionStatus.Crashed }),
...(crashed && { status: 'crashed' }),
errors: session.errors || Number(errored || crashed),
});
this.captureSession(session);
@@ -541,7 +539,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
// 0.0 === 0% events are sent
// Sampling for transaction happens somewhere else
if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
recordLostEvent(Outcome.SampleRate, 'event');
recordLostEvent('sample_rate', 'event');
return SyncPromise.reject(
new SentryError(
`Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
@@ -552,7 +550,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
return this._prepareEvent(event, scope, hint)
.then(prepared => {
if (prepared === null) {
recordLostEvent(Outcome.EventProcessor, event.type || 'event');
recordLostEvent('event_processor', event.type || 'event');
throw new SentryError('An event processor returned null, will not send event.');
}

@@ -566,7 +564,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
})
.then(processedEvent => {
if (processedEvent === null) {
recordLostEvent(Outcome.BeforeSend, event.type || 'event');
recordLostEvent('before_send', event.type || 'event');
throw new SentryError('`beforeSend` returned `null`, will not send event.');
}

8 changes: 4 additions & 4 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Hub, Scope, Session } from '@sentry/hub';
import { Event, Outcome, Span, Transport } from '@sentry/types';
import { Event, Span, Transport } from '@sentry/types';
import { logger, SentryError, SyncPromise } from '@sentry/utils';

import * as integrationModule from '../../src/integration';
@@ -882,7 +882,7 @@ describe('BaseClient', () => {

client.captureEvent({ message: 'hello' }, {});

expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.BeforeSend, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'event');
});

test('eventProcessor can drop the even when it returns null', () => {
@@ -914,7 +914,7 @@ describe('BaseClient', () => {
scope.addEventProcessor(() => null);
client.captureEvent({ message: 'hello' }, {}, scope);

expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.EventProcessor, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'event');
});

test('eventProcessor sends an event and logs when it crashes', () => {
@@ -958,7 +958,7 @@ describe('BaseClient', () => {
);

client.captureEvent({ message: 'hello' }, {});
expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.SampleRate, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'event');
});
});

10 changes: 5 additions & 5 deletions packages/core/test/lib/request.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DebugMeta, Event, SentryRequest, TransactionSamplingMethod } from '@sentry/types';
import { DebugMeta, Event, SentryRequest } from '@sentry/types';

import { initAPIDetails } from '../../src/api';
import { eventToSentryRequest, sessionToSentryRequest } from '../../src/request';
@@ -44,21 +44,21 @@ describe('eventToSentryRequest', () => {
});

it('adds transaction sampling information to item header', () => {
event.debug_meta = { transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 } };
event.debug_meta = { transactionSampling: { method: 'client_rate', rate: 0.1121 } };

const result = eventToSentryRequest(event, api);
const envelope = parseEnvelopeRequest(result);

expect(envelope.itemHeader).toEqual(
expect.objectContaining({
sample_rates: [{ id: TransactionSamplingMethod.Rate, rate: 0.1121 }],
sample_rates: [{ id: 'client_rate', rate: 0.1121 }],
}),
);
});

it('removes transaction sampling information (and only that) from debug_meta', () => {
event.debug_meta = {
transactionSampling: { method: TransactionSamplingMethod.Sampler, rate: 0.1121 },
transactionSampling: { method: 'client_sampler', rate: 0.1121 },
dog: 'Charlie',
} as DebugMeta;

@@ -71,7 +71,7 @@ describe('eventToSentryRequest', () => {

it('removes debug_meta entirely if it ends up empty', () => {
event.debug_meta = {
transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 },
transactionSampling: { method: 'client_rate', rate: 0.1121 },
} as DebugMeta;

const result = eventToSentryRequest(event, api);
5 changes: 2 additions & 3 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ import {
IntegrationClass,
Primitive,
SessionContext,
SessionStatus,
SeverityLevel,
Span,
SpanContext,
@@ -451,8 +450,8 @@ export class Hub implements HubInterface {
if (scope) {
// End existing session if there's one
const currentSession = scope.getSession && scope.getSession();
if (currentSession && currentSession.status === SessionStatus.Ok) {
currentSession.update({ status: SessionStatus.Exited });
if (currentSession && currentSession.status === 'ok') {
currentSession.update({ status: 'exited' });
}
this.endSession();

8 changes: 4 additions & 4 deletions packages/hub/src/session.ts
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@ export class Session implements SessionInterface {
public timestamp: number;
public started: number;
public duration?: number = 0;
public status: SessionStatus = SessionStatus.Ok;
public status: SessionStatus = 'ok';
public environment?: string;
public ipAddress?: string;
public init: boolean = true;
@@ -88,11 +88,11 @@ export class Session implements SessionInterface {
}

/** JSDoc */
public close(status?: Exclude<SessionStatus, SessionStatus.Ok>): void {
public close(status?: Exclude<SessionStatus, 'ok'>): void {
if (status) {
this.update({ status });
} else if (this.status === SessionStatus.Ok) {
this.update({ status: SessionStatus.Exited });
} else if (this.status === 'ok') {
this.update({ status: 'exited' });
} else {
this.update();
}
6 changes: 3 additions & 3 deletions packages/hub/src/sessionflusher.ts
Original file line number Diff line number Diff line change
@@ -113,13 +113,13 @@ export class SessionFlusher implements SessionFlusherLike {
}

switch (status) {
case RequestSessionStatus.Errored:
case 'errored':
aggregationCounts.errored = (aggregationCounts.errored || 0) + 1;
return aggregationCounts.errored;
case RequestSessionStatus.Ok:
case 'ok':
aggregationCounts.exited = (aggregationCounts.exited || 0) + 1;
return aggregationCounts.exited;
case RequestSessionStatus.Crashed:
default:
aggregationCounts.crashed = (aggregationCounts.crashed || 0) + 1;
return aggregationCounts.crashed;
}
Loading