From d4c9473835e3d08b42fa9c84a833ee7c1418fbc4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 08:50:12 -0500 Subject: [PATCH 1/7] feat(v8/core): Remove getters for span.op --- .../event-proxy-server.ts | 18 ++++++------ packages/browser/src/profiling/utils.ts | 4 +-- .../bun/test/integrations/bunserver.test.ts | 4 +-- packages/core/src/tracing/idletransaction.ts | 8 +++--- packages/core/src/tracing/sampling.ts | 8 ++++-- packages/core/src/tracing/sentrySpan.ts | 28 ++----------------- packages/core/src/tracing/transaction.ts | 12 ++++---- packages/core/test/lib/tracing/trace.test.ts | 3 +- packages/node/test/integrations/http.test.ts | 6 ++-- .../test/spanprocessor.test.ts | 3 -- .../src/browser/metrics/index.ts | 3 +- .../test/browser/browsertracing.test.ts | 12 ++++---- .../test/browser/metrics/utils.test.ts | 3 +- packages/tracing/test/span.test.ts | 4 +-- packages/types/src/span.ts | 10 +------ packages/types/src/transaction.ts | 2 +- 16 files changed, 45 insertions(+), 83 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/event-proxy-server.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/event-proxy-server.ts index d14ca5cb5e72..2a4dc2b525d9 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/event-proxy-server.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/event-proxy-server.ts @@ -6,7 +6,7 @@ import * as os from 'os'; import * as path from 'path'; import * as util from 'util'; import * as zlib from 'zlib'; -import type { Envelope, EnvelopeItem, Event } from '@sentry/types'; +import type { Envelope, EnvelopeItem, SerializedEvent } from '@sentry/types'; import { parseEnvelope } from '@sentry/utils'; const readFile = util.promisify(fs.readFile); @@ -210,13 +210,13 @@ export function waitForEnvelopeItem( export function waitForError( proxyServerName: string, - callback: (transactionEvent: Event) => Promise | boolean, -): Promise { + callback: (transactionEvent: SerializedEvent) => Promise | boolean, +): Promise { return new Promise((resolve, reject) => { waitForEnvelopeItem(proxyServerName, async envelopeItem => { const [envelopeItemHeader, envelopeItemBody] = envelopeItem; - if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as Event))) { - resolve(envelopeItemBody as Event); + if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as SerializedEvent))) { + resolve(envelopeItemBody as SerializedEvent); return true; } return false; @@ -226,13 +226,13 @@ export function waitForError( export function waitForTransaction( proxyServerName: string, - callback: (transactionEvent: Event) => Promise | boolean, -): Promise { + callback: (transactionEvent: SerializedEvent) => Promise | boolean, +): Promise { return new Promise((resolve, reject) => { waitForEnvelopeItem(proxyServerName, async envelopeItem => { const [envelopeItemHeader, envelopeItemBody] = envelopeItem; - if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as Event))) { - resolve(envelopeItemBody as Event); + if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as SerializedEvent))) { + resolve(envelopeItemBody as SerializedEvent); return true; } return false; diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts index 9114884384b7..f4f700a2ab6f 100644 --- a/packages/browser/src/profiling/utils.ts +++ b/packages/browser/src/profiling/utils.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ -import { DEFAULT_ENVIRONMENT, getClient } from '@sentry/core'; +import { DEFAULT_ENVIRONMENT, getClient, spanToJSON } from '@sentry/core'; import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types'; import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling'; import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils'; @@ -202,7 +202,7 @@ export function isProfiledTransactionEvent(event: Event): event is ProfiledEvent * */ export function isAutomatedPageLoadTransaction(transaction: Transaction): boolean { - return transaction.op === 'pageload'; + return spanToJSON(transaction).op === 'pageload'; } /** diff --git a/packages/bun/test/integrations/bunserver.test.ts b/packages/bun/test/integrations/bunserver.test.ts index b51fb649fd25..c6bcf62de229 100644 --- a/packages/bun/test/integrations/bunserver.test.ts +++ b/packages/bun/test/integrations/bunserver.test.ts @@ -29,7 +29,7 @@ describe('Bun Serve Integration', () => { expect(transaction.tags).toEqual({ 'http.status_code': '200', }); - expect(transaction.op).toEqual('http.server'); + expect(spanToJSON(transaction).op).toEqual('http.server'); expect(spanToJSON(transaction).description).toEqual('GET /'); }); @@ -52,7 +52,7 @@ describe('Bun Serve Integration', () => { expect(transaction.tags).toEqual({ 'http.status_code': '200', }); - expect(transaction.op).toEqual('http.server'); + expect(spanToJSON(transaction).op).toEqual('http.server'); expect(spanToJSON(transaction).description).toEqual('POST /'); }); diff --git a/packages/core/src/tracing/idletransaction.ts b/packages/core/src/tracing/idletransaction.ts index 3e9381edc83b..7fae765c60df 100644 --- a/packages/core/src/tracing/idletransaction.ts +++ b/packages/core/src/tracing/idletransaction.ts @@ -162,16 +162,16 @@ export class IdleTransaction extends Transaction { this._finished = true; this.activities = {}; - // eslint-disable-next-line deprecation/deprecation - if (this.op === 'ui.action.click') { + const op = spanToJSON(this).op; + + if (op === 'ui.action.click') { this.setAttribute(FINISH_REASON_TAG, this._finishReason); } // eslint-disable-next-line deprecation/deprecation if (this.spanRecorder) { DEBUG_BUILD && - // eslint-disable-next-line deprecation/deprecation - logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), this.op); + logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), op); for (const callback of this._beforeFinishCallbacks) { callback(this, endTimestampInS); diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index 427a7076d6d0..6bae27ffe1a1 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -94,9 +94,11 @@ export function sampleTransaction( return transaction; } - DEBUG_BUILD && - // eslint-disable-next-line deprecation/deprecation - logger.log(`[Tracing] starting ${transaction.op} transaction - ${spanToJSON(transaction).description}`); + if (DEBUG_BUILD) { + const { op, description } = spanToJSON(transaction); + logger.log(`[Tracing] starting ${op} transaction - ${description}`); + } + return transaction; } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index b4566a55e399..904136d3d04d 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -1,4 +1,3 @@ -/* eslint-disable max-lines */ import type { Instrumenter, Primitive, @@ -296,25 +295,6 @@ export class SentrySpan implements SpanInterface { this._status = status; } - /** - * Operation of the span - * - * @deprecated Use `spanToJSON().op` to read the op instead. - */ - public get op(): string | undefined { - return this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined; - } - - /** - * Operation of the span - * - * @deprecated Use `startSpan()` functions to set or `span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'op') - * to update the span instead. - */ - public set op(op: string | undefined) { - this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op); - } - /* eslint-enable @typescript-eslint/member-ordering */ /** @inheritdoc */ @@ -493,8 +473,7 @@ export class SentrySpan implements SpanInterface { data: this._getData(), name: this._name, endTimestamp: this._endTime, - // eslint-disable-next-line deprecation/deprecation - op: this.op, + op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP], parentSpanId: this._parentSpanId, sampled: this._sampled, spanId: this._spanId, @@ -516,8 +495,7 @@ export class SentrySpan implements SpanInterface { this.data = spanContext.data || {}; this._name = spanContext.name; this._endTime = spanContext.endTimestamp; - // eslint-disable-next-line deprecation/deprecation - this.op = spanContext.op; + this._attributes = { ...this._attributes, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: spanContext.op }; this._parentSpanId = spanContext.parentSpanId; this._sampled = spanContext.sampled; this._spanId = spanContext.spanId || this._spanId; @@ -551,7 +529,7 @@ export class SentrySpan implements SpanInterface { return dropUndefinedKeys({ data: this._getData(), description: this._name, - op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined, + op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP], parent_span_id: this._parentSpanId, span_id: this._spanId, start_timestamp: this._startTime, diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 8b3f5e62288d..348405d6c12a 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -314,18 +314,16 @@ export class Transaction extends SentrySpan implements TransactionInterface { const hasMeasurements = Object.keys(this._measurements).length > 0; - if (hasMeasurements) { - DEBUG_BUILD && + if (DEBUG_BUILD) { + if (hasMeasurements) { logger.log( '[Measurements] Adding measurements to transaction', JSON.stringify(this._measurements, undefined, 2), ); - transaction.measurements = this._measurements; - } - - // eslint-disable-next-line deprecation/deprecation - DEBUG_BUILD && logger.log(`[Tracing] Finishing ${this.op} transaction: ${this._name}.`); + } + logger.log(`[Tracing] Finishing ${spanToJSON(this).op} transaction: ${this._name}.`); + } return transaction; } } diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index a260b8854cd0..c9c0b08cf1d7 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -145,8 +145,7 @@ describe('startSpan', () => { try { await startSpan({ name: 'GET users/[id]' }, span => { if (span) { - // eslint-disable-next-line deprecation/deprecation - span.op = 'http.server'; + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); } return callback(); }); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 6dbc6b9c8b62..a7c436ba92a9 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -413,8 +413,7 @@ describe('tracing', () => { const request = http.get(url); // There should be no http spans - // eslint-disable-next-line deprecation/deprecation - const httpSpans = spans.filter(span => span.op?.startsWith('http')); + const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http')); expect(httpSpans.length).toBe(0); // And headers are not attached without span creation @@ -523,8 +522,7 @@ describe('tracing', () => { const request = http.get(url); // There should be no http spans - // eslint-disable-next-line deprecation/deprecation - const httpSpans = spans.filter(span => span.op?.startsWith('http')); + const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http')); expect(httpSpans.length).toBe(0); // And headers are not attached without span creation diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index b5e2a26fa4c2..d5663530122e 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -832,10 +832,7 @@ describe('SentrySpanProcessor', () => { parentOtelSpan.setAttribute(SemanticAttributes.FAAS_TRIGGER, 'test faas trigger'); parentOtelSpan.end(); - // eslint-disable-next-line deprecation/deprecation - expect(transaction.op).toBe('test faas trigger'); expect(spanToJSON(transaction).op).toBe('test faas trigger'); - expect(spanToJSON(transaction).description).toBe('test operation'); }); }); diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index 114ac74bd4e9..9bd3a19e770d 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -194,8 +194,7 @@ export function addPerformanceEntries(transaction: Transaction): void { const startTime = msToSec(entry.startTime); const duration = msToSec(entry.duration); - // eslint-disable-next-line deprecation/deprecation - if (transaction.op === 'navigation' && transactionStartTime && timeOrigin + startTime < transactionStartTime) { + if (op === 'navigation' && transactionStartTime && timeOrigin + startTime < transactionStartTime) { return; } diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index f57c3c78f1ec..1b819a4e7e09 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -176,7 +176,7 @@ describe('BrowserTracing', () => { const transaction = getActiveTransaction() as IdleTransaction; expect(transaction).toBeDefined(); expect(spanToJSON(transaction).description).toBe('a/path'); - expect(transaction.op).toBe('pageload'); + expect(spanToJSON(transaction).op).toBe('pageload'); }); it('trims all transactions', () => { @@ -230,7 +230,7 @@ describe('BrowserTracing', () => { }); const transaction = getActiveTransaction() as IdleTransaction; expect(transaction).toBeDefined(); - expect(transaction.op).toBe('not-pageload'); + expect(spanToJSON(transaction).op).toBe('not-pageload'); expect(mockBeforeNavigation).toHaveBeenCalledTimes(1); }); @@ -373,7 +373,7 @@ describe('BrowserTracing', () => { const transaction = getActiveTransaction() as IdleTransaction; expect(transaction).toBeDefined(); - expect(transaction.op).toBe('pageload'); + expect(spanToJSON(transaction).op).toBe('pageload'); }); it('is not created if the option is false', () => { @@ -480,7 +480,7 @@ describe('BrowserTracing', () => { const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; expect(transaction).toBeDefined(); - expect(transaction.op).toBe('pageload'); + expect(spanToJSON(transaction).op).toBe('pageload'); expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toBe(false); @@ -500,7 +500,7 @@ describe('BrowserTracing', () => { const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; expect(transaction).toBeDefined(); - expect(transaction.op).toBe('pageload'); + expect(spanToJSON(transaction).op).toBe('pageload'); expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toBe(false); @@ -520,7 +520,7 @@ describe('BrowserTracing', () => { const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; expect(transaction).toBeDefined(); - expect(transaction.op).toBe('navigation'); + expect(spanToJSON(transaction).op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); expect(dynamicSamplingContext).toStrictEqual({ diff --git a/packages/tracing-internal/test/browser/metrics/utils.test.ts b/packages/tracing-internal/test/browser/metrics/utils.test.ts index a7de3e37acf8..9cbae997cd7e 100644 --- a/packages/tracing-internal/test/browser/metrics/utils.test.ts +++ b/packages/tracing-internal/test/browser/metrics/utils.test.ts @@ -12,8 +12,7 @@ describe('_startChild()', () => { expect(span).toBeInstanceOf(SentrySpan); expect(spanToJSON(span).description).toBe('evaluation'); - // eslint-disable-next-line deprecation/deprecation - expect(span.op).toBe('script'); + expect(spanToJSON(span).op).toBe('script'); expect(spanToJSON(span).op).toBe('script'); }); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 03b286b23c12..483cc5cb3987 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -507,7 +507,7 @@ describe('SentrySpan', () => { expect(span.spanContext().spanId).toBe('d'); expect(span.sampled).toBe(true); expect(spanToJSON(span).description).toBe(undefined); - expect(span.op).toBe(undefined); + expect(spanToJSON(span).op).toBe(undefined); expect(span.tags).toStrictEqual({}); }); @@ -545,7 +545,7 @@ describe('SentrySpan', () => { expect(span.spanContext().spanId).toBe('b'); expect(spanToJSON(span).description).toBe('new'); expect(spanToJSON(span).timestamp).toBe(1); - expect(span.op).toBe('new-op'); + expect(spanToJSON(span).op).toBe('new-op'); expect(span.sampled).toBe(true); expect(span.tags).toStrictEqual({ tag1: 'bye' }); expect(span.data).toStrictEqual({ diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 2e6e0b084900..bb4a97dea860 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -174,15 +174,7 @@ export interface SpanContext { } /** Span holding trace_id, span_id */ -export interface Span extends Omit { - /** - * Operation of the Span. - * - * @deprecated Use `startSpan()` functions to set, `span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'op') - * to update and `spanToJSON().op` to read the op instead - */ - op?: string | undefined; - +export interface Span extends Omit { /** * The ID of the span. * @deprecated Use `spanContext().spanId` instead. diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index f129cfebb3de..11bcdf290a29 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -43,7 +43,7 @@ export type TraceparentData = Pick, Span { +export interface Transaction extends Omit, Span { /** * The ID of the transaction. * @deprecated Use `spanContext().spanId` instead. From e291f97d1c69a2e93cff598690fb9186b46c1292 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 09:43:35 -0500 Subject: [PATCH 2/7] remaining fixes --- packages/node/src/handlers.ts | 3 ++- .../src/browser/browserTracingIntegration.ts | 15 +++++++++------ .../src/browser/browsertracing.ts | 17 ++++++++++------- packages/tracing-internal/src/browser/router.ts | 5 +++-- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 8e1cb695c05b..b66315afcb73 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,6 +1,7 @@ import type * as http from 'http'; /* eslint-disable @typescript-eslint/no-explicit-any */ import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureException, continueTrace, @@ -306,7 +307,7 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) { if (sentryTransaction) { sentryTransaction.updateName(`trpc/${path}`); sentryTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); - sentryTransaction.op = 'rpc.server'; + sentryTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'rpc.server'); const trpcContext: Record = { procedure_type: type, diff --git a/packages/tracing-internal/src/browser/browserTracingIntegration.ts b/packages/tracing-internal/src/browser/browserTracingIntegration.ts index ad70af5aa666..82de3577a6da 100644 --- a/packages/tracing-internal/src/browser/browserTracingIntegration.ts +++ b/packages/tracing-internal/src/browser/browserTracingIntegration.ts @@ -433,12 +433,15 @@ function registerInteractionListener( // eslint-disable-next-line deprecation/deprecation const currentTransaction = getActiveTransaction(); - if (currentTransaction && currentTransaction.op && ['navigation', 'pageload'].includes(currentTransaction.op)) { - DEBUG_BUILD && - logger.warn( - `[Tracing] Did not create ${op} transaction because a pageload or navigation transaction is in progress.`, - ); - return undefined; + if (currentTransaction) { + const currentTransactionOp = spanToJSON(currentTransaction).op; + if (currentTransactionOp && ['navigation', 'pageload'].includes(currentTransactionOp)) { + DEBUG_BUILD && + logger.warn( + `[Tracing] Did not create ${op} transaction because a pageload or navigation transaction is in progress.`, + ); + return undefined; + } } if (inflightInteractionTransaction) { diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 2aceb97bd6a1..300bc5895d16 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -1,5 +1,5 @@ -/* eslint-disable max-lines */ import type { Hub, IdleTransaction } from '@sentry/core'; +import { spanToJSON } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, TRACING_DEFAULTS, @@ -389,12 +389,15 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line deprecation/deprecation const currentTransaction = getActiveTransaction(); - if (currentTransaction && currentTransaction.op && ['navigation', 'pageload'].includes(currentTransaction.op)) { - DEBUG_BUILD && - logger.warn( - `[Tracing] Did not create ${op} transaction because a pageload or navigation transaction is in progress.`, - ); - return undefined; + if (currentTransaction) { + const currentTransactionOp = spanToJSON(currentTransaction).op; + if (currentTransactionOp && ['navigation', 'pageload'].includes(currentTransactionOp)) { + DEBUG_BUILD && + logger.warn( + `[Tracing] Did not create ${op} transaction because a pageload or navigation transaction is in progress.`, + ); + return undefined; + } } if (inflightInteractionTransaction) { diff --git a/packages/tracing-internal/src/browser/router.ts b/packages/tracing-internal/src/browser/router.ts index 895a37c60ff1..ff08af13a0cd 100644 --- a/packages/tracing-internal/src/browser/router.ts +++ b/packages/tracing-internal/src/browser/router.ts @@ -1,4 +1,4 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; import type { Transaction, TransactionContext } from '@sentry/types'; import { addHistoryInstrumentationHandler, browserPerformanceTimeOrigin, logger } from '@sentry/utils'; @@ -53,7 +53,8 @@ export function instrumentRoutingWithDefaults( if (from !== to) { startingUrl = undefined; if (activeTransaction) { - DEBUG_BUILD && logger.log(`[Tracing] Finishing current transaction with op: ${activeTransaction.op}`); + DEBUG_BUILD && + logger.log(`[Tracing] Finishing current transaction with op: ${spanToJSON(activeTransaction).op}`); // If there's an open transaction on the scope, we need to finish it before creating an new one. activeTransaction.end(); } From b1efacf1882a86412ea45a0b93337389fd2f90fb Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 10:35:36 -0500 Subject: [PATCH 3/7] more op fixes --- packages/core/test/lib/tracing/trace.test.ts | 44 ------------------- packages/node/test/integrations/http.test.ts | 6 --- .../test/browser/browsertracing.test.ts | 8 ++-- 3 files changed, 4 insertions(+), 54 deletions(-) diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index c9c0b08cf1d7..df1b1cf5b5bb 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -136,26 +136,6 @@ describe('startSpan', () => { expect(ref.parentSpanId).toEqual('1234567890123456'); }); - // TODO (v8): Remove this test in favour of the one below - it('(deprecated op) allows for transaction to be mutated', async () => { - let ref: any = undefined; - client.on('finishTransaction', transaction => { - ref = transaction; - }); - try { - await startSpan({ name: 'GET users/[id]' }, span => { - if (span) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); - } - return callback(); - }); - } catch (e) { - // - } - - expect(spanToJSON(ref).op).toEqual('http.server'); - }); - it('allows for transaction to be mutated', async () => { let ref: any = undefined; client.on('finishTransaction', transaction => { @@ -196,30 +176,6 @@ describe('startSpan', () => { expect(ref.spanRecorder.spans[1].status).toEqual(isError ? 'internal_error' : undefined); }); - // TODO (v8): Remove this test in favour of the one below - it('(deprecated op) allows for span to be mutated', async () => { - let ref: any = undefined; - client.on('finishTransaction', transaction => { - ref = transaction; - }); - try { - await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => { - return startSpan({ name: 'SELECT * from users' }, childSpan => { - if (childSpan) { - // eslint-disable-next-line deprecation/deprecation - childSpan.op = 'db.query'; - } - return callback(); - }); - }); - } catch (e) { - // - } - - expect(ref.spanRecorder.spans).toHaveLength(2); - expect(ref.spanRecorder.spans[1].op).toEqual('db.query'); - }); - it('allows for span to be mutated', async () => { let ref: any = undefined; client.on('finishTransaction', transaction => { diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index a7c436ba92a9..63559d9102d7 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -90,8 +90,6 @@ describe('tracing', () => { // our span is at index 1 because the transaction itself is at index 0 expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/'); - // eslint-disable-next-line deprecation/deprecation - expect(spans[1].op).toEqual('http.client'); expect(spanToJSON(spans[1]).op).toEqual('http.client'); }); @@ -301,8 +299,6 @@ describe('tracing', () => { // our span is at index 1 because the transaction itself is at index 0 expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel'); - // eslint-disable-next-line deprecation/deprecation - expect(spans[1].op).toEqual('http.client'); expect(spanToJSON(spans[1]).op).toEqual('http.client'); const spanAttributes = spanToJSON(spans[1]).data || {}; @@ -328,8 +324,6 @@ describe('tracing', () => { // our span is at index 1 because the transaction itself is at index 0 expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel'); - // eslint-disable-next-line deprecation/deprecation - expect(spans[1].op).toEqual('http.client'); expect(spanToJSON(spans[1]).op).toEqual('http.client'); expect(spanAttributes['http.method']).toEqual('GET'); expect(spanAttributes.url).toEqual('http://dogs.are.great/spaniel'); diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index 1b819a4e7e09..12e455d13e8e 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -399,12 +399,12 @@ describe('BrowserTracing', () => { it('is created on location change', () => { createBrowserTracing(true); const transaction1 = getActiveTransaction() as IdleTransaction; - expect(transaction1.op).toBe('pageload'); + expect(spanToJSON(transaction1).op).toBe('pageload'); expect(spanToJSON(transaction1).timestamp).not.toBeDefined(); mockChangeHistory({ to: 'here', from: 'there' }); const transaction2 = getActiveTransaction() as IdleTransaction; - expect(transaction2.op).toBe('navigation'); + expect(spanToJSON(transaction2).op).toBe('navigation'); expect(spanToJSON(transaction1).timestamp).toBeDefined(); }); @@ -412,12 +412,12 @@ describe('BrowserTracing', () => { it('is not created if startTransactionOnLocationChange is false', () => { createBrowserTracing(true, { startTransactionOnLocationChange: false }); const transaction1 = getActiveTransaction() as IdleTransaction; - expect(transaction1.op).toBe('pageload'); + expect(spanToJSON(transaction1).op).toBe('pageload'); expect(spanToJSON(transaction1).timestamp).not.toBeDefined(); mockChangeHistory({ to: 'here', from: 'there' }); const transaction2 = getActiveTransaction() as IdleTransaction; - expect(transaction2.op).toBe('pageload'); + expect(spanToJSON(transaction2).op).toBe('pageload'); }); }); }); From e652051e33246a692b95de04ebe74768ff12fe63 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 10:53:08 -0500 Subject: [PATCH 4/7] more op fixes again --- packages/browser/test/unit/profiling/integration.test.ts | 4 +++- packages/core/test/lib/tracing/trace.test.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/browser/test/unit/profiling/integration.test.ts b/packages/browser/test/unit/profiling/integration.test.ts index 9394221b0e4b..1211c95522a3 100644 --- a/packages/browser/test/unit/profiling/integration.test.ts +++ b/packages/browser/test/unit/profiling/integration.test.ts @@ -1,4 +1,5 @@ import type { BrowserClient } from '@sentry/browser'; +import { spanToJSON } from '@sentry/browser'; import * as Sentry from '@sentry/browser'; import type { JSSelfProfile } from '../../../src/profiling/jsSelfProfiling'; @@ -50,7 +51,8 @@ describe('BrowserProfilingIntegration', () => { // eslint-disable-next-line deprecation/deprecation const currentTransaction = Sentry.getCurrentScope().getTransaction(); - expect(currentTransaction?.op).toBe('pageload'); + expect(currentTransaction).toBeDefined(); + expect(spanToJSON(currentTransaction as any)?.op).toBe('pageload'); currentTransaction?.end(); await client?.flush(1000); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index df1b1cf5b5bb..d74d95824c9f 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -152,7 +152,7 @@ describe('startSpan', () => { // } - expect(ref.op).toEqual('http.server'); + expect(spanToJSON(ref).op).toEqual('http.server'); }); it('creates a span with correct description', async () => { From 5c68ced638c2be326d6608fac2e5ced41a3523bc Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 11:02:15 -0500 Subject: [PATCH 5/7] more span test changes: --- .../test/spanprocessor.test.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index d5663530122e..83a534f16baf 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -486,15 +486,11 @@ describe('SentrySpanProcessor', () => { child.updateName('new name'); - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe(undefined); expect(sentrySpan && spanToJSON(sentrySpan).op).toBe(undefined); expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe('SELECT * FROM users;'); child.end(); - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe(undefined); expect(sentrySpan && spanToJSON(sentrySpan).op).toBe(undefined); expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe('new name'); @@ -514,8 +510,6 @@ describe('SentrySpanProcessor', () => { child.end(); - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('http.client'); expect(spanToJSON(sentrySpan!).op).toBe('http.client'); parentOtelSpan.end(); @@ -534,8 +528,6 @@ describe('SentrySpanProcessor', () => { child.end(); - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('http.server'); expect(spanToJSON(sentrySpan!).op).toBe('http.server'); parentOtelSpan.end(); @@ -721,9 +713,6 @@ describe('SentrySpanProcessor', () => { child.end(); const { description, op } = spanToJSON(sentrySpan!); - - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('db'); expect(op).toBe('db'); expect(description).toBe('SELECT * FROM users'); @@ -744,9 +733,6 @@ describe('SentrySpanProcessor', () => { child.end(); const { description, op } = spanToJSON(sentrySpan!); - - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('db'); expect(op).toBe('db'); expect(description).toBe('fetch users from DB'); @@ -767,8 +753,6 @@ describe('SentrySpanProcessor', () => { child.end(); const { op, description } = spanToJSON(sentrySpan!); - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('rpc'); expect(op).toBe('rpc'); expect(description).toBe('test operation'); @@ -789,9 +773,6 @@ describe('SentrySpanProcessor', () => { child.end(); const { op, description } = spanToJSON(sentrySpan!); - - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('message'); expect(op).toBe('message'); expect(description).toBe('test operation'); @@ -812,9 +793,6 @@ describe('SentrySpanProcessor', () => { child.end(); const { op, description } = spanToJSON(sentrySpan!); - - // eslint-disable-next-line deprecation/deprecation - expect(sentrySpan?.op).toBe('test faas trigger'); expect(op).toBe('test faas trigger'); expect(description).toBe('test operation'); From ae28ccb6a2aa5e1bc66b78c52ff1d72d73499e59 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 11:11:26 -0500 Subject: [PATCH 6/7] woops bad refactor --- packages/core/src/tracing/transaction.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 348405d6c12a..903554b08c9b 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -314,16 +314,16 @@ export class Transaction extends SentrySpan implements TransactionInterface { const hasMeasurements = Object.keys(this._measurements).length > 0; - if (DEBUG_BUILD) { - if (hasMeasurements) { + if (hasMeasurements) { + DEBUG_BUILD && logger.log( '[Measurements] Adding measurements to transaction', JSON.stringify(this._measurements, undefined, 2), ); - } - - logger.log(`[Tracing] Finishing ${spanToJSON(this).op} transaction: ${this._name}.`); + transaction.measurements = this._measurements; } + + DEBUG_BUILD && logger.log(`[Tracing] Finishing ${spanToJSON(this).op} transaction: ${this._name}.`); return transaction; } } From 1da3927f783239091bdd43543688f7c55948e803 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 21 Feb 2024 11:57:20 -0500 Subject: [PATCH 7/7] lint fix --- packages/browser/test/unit/profiling/integration.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser/test/unit/profiling/integration.test.ts b/packages/browser/test/unit/profiling/integration.test.ts index c679279e1f97..fe95cd5ec83c 100644 --- a/packages/browser/test/unit/profiling/integration.test.ts +++ b/packages/browser/test/unit/profiling/integration.test.ts @@ -1,5 +1,4 @@ import type { BrowserClient } from '@sentry/browser'; -import { spanToJSON } from '@sentry/browser'; import * as Sentry from '@sentry/browser'; import type { JSSelfProfile } from '../../../src/profiling/jsSelfProfiling';