Skip to content

Commit

Permalink
feat(v8/core): Remove getters for span.op (#10767)
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Feb 21, 2024
1 parent 15efcea commit c6f520c
Show file tree
Hide file tree
Showing 20 changed files with 69 additions and 171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -210,13 +210,13 @@ export function waitForEnvelopeItem(

export function waitForError(
proxyServerName: string,
callback: (transactionEvent: Event) => Promise<boolean> | boolean,
): Promise<Event> {
callback: (transactionEvent: SerializedEvent) => Promise<boolean> | boolean,
): Promise<SerializedEvent> {
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;
Expand All @@ -226,13 +226,13 @@ export function waitForError(

export function waitForTransaction(
proxyServerName: string,
callback: (transactionEvent: Event) => Promise<boolean> | boolean,
): Promise<Event> {
callback: (transactionEvent: SerializedEvent) => Promise<boolean> | boolean,
): Promise<SerializedEvent> {
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;
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/profiling/utils.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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';
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/bun/test/integrations/bunserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 /');
});

Expand All @@ -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 /');
});

Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/tracing/idletransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/tracing/sampling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,11 @@ export function sampleTransaction<T extends Transaction>(
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;
}

Expand Down
28 changes: 3 additions & 25 deletions packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable max-lines */
import type {
Instrumenter,
Primitive,
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 1 addition & 3 deletions packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,7 @@ export class Transaction extends SentrySpan implements TransactionInterface {
transaction.measurements = this._measurements;
}

// eslint-disable-next-line deprecation/deprecation
DEBUG_BUILD && logger.log(`[Tracing] Finishing ${this.op} transaction: ${this._name}.`);

DEBUG_BUILD && logger.log(`[Tracing] Finishing ${spanToJSON(this).op} transaction: ${this._name}.`);
return transaction;
}
}
47 changes: 1 addition & 46 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,27 +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) {
// eslint-disable-next-line deprecation/deprecation
span.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 => {
Expand All @@ -173,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 () => {
Expand All @@ -197,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 => {
Expand Down
3 changes: 2 additions & 1 deletion packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<string, unknown> = {
procedure_type: type,
Expand Down
12 changes: 2 additions & 10 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down Expand Up @@ -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 || {};
Expand All @@ -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');
Expand Down Expand Up @@ -413,8 +407,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
Expand Down Expand Up @@ -523,8 +516,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
Expand Down
Loading

0 comments on commit c6f520c

Please sign in to comment.