Skip to content

Commit 743c6c4

Browse files
AbhiPrasadAbhiPrasad
authored
feat(v8/core): Remove getters for span.op (#10767)
1 parent 15216cd commit 743c6c4

File tree

20 files changed

+69
-171
lines changed

20 files changed

+69
-171
lines changed

‎dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/event-proxy-server.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as os from 'os';
66
import * as path from 'path';
77
import * as util from 'util';
88
import * as zlib from 'zlib';
9-
import type { Envelope, EnvelopeItem, Event } from '@sentry/types';
9+
import type { Envelope, EnvelopeItem, SerializedEvent } from '@sentry/types';
1010
import { parseEnvelope } from '@sentry/utils';
1111

1212
const readFile = util.promisify(fs.readFile);
@@ -210,13 +210,13 @@ export function waitForEnvelopeItem(
210210

211211
export function waitForError(
212212
proxyServerName: string,
213-
callback: (transactionEvent: Event) => Promise<boolean> | boolean,
214-
): Promise<Event> {
213+
callback: (transactionEvent: SerializedEvent) => Promise<boolean> | boolean,
214+
): Promise<SerializedEvent> {
215215
return new Promise((resolve, reject) => {
216216
waitForEnvelopeItem(proxyServerName, async envelopeItem => {
217217
const [envelopeItemHeader, envelopeItemBody] = envelopeItem;
218-
if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as Event))) {
219-
resolve(envelopeItemBody as Event);
218+
if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as SerializedEvent))) {
219+
resolve(envelopeItemBody as SerializedEvent);
220220
return true;
221221
}
222222
return false;
@@ -226,13 +226,13 @@ export function waitForError(
226226

227227
export function waitForTransaction(
228228
proxyServerName: string,
229-
callback: (transactionEvent: Event) => Promise<boolean> | boolean,
230-
): Promise<Event> {
229+
callback: (transactionEvent: SerializedEvent) => Promise<boolean> | boolean,
230+
): Promise<SerializedEvent> {
231231
return new Promise((resolve, reject) => {
232232
waitForEnvelopeItem(proxyServerName, async envelopeItem => {
233233
const [envelopeItemHeader, envelopeItemBody] = envelopeItem;
234-
if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as Event))) {
235-
resolve(envelopeItemBody as Event);
234+
if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as SerializedEvent))) {
235+
resolve(envelopeItemBody as SerializedEvent);
236236
return true;
237237
}
238238
return false;

‎packages/browser/src/profiling/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */
22

3-
import { DEFAULT_ENVIRONMENT, getClient } from '@sentry/core';
3+
import { DEFAULT_ENVIRONMENT, getClient, spanToJSON } from '@sentry/core';
44
import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
55
import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
66
import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';
@@ -202,7 +202,7 @@ export function isProfiledTransactionEvent(event: Event): event is ProfiledEvent
202202
*
203203
*/
204204
export function isAutomatedPageLoadTransaction(transaction: Transaction): boolean {
205-
return transaction.op === 'pageload';
205+
return spanToJSON(transaction).op === 'pageload';
206206
}
207207

208208
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('Bun Serve Integration', () => {
2929
expect(transaction.tags).toEqual({
3030
'http.status_code': '200',
3131
});
32-
expect(transaction.op).toEqual('http.server');
32+
expect(spanToJSON(transaction).op).toEqual('http.server');
3333
expect(spanToJSON(transaction).description).toEqual('GET /');
3434
});
3535

@@ -52,7 +52,7 @@ describe('Bun Serve Integration', () => {
5252
expect(transaction.tags).toEqual({
5353
'http.status_code': '200',
5454
});
55-
expect(transaction.op).toEqual('http.server');
55+
expect(spanToJSON(transaction).op).toEqual('http.server');
5656
expect(spanToJSON(transaction).description).toEqual('POST /');
5757
});
5858

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,16 +162,16 @@ export class IdleTransaction extends Transaction {
162162
this._finished = true;
163163
this.activities = {};
164164

165-
// eslint-disable-next-line deprecation/deprecation
166-
if (this.op === 'ui.action.click') {
165+
const op = spanToJSON(this).op;
166+
167+
if (op === 'ui.action.click') {
167168
this.setAttribute(FINISH_REASON_TAG, this._finishReason);
168169
}
169170

170171
// eslint-disable-next-line deprecation/deprecation
171172
if (this.spanRecorder) {
172173
DEBUG_BUILD &&
173-
// eslint-disable-next-line deprecation/deprecation
174-
logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), this.op);
174+
logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), op);
175175

176176
for (const callback of this._beforeFinishCallbacks) {
177177
callback(this, endTimestampInS);

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,11 @@ export function sampleTransaction<T extends Transaction>(
9494
return transaction;
9595
}
9696

97-
DEBUG_BUILD &&
98-
// eslint-disable-next-line deprecation/deprecation
99-
logger.log(`[Tracing] starting ${transaction.op} transaction - ${spanToJSON(transaction).description}`);
97+
if (DEBUG_BUILD) {
98+
const { op, description } = spanToJSON(transaction);
99+
logger.log(`[Tracing] starting ${op} transaction - ${description}`);
100+
}
101+
100102
return transaction;
101103
}
102104

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

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable max-lines */
21
import type {
32
Instrumenter,
43
Primitive,
@@ -296,25 +295,6 @@ export class SentrySpan implements SpanInterface {
296295
this._status = status;
297296
}
298297

299-
/**
300-
* Operation of the span
301-
*
302-
* @deprecated Use `spanToJSON().op` to read the op instead.
303-
*/
304-
public get op(): string | undefined {
305-
return this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined;
306-
}
307-
308-
/**
309-
* Operation of the span
310-
*
311-
* @deprecated Use `startSpan()` functions to set or `span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'op')
312-
* to update the span instead.
313-
*/
314-
public set op(op: string | undefined) {
315-
this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op);
316-
}
317-
318298
/* eslint-enable @typescript-eslint/member-ordering */
319299

320300
/** @inheritdoc */
@@ -493,8 +473,7 @@ export class SentrySpan implements SpanInterface {
493473
data: this._getData(),
494474
name: this._name,
495475
endTimestamp: this._endTime,
496-
// eslint-disable-next-line deprecation/deprecation
497-
op: this.op,
476+
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
498477
parentSpanId: this._parentSpanId,
499478
sampled: this._sampled,
500479
spanId: this._spanId,
@@ -516,8 +495,7 @@ export class SentrySpan implements SpanInterface {
516495
this.data = spanContext.data || {};
517496
this._name = spanContext.name;
518497
this._endTime = spanContext.endTimestamp;
519-
// eslint-disable-next-line deprecation/deprecation
520-
this.op = spanContext.op;
498+
this._attributes = { ...this._attributes, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: spanContext.op };
521499
this._parentSpanId = spanContext.parentSpanId;
522500
this._sampled = spanContext.sampled;
523501
this._spanId = spanContext.spanId || this._spanId;
@@ -551,7 +529,7 @@ export class SentrySpan implements SpanInterface {
551529
return dropUndefinedKeys({
552530
data: this._getData(),
553531
description: this._name,
554-
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined,
532+
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
555533
parent_span_id: this._parentSpanId,
556534
span_id: this._spanId,
557535
start_timestamp: this._startTime,

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ export class Transaction extends SentrySpan implements TransactionInterface {
323323
transaction.measurements = this._measurements;
324324
}
325325

326-
// eslint-disable-next-line deprecation/deprecation
327-
DEBUG_BUILD && logger.log(`[Tracing] Finishing ${this.op} transaction: ${this._name}.`);
328-
326+
DEBUG_BUILD && logger.log(`[Tracing] Finishing ${spanToJSON(this).op} transaction: ${this._name}.`);
329327
return transaction;
330328
}
331329
}

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

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -136,27 +136,6 @@ describe('startSpan', () => {
136136
expect(ref.parentSpanId).toEqual('1234567890123456');
137137
});
138138

139-
// TODO (v8): Remove this test in favour of the one below
140-
it('(deprecated op) allows for transaction to be mutated', async () => {
141-
let ref: any = undefined;
142-
client.on('finishTransaction', transaction => {
143-
ref = transaction;
144-
});
145-
try {
146-
await startSpan({ name: 'GET users/[id]' }, span => {
147-
if (span) {
148-
// eslint-disable-next-line deprecation/deprecation
149-
span.op = 'http.server';
150-
}
151-
return callback();
152-
});
153-
} catch (e) {
154-
//
155-
}
156-
157-
expect(spanToJSON(ref).op).toEqual('http.server');
158-
});
159-
160139
it('allows for transaction to be mutated', async () => {
161140
let ref: any = undefined;
162141
client.on('finishTransaction', transaction => {
@@ -173,7 +152,7 @@ describe('startSpan', () => {
173152
//
174153
}
175154

176-
expect(ref.op).toEqual('http.server');
155+
expect(spanToJSON(ref).op).toEqual('http.server');
177156
});
178157

179158
it('creates a span with correct description', async () => {
@@ -197,30 +176,6 @@ describe('startSpan', () => {
197176
expect(ref.spanRecorder.spans[1].status).toEqual(isError ? 'internal_error' : undefined);
198177
});
199178

200-
// TODO (v8): Remove this test in favour of the one below
201-
it('(deprecated op) allows for span to be mutated', async () => {
202-
let ref: any = undefined;
203-
client.on('finishTransaction', transaction => {
204-
ref = transaction;
205-
});
206-
try {
207-
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
208-
return startSpan({ name: 'SELECT * from users' }, childSpan => {
209-
if (childSpan) {
210-
// eslint-disable-next-line deprecation/deprecation
211-
childSpan.op = 'db.query';
212-
}
213-
return callback();
214-
});
215-
});
216-
} catch (e) {
217-
//
218-
}
219-
220-
expect(ref.spanRecorder.spans).toHaveLength(2);
221-
expect(ref.spanRecorder.spans[1].op).toEqual('db.query');
222-
});
223-
224179
it('allows for span to be mutated', async () => {
225180
let ref: any = undefined;
226181
client.on('finishTransaction', transaction => {

‎packages/node/src/handlers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type * as http from 'http';
22
/* eslint-disable @typescript-eslint/no-explicit-any */
33
import {
4+
SEMANTIC_ATTRIBUTE_SENTRY_OP,
45
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
56
captureException,
67
continueTrace,
@@ -306,7 +307,7 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
306307
if (sentryTransaction) {
307308
sentryTransaction.updateName(`trpc/${path}`);
308309
sentryTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
309-
sentryTransaction.op = 'rpc.server';
310+
sentryTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'rpc.server');
310311

311312
const trpcContext: Record<string, unknown> = {
312313
procedure_type: type,

‎packages/node/test/integrations/http.test.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ describe('tracing', () => {
9090

9191
// our span is at index 1 because the transaction itself is at index 0
9292
expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/');
93-
// eslint-disable-next-line deprecation/deprecation
94-
expect(spans[1].op).toEqual('http.client');
9593
expect(spanToJSON(spans[1]).op).toEqual('http.client');
9694
});
9795

@@ -301,8 +299,6 @@ describe('tracing', () => {
301299

302300
// our span is at index 1 because the transaction itself is at index 0
303301
expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel');
304-
// eslint-disable-next-line deprecation/deprecation
305-
expect(spans[1].op).toEqual('http.client');
306302
expect(spanToJSON(spans[1]).op).toEqual('http.client');
307303

308304
const spanAttributes = spanToJSON(spans[1]).data || {};
@@ -328,8 +324,6 @@ describe('tracing', () => {
328324

329325
// our span is at index 1 because the transaction itself is at index 0
330326
expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel');
331-
// eslint-disable-next-line deprecation/deprecation
332-
expect(spans[1].op).toEqual('http.client');
333327
expect(spanToJSON(spans[1]).op).toEqual('http.client');
334328
expect(spanAttributes['http.method']).toEqual('GET');
335329
expect(spanAttributes.url).toEqual('http://dogs.are.great/spaniel');
@@ -413,8 +407,7 @@ describe('tracing', () => {
413407
const request = http.get(url);
414408

415409
// There should be no http spans
416-
// eslint-disable-next-line deprecation/deprecation
417-
const httpSpans = spans.filter(span => span.op?.startsWith('http'));
410+
const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http'));
418411
expect(httpSpans.length).toBe(0);
419412

420413
// And headers are not attached without span creation
@@ -523,8 +516,7 @@ describe('tracing', () => {
523516
const request = http.get(url);
524517

525518
// There should be no http spans
526-
// eslint-disable-next-line deprecation/deprecation
527-
const httpSpans = spans.filter(span => span.op?.startsWith('http'));
519+
const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http'));
528520
expect(httpSpans.length).toBe(0);
529521

530522
// And headers are not attached without span creation

0 commit comments

Comments
 (0)