Skip to content

Commit

Permalink
feat(tracing|core): Remove transaction name change recording (#7197)
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad authored Feb 16, 2023
1 parent 4c07d02 commit 5359ba9
Show file tree
Hide file tree
Showing 23 changed files with 2 additions and 307 deletions.
9 changes: 0 additions & 9 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
processedEvent.transaction_info = {
...transactionInfo,
source,
changes: [
...transactionInfo.changes,
{
source,
// use the same timestamp as the processed event.
timestamp: processedEvent.timestamp as number,
propagations: transactionInfo.propagations,
},
],
};
}

Expand Down
20 changes: 0 additions & 20 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1368,8 +1368,6 @@ describe('BaseClient', () => {
type: 'transaction',
transaction_info: {
source: 'url',
changes: [],
propagations: 3,
},
};

Expand All @@ -1383,14 +1381,6 @@ describe('BaseClient', () => {
expect(TestClient.instance!.event!.transaction).toEqual('/adopt/dont/shop');
expect(TestClient.instance!.event!.transaction_info).toEqual({
source: 'custom',
changes: [
{
propagations: 3,
source: 'custom',
timestamp: expect.any(Number),
},
],
propagations: 3,
});
});

Expand All @@ -1407,23 +1397,13 @@ describe('BaseClient', () => {
type: 'transaction',
transaction_info: {
source: 'url',
changes: [],
propagations: 3,
},
});

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop');
expect(TestClient.instance!.event!.transaction_info).toEqual({
source: 'custom',
changes: [
{
propagations: 3,
source: 'custom',
timestamp: expect.any(Number),
},
],
propagations: 3,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ describe('Error Server-side Props', () => {
transaction: '/withErrorServerSideProps',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ describe('Tracing 200', () => {
transaction: 'GET /api/users',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ describe('Tracing 500', () => {
transaction: 'GET /api/broken',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ describe('Tracing HTTP', () => {
transaction: 'GET /api/http',
transaction_info: {
source: 'route',
changes: [],
propagations: 1,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ describe('getInitialProps', () => {
transaction: '/[id]/withInitialProps',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ describe('getServerSideProps', () => {
transaction: '/[id]/withServerSideProps',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ describe('tracingServerGetServerSidePropsCustomPageExtension', () => {
transaction: '/customPageExtension',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ describe('getServerSideProps', () => {
transaction: `GET ${transactionName}`,
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
type: 'transaction',
request: {
Expand Down
24 changes: 0 additions & 24 deletions packages/node-integration-tests/suites/express/tracing/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ test('should set a correct transaction name for routes specified in RegEx', asyn
transaction: 'GET /\\/test\\/regex/',
transaction_info: {
source: 'route',
changes: [
{
propagations: 0,
source: 'url',
timestamp: expect.any(Number),
},
],
propagations: 0,
},
contexts: {
trace: {
Expand Down Expand Up @@ -74,14 +66,6 @@ test.each([['array1'], ['array5']])(
transaction: 'GET /test/array1,/\\/test\\/array[2-9]',
transaction_info: {
source: 'route',
changes: [
{
propagations: 0,
source: 'url',
timestamp: expect.any(Number),
},
],
propagations: 0,
},
contexts: {
trace: {
Expand Down Expand Up @@ -118,14 +102,6 @@ test.each([
transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?',
transaction_info: {
source: 'route',
changes: [
{
propagations: 0,
source: 'url',
timestamp: expect.any(Number),
},
],
propagations: 0,
},
contexts: {
trace: {
Expand Down
5 changes: 0 additions & 5 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,6 @@ function _createWrappedRequestMethodFactory(
`[Tracing] Not adding sentry-trace header to outgoing request (${requestUrl}) due to mismatching tracePropagationTargets option.`,
);
}

const transaction = parentSpan.transaction;
if (transaction) {
transaction.metadata.propagations++;
}
}
}

Expand Down
14 changes: 0 additions & 14 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,20 +175,6 @@ describe('tracing', () => {
expect(baggage).not.toBeDefined();
});

it('records outgoing propagations on the transaction', () => {
nock('http://dogs.are.great').get('/').reply(200);

const transaction = createTransactionOnScope();

expect(transaction.metadata.propagations).toBe(0);

http.get('http://dogs.are.great/');
expect(transaction.metadata.propagations).toBe(1);

http.get('http://dogs.are.great/');
expect(transaction.metadata.propagations).toBe(2);
});

it("doesn't attach when using otel instrumenter", () => {
const loggerLogSpy = jest.spyOn(logger, 'log');

Expand Down
2 changes: 0 additions & 2 deletions packages/remix/test/integration/test/server/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
transaction: 'routes/loader-json-response/$id',
transaction_info: {
source: 'route',
changes: [],
propagations: 0,
},
spans: [
{
Expand Down
4 changes: 0 additions & 4 deletions packages/replay/test/fixtures/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,10 @@ export function Transaction(obj?: Partial<Event>): any {
"[Tracing] Starting 'resource.other' span on transaction '/organizations/:orgId/replays/:replaySlug/' (b44b173b1c74a782).",
},
},
changes: [],
propagations: 2,
sampleRate: 1,
}, // }}}
transaction_info: {
source: 'route',
changes: [],
propagations: 2,
},
measurements: {
longTaskCount: {
Expand Down
4 changes: 0 additions & 4 deletions packages/tracing/src/browser/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ export function fetchCallback(
span,
options,
);

activeTransaction.metadata.propagations++;
}
}
}
Expand Down Expand Up @@ -356,8 +354,6 @@ export function xhrCallback(
// https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
handlerData.xhr.setRequestHeader(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
}

activeTransaction.metadata.propagations++;
} catch (_) {
// Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
}
Expand Down
17 changes: 1 addition & 16 deletions packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
TransactionContext,
TransactionMetadata,
} from '@sentry/types';
import { dropUndefinedKeys, logger, timestampInSeconds } from '@sentry/utils';
import { dropUndefinedKeys, logger } from '@sentry/utils';

import { Span as SpanClass, SpanRecorder } from './span';

Expand Down Expand Up @@ -52,8 +52,6 @@ export class Transaction extends SpanClass implements TransactionInterface {
source: 'custom',
...transactionContext.metadata,
spanMetadata: {},
changes: [],
propagations: 0,
};

this._trimEnd = transactionContext.trimEnd;
Expand Down Expand Up @@ -84,17 +82,6 @@ export class Transaction extends SpanClass implements TransactionInterface {
* JSDoc
*/
public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void {
// `source` could change without the name changing if we discover that an unparameterized route is actually
// parameterized by virtue of having no parameters in its path
if (name !== this.name || source !== this.metadata.source) {
this.metadata.changes.push({
// log previous source
source: this.metadata.source,
timestamp: timestampInSeconds(),
propagations: this.metadata.propagations,
});
}

this._name = name;
this.metadata.source = source;
}
Expand Down Expand Up @@ -197,8 +184,6 @@ export class Transaction extends SpanClass implements TransactionInterface {
...(metadata.source && {
transaction_info: {
source: metadata.source,
changes: metadata.changes,
propagations: metadata.propagations,
},
}),
};
Expand Down
26 changes: 0 additions & 26 deletions packages/tracing/test/browser/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,6 @@ describe('callbacks', () => {

expect(newSpan).toBeUndefined();
});

it('records outgoing propogations', () => {
const firstReqData = { ...fetchHandlerData };
const secondReqData = { ...fetchHandlerData };

expect(transaction.metadata.propagations).toBe(0);

fetchCallback(firstReqData, alwaysCreateSpan, alwaysAttachHeaders, {});
expect(transaction.metadata.propagations).toBe(1);

fetchCallback(secondReqData, alwaysCreateSpan, alwaysAttachHeaders, {});
expect(transaction.metadata.propagations).toBe(2);
});
});

describe('xhrCallback()', () => {
Expand Down Expand Up @@ -370,19 +357,6 @@ describe('callbacks', () => {

expect(newSpan).toBeUndefined();
});

it('records outgoing propogations', () => {
const firstReqData = { ...xhrHandlerData };
const secondReqData = { ...xhrHandlerData };

expect(transaction.metadata.propagations).toBe(0);

xhrCallback(firstReqData, alwaysCreateSpan, alwaysAttachHeaders, {});
expect(transaction.metadata.propagations).toBe(1);

xhrCallback(secondReqData, alwaysCreateSpan, alwaysAttachHeaders, {});
expect(transaction.metadata.propagations).toBe(2);
});
});
});

Expand Down
2 changes: 0 additions & 2 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,6 @@ describe('Span', () => {
expect.objectContaining({
transaction_info: {
source: 'url',
changes: [],
propagations: 0,
},
}),
);
Expand Down
Loading

0 comments on commit 5359ba9

Please sign in to comment.