8000 ref(core): Remove `startTransaction` & `finishTransaction` hooks (#11… · jonator/sentry-javascript@eaee46f · GitHub
[go: up one dir, main page]
Skip to content

Commit eaee46f

Browse files
mydeamydea
authored
ref(core): Remove startTransaction & finishTransaction hooks (getsentry#11008)
This refactors the remaining usage of `startTransaction` and `finishTransaction` hooks (most of it in tests), and removes these hooks. This is on top of getsentry#11007
1 parent d51fbd3 commit eaee46f

File tree

11 files changed

+79
-118
lines changed

11 files changed

+79
-118
lines changed

‎packages/bun/test/integrations/bunserver.test.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ describe('Bun Serve Integration', () => {
2323
});
2424

2525
test('generates a transaction around a request', async () => {
26-
client.on('finishTransaction', transaction => {
27-
expect(spanToJSON(transaction).status).toBe('ok');
28-
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
29-
expect(spanToJSON(transaction).op).toEqual('http.server');
30-
expect(spanToJSON(transaction).description).toEqual('GET /');
26+
client.on('spanEnd', span => {
27+
expect(spanToJSON(span).status).toBe('ok');
28+
expect(spanToJSON(span).data?.['http.response.status_code']).toEqual(200);
29+
expect(spanToJSON(span).op).toEqual('http.server');
30+
expect(spanToJSON(span).description).toEqual('GET /');
3131
});
3232

3333
const server = Bun.serve({
@@ -43,11 +43,11 @@ describe('Bun Serve Integration', () => {
4343
});
4444

4545
test('generates a post transaction', async () => {
46-
client.on('finishTransaction', transaction => {
47-
expect(spanToJSON(transaction).status).toBe('ok');
48-
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
49-
expect(spanToJSON(transaction).op).toEqual('http.server');
50-
expect(spanToJSON(transaction).description).toEqual('POST /');
46+
client.on('spanEnd', span => {
47+
expect(spanToJSON(span).status).toBe('ok');
48+
expect(spanToJSON(span).data?.['http.response.status_code']).toEqual(200);
49+
expect(spanToJSON(span).op).toEqual('http.server');
50+
expect(spanToJSON(span).description).toEqual('POST /');
5151
});
5252

5353
const server = Bun.serve({
@@ -72,14 +72,13 @@ describe('Bun Serve Integration', () => {
7272
const SENTRY_TRACE_HEADER = `${TRACE_ID}-${PARENT_SPAN_ID}-${PARENT_SAMPLED}`;
7373
const SENTRY_BAGGAGE_HEADER = 'sentry-version=1.0,sentry-environment=production';
7474

75-
client.on('finishTransaction', transaction => {
76-
expect(transaction.spanContext().traceId).toBe(TRACE_ID);
77-
expect(transaction.parentSpanId).toBe(PARENT_SPAN_ID);
78-
expect(spanIsSampled(transaction)).toBe(true);
79-
// span.endTimestamp is already set in `finishTransaction` hook
80-
expect(transaction.isRecording()).toBe(false);
75+
client.on('spanEnd', span => {
76+
expect(span.spanContext().traceId).toBe(TRACE_ID);
77+
expect(spanToJSON(span).parent_span_id).toBe(PARENT_SPAN_ID);
78+
expect(spanIsSampled(span)).toBe(true);
79+
expect(span.isRecording()).toBe(false);
8180

82-
expect(getDynamicSamplingContextFromSpan(transaction)).toStrictEqual({
81+
expect(getDynamicSamplingContextFromSpan(span)).toStrictEqual({
8382
version: '1.0',
8483
environment: 'production',
8584
});
@@ -100,7 +99,7 @@ describe('Bun Serve Integration', () => {
10099
});
101100

102101
test('does not create transactions for OPTIONS or HEAD requests', async () => {
103-
client.on('finishTransaction', () => {
102+
client.on('spanEnd', () => {
104103
// This will never run, but we want to make sure it doesn't run.
105104
expect(false).toEqual(true);
106105
});

‎packages/core/src/baseclient.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import type {
2424
SeverityLevel,
2525
Span,
2626
StartSpanOptions,
27-
Transaction,
2827
TransactionEvent,
2928
Transport,
3029
TransportMakeRequestResponse,
@@ -417,12 +416,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
417416
// Keep on() & emit() signatures in sync with types' client.ts interface
418417
/* eslint-disable @typescript-eslint/unified-signatures */
419418

420-
/** @inheritdoc */
421-
public on(hook: 'startTransaction', callback: (transaction: Transaction) => void): void;
422-
423-
/** @inheritdoc */
424-
public on(hook: 'finishTransaction', callback: (transaction: Transaction) => void): void;
425-
426419
/** @inheritdoc */
427420
public on(hook: 'spanStart', callback: (span: Span) => void): void;
428421

@@ -476,12 +469,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
476469
this._hooks[hook].push(callback);
477470
}
478471

479-
/** @inheritdoc */
480-
public emit(hook: 'startTransaction', transaction: Transaction): void;
481-
482-
/** @inheritdoc */
483-
public emit(hook: 'finishTransaction', transaction: Transaction): void;
484-
485472
/** @inheritdoc */
486473
public emit(hook: 'spanStart', span: Span): void;
487474

‎packages/core/src/tracing/trace.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,6 @@ function _startTransaction(transactionContext: TransactionContext): Transaction
416416
},
417417
});
418418
if (client) {
419-
client.emit('startTransaction', transaction);
420419
client.emit('spanStart', transaction);
421420
}
422421
return transaction;

‎packages/core/src/tracing/transaction.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,6 @@ export class Transaction extends SentrySpan implements TransactionInterface {
224224

225225
// eslint-disable-next-line deprecation/deprecation
226226
const client = this._hub.getClient();
227-
if (client) {
228-
client.emit('finishTransaction', this);
229-
}
230227

231228
if (this._sampled !== true) {
232229
// At this point if `sampled !== true` we want to discard the transaction.

‎packages/core/test/lib/base.test.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Client, Envelope, Event, Transaction } from '@sentry/types';
1+
import type { Client, Envelope, Event } from '@sentry/types';
22
import { SentryError, SyncPromise, dsnToString, logger } from '@sentry/utils';
33

44
import {
@@ -1935,20 +1935,6 @@ describe('BaseClient', () => {
19351935
] as const;
19361936

19371937
describe.each(scenarios)('with client %s', (_, client) => {
1938-
it('should call a startTransaction hook', () => {
1939-
expect.assertions(1);
1940-
1941-
const mockTransaction = {
1942-
traceId: '86f39e84263a4de99c326acab3bfe3bd',
1943-
} as Transaction;
1944-
1945-
client.on('startTransaction', transaction => {
1946-
expect(transaction).toEqual(mockTransaction);
1947-
});
1948-
1949-
client.emit('startTransaction', mockTransaction);
1950-
});
1951-
19521938
it('should call a beforeEnvelope hook', () => {
19531939
expect.assertions(1);
19541940

‎packages/core/test/lib/tracing/trace.test.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
withActiveSpan,
2424
} from '../../../src/tracing';
2525
import { SentryNonRecordingSpan } from '../../../src/tracing/sentryNonRecordingSpan';
26-
import { getActiveSpan, getSpanDescendants } from '../../../src/utils/spanUtils';
26+
import { getActiveSpan, getRootSpan, getSpanDescendants } from '../../../src/utils/spanUtils';
2727
import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
2828

2929
beforeAll(() => {
@@ -84,8 +84,8 @@ describe('startSpan', () => {
8484

8585
it('creates a transaction', async () => {
8686
let _span: Span | undefined = undefined;
87-
client.on('finishTransaction', transaction => {
88-
_span = transaction;
87+
client.on('spanEnd', span => {
88+
_span = span;
8989
});
9090
try {
9191
await startSpan({ name: 'GET users/[id]' }, () => {
@@ -102,8 +102,8 @@ describe('startSpan', () => {
102102

103103
it('allows traceparent information to be overriden', async () => {
104104
let _span: Span | undefined = undefined;
105-
client.on('finishTransaction', transaction => {
106-
_span = transaction;
105+
client.on('spanEnd', span => {
106+
_span = span;
107107
});
108108
try {
109109
await startSpan(
@@ -129,8 +129,8 @@ describe('startSpan', () => {
129129

130130
it('allows for transaction to be mutated', async () => {
131131
let _span: Span | undefined = undefined;
132-
client.on('finishTransaction', transaction => {
133-
_span = transaction;
132+
client.on('spanEnd', span => {
133+
_span = span;
134134
});
135135
try {
136136
await startSpan({ name: 'GET users/[id]' }, span => {
@@ -146,8 +146,10 @@ describe('startSpan', () => {
146146

147147
it('creates a span with correct description', async () => {
148148
let _span: Span | undefined = undefined;
149-
client.on('finishTransaction', transaction => {
150-
_span = transaction;
149+
client.on('spanEnd', span => {
150+
if (span === getRootSpan(span)) {
151+
_span = span;
152+
}
151153
});
152154
try {
153155
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
@@ -170,8 +172,10 @@ describe('startSpan', () => {
170172

171173
it('allows for span to be mutated', async () => {
172174
let _span: Span | undefined = undefined;
173-
client.on('finishTransaction', transaction => {
174-
_span = transaction;
175+
client.on('spanEnd', span => {
176+
if (span === getRootSpan(span)) {
177+
_span = span;
178+
}
175179
});
176180
try {
177181
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
@@ -200,8 +204,8 @@ describe('startSpan', () => {
200204
{ origin: 'manual', attributes: { 'sentry.origin': 'auto.http.browser' } },
201205
])('correctly sets the span origin', async () => {
202206
let _span: Span | undefined = undefined;
203-
client.on('finishTransaction', transaction => {
204-
_span = transaction;
207+
client.on('spanEnd', span => {
208+
_span = span;
205209
});
206210
try {
207211
await startSpan({ name: 'GET users/[id]', origin: 'auto.http.browser' }, () => {

‎packages/replay/src/replay.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ import { EventType, record } from '@sentry-internal/rrweb';
33
import {
44
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
55
captureException,
6+
getActiveSpan,
67
getClient,
7-
getCurrentScope,
8+
getRootSpan,
89
spanToJSON,
910
} from '@sentry/core';
10-
import type { ReplayRecordingMode, Transaction } from '@sentry/types';
11+
import type { ReplayRecordingMode, Span } from '@sentry/types';
1112
import { logger } from '@sentry/utils';
1213

1314
import {
@@ -87,10 +88,10 @@ export class ReplayContainer implements ReplayContainerInterface {
8788
public recordingMode: ReplayRecordingMode;
8889

8990
/**
90-
* The current or last active transcation.
91+
* The current or last active span.
9192
* This is only available when performance is enabled.
9293
*/
93-
public lastTransaction?: Transaction;
94+
public lastActiveSpan?: Span;
9495

9596
/**
9697
* These are here so we can overwrite them in tests etc.
@@ -720,16 +721,16 @@ export class ReplayContainer implements ReplayContainerInterface {
720721
* This is only available if performance is enabled, and if an instrumented router is used.
721722
*/
722723
public getCurrentRoute(): string | undefined {
723-
// eslint-disable-next-line deprecation/deprecation
724-
const lastTransaction = this.lastTransaction || getCurrentScope().getTransaction();
724+
const lastActiveSpan = this.lastActiveSpan || getActiveSpan();
725+
const lastRootSpan = lastActiveSpan && getRootSpan(lastActiveSpan);
725726

726-
const attributes = (lastTransaction && spanToJSON(lastTransaction).data) || {};
727+
const attributes = (lastRootSpan && spanToJSON(lastRootSpan).data) || {};
727728
const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
728-
if (!lastTransaction || !source || !['route', 'custom'].includes(source)) {
729+
if (!lastRootSpan || !source || !['route', 'custom'].includes(source)) {
729730
return undefined;
730731
}
731732

732-
return spanToJSON(lastTransaction).description;
733+
return spanToJSON(lastRootSpan).description;
733734
}
734735

735736
/**

‎packages/replay/src/types/replay.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {
66
ReplayRecordingData,
77
ReplayRecordingMode,
88
SentryWrappedXMLHttpRequest,
9-
Transaction,
9+
Span,
1010
XhrBreadcrumbHint,
1111
} from '@sentry/types';
1212

@@ -478,7 +478,7 @@ export interface ReplayContainer {
478478
session: Session | undefined;
479479
recordingMode: ReplayRecordingMode;
480480
timeouts: Timeouts;
481-
lastTransaction?: Transaction;
481+
lastActiveSpan?: Span;
482482
throttledAddEvent: (
483483
event: RecordingEvent,
484484
isCheckout?: boolean,

‎packages/replay/src/util/addGlobalListeners.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ export function addGlobalListeners(replay: ReplayContainer): void {
4444
}
4545
});
4646

47-
client.on('startTransaction', transaction => {
48-
replay.lastTransaction = transaction;
47+
client.on('spanStart', span => {
48+
replay.lastActiveSpan = span;
4949
});
5050

51-
// We may be missing the initial startTransaction due to timing issues,
51+
// We may be missing the initial spanStart due to timing issues,
5252 63FF
// so we capture it on finish again.
53-
client.on('finishTransaction', transaction => {
54-
replay.lastTransaction = transaction;
53+
client.on('spanEnd', span => {
54+
replay.lastActiveSpan = span;
5555
});
5656

5757
// We want to flush replay

0 commit comments

Comments
 (0)
0