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

feat(v8): Remove deprecated span id fields #11180

Merged
merged 4 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ app.use(cors());
app.get('/test/express', (_req, res) => {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.getCurrentScope().getTransaction();
if (transaction) {
// eslint-disable-next-line deprecation/deprecation
transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
}
const traceId = transaction?.spanContext().traceId;
const headers = http.get('http://somewhere.not.sentry/').getHeaders();

if (traceId) {
headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__');
}
// Responding with the headers outgoing request headers back to the assertions.
res.send({ test_data: headers });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test('should attach a baggage header to an outgoing request.', async () => {
host: 'somewhere.not.sentry',
baggage:
'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public' +
',sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' +
',sentry-trace_id=__SENTRY_TRACE_ID__,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' +
',sentry-sampled=true',
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,9 @@ app.use(cors());
app.get('/test/express', (_req, res) => {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.getCurrentScope().getTransaction();
if (transaction) {
// eslint-disable-next-line deprecation/deprecation
transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
}
const headers = http.get('http://somewhere.not.sentry/').getHeaders();
transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');

const headers = http.get('http://somewhere.not.sentry/').getHeaders();
// Responding with the headers outgoing request headers back to the assertions.
res.send({ test_data: headers });
});
Expand Down
50 changes: 0 additions & 50 deletions packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,56 +97,6 @@ export class SentrySpan implements Span {
// This rule conflicts with another eslint rule :(
/* eslint-disable @typescript-eslint/member-ordering */

/**
* The ID of the trace.
* @deprecated Use `spanContext().traceId` instead.
*/
public get traceId(): string {
return this._traceId;
}

/**
* The ID of the trace.
* @deprecated You cannot update the traceId of a span after span creation.
*/
public set traceId(traceId: string) {
this._traceId = traceId;
}

/**
* The ID of the span.
* @deprecated Use `spanContext().spanId` instead.
*/
public get spanId(): string {
return this._spanId;
}

/**
* The ID of the span.
* @deprecated You cannot update the spanId of a span after span creation.
*/
public set spanId(spanId: string) {
this._spanId = spanId;
}

/**
* @inheritDoc
*
* @deprecated Use `startSpan` functions instead.
*/
public set parentSpanId(string) {
this._parentSpanId = string;
}

/**
* @inheritDoc
*
* @deprecated Use `spanToJSON(span).parent_span_id` instead.
*/
public get parentSpanId(): string | undefined {
return this._parentSpanId;
}

/**
* Was this span chosen to be sent as part of the sample?
* @deprecated Use `isRecording()` instead.
Expand Down
27 changes: 19 additions & 8 deletions packages/core/test/lib/tracing/sentrySpan.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { timestampInSeconds } from '@sentry/utils';
import { SentrySpan } from '../../../src/tracing/sentrySpan';
import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus';
import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON, spanToTraceContext } from '../../../src/utils/spanUtils';
import {
TRACE_FLAG_NONE,
TRACE_FLAG_SAMPLED,
spanIsSampled,
spanToJSON,
spanToTraceContext,
} from '../../../src/utils/spanUtils';

describe('SentrySpan', () => {
describe('name', () => {
Expand All @@ -25,9 +31,9 @@ describe('SentrySpan', () => {
const span = new SentrySpan({ sampled: true });
// eslint-disable-next-line deprecation/deprecation
const span2 = span.startChild();
expect((span2 as any).parentSpanId).toBe((span as any).spanId);
expect((span2 as any).traceId).toBe((span as any).traceId);
expect((span2 as any).sampled).toBe((span as any).sampled);
expect(spanToJSON(span2).parent_span_id).toBe(span.spanContext().spanId);
expect(span.spanContext().traceId).toBe(span.spanContext().traceId);
expect(spanIsSampled(span2)).toBe(spanIsSampled(span));
});
});

Expand Down Expand Up @@ -58,18 +64,23 @@ describe('SentrySpan', () => {
});

test('with parent', () => {
const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any;
const spanB = new SentrySpan({ traceId: 'c', spanId: 'd', sampled: false, parentSpanId: spanA.spanId });
const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' });
const spanB = new SentrySpan({
traceId: 'c',
spanId: 'd',
sampled: false,
parentSpanId: spanA.spanContext().spanId,
});
const serialized = spanToJSON(spanB);
expect(serialized).toHaveProperty('parent_span_id', 'b');
expect(serialized).toHaveProperty('span_id', 'd');
expect(serialized).toHaveProperty('trace_id', 'c');
});

test('should drop all `undefined` values', () => {
const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any;
const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' });
const spanB = new SentrySpan({
parentSpanId: spanA.spanId,
parentSpanId: spanA.spanContext().spanId,
spanId: 'd',
traceId: 'c',
});
Expand Down
30 changes: 17 additions & 13 deletions packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getMainCarrier,
mergeScopeData,
setCurrentClient,
spanIsSampled,
spanToJSON,
withScope,
} from '@sentry/core';
Expand Down Expand Up @@ -289,7 +290,7 @@ describe('tracingHandler', () => {

sentryTracingMiddleware(req, res, next);

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

expect(getPropagationContext()).toEqual({
traceId: '12312012123120121231201212312012',
Expand All @@ -300,9 +301,10 @@ describe('tracingHandler', () => {
});

// since we have no tracesSampler defined, the default behavior (inherit if possible) applies
expect(transaction.traceId).toEqual('12312012123120121231201212312012');
expect(transaction.parentSpanId).toEqual('1121201211212012');
expect(transaction.sampled).toEqual(false);
expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012');
expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012');
expect(spanIsSampled(transaction)).toEqual(false);
// eslint-disable-next-line deprecation/deprecation
expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({});
});

Expand All @@ -322,12 +324,13 @@ describe('tracingHandler', () => {
dsc: { version: '1.0', environment: 'production' },
});

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

// since we have no tracesSampler defined, the default behavior (inherit if possible) applies
expect(transaction.traceId).toEqual('12312012123120121231201212312012');
expect(transaction.parentSpanId).toEqual('1121201211212012');
expect(transaction.sampled).toEqual(true);
expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012');
expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012');
expect(spanIsSampled(transaction)).toEqual(true);
// eslint-disable-next-line deprecation/deprecation
expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
});

Expand All @@ -341,7 +344,8 @@ describe('tracingHandler', () => {

expect(getPropagationContext().dsc).toEqual({ version: '1.0', environment: 'production' });

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;
// eslint-disable-next-line deprecation/deprecation
expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
});

Expand All @@ -364,7 +368,7 @@ describe('tracingHandler', () => {
it('puts its transaction on the response object', () => {
sentryTracingMiddleware(req, res, next);

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

expect(transaction).toBeDefined();

Expand Down Expand Up @@ -396,7 +400,7 @@ describe('tracingHandler', () => {

sentryTracingMiddleware(req, res, next);

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
});
Expand All @@ -406,7 +410,7 @@ describe('tracingHandler', () => {

sentryTracingMiddleware(req, res, next);

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
});
Expand All @@ -416,7 +420,7 @@ describe('tracingHandler', () => {

sentryTracingMiddleware(req, res, next);

const transaction = (res as any).__sentry_transaction;
const transaction = (res as any).__sentry_transaction as Transaction;

expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
});
Expand Down
3 changes: 1 addition & 2 deletions packages/types/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ import type { Profile } from './profiling';
import type { ReplayEvent, ReplayRecordingData } from './replay';
import type { SdkInfo } from './sdkinfo';
import type { SerializedSession, Session, SessionAggregates } from './session';
import type { Transaction } from './transaction';

// Based on: https://develop.sentry.dev/sdk/envelopes/

// Based on https://github.com/getsentry/relay/blob/b23b8d3b2360a54aaa4d19ecae0231201f31df5e/relay-sampling/src/lib.rs#L685-L707
export type DynamicSamplingContext = {
trace_id: Transaction['traceId'];
trace_id: string;
public_key: DsnComponents['publicKey'];
sample_rate?: string;
release?: string;
Expand Down
14 changes: 1 addition & 13 deletions packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,7 @@ export interface TraceparentData {
/**
* Transaction "Class", inherits Span only has `setName`
*/
export interface Transaction extends Omit<TransactionContext, 'name' | 'op'>, Span {
/**
* The ID of the transaction.
* @deprecated Use `spanContext().spanId` instead.
*/
spanId: string;

/**
* The ID of the trace.
* @deprecated Use `spanContext().traceId` instead.
*/
traceId: string;

export interface Transaction extends Omit<TransactionContext, 'name' | 'op' | 'spanId' | 'traceId'>, Span {
/**
* Was this transaction chosen to be sent as part of the sample?
* @deprecated Use `spanIsSampled(transaction)` instead.
Expand Down