From 78dd562dc2f5ed550da9d17ad4b6e11c7bf9b493 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Dec 2022 12:03:40 +0100 Subject: [PATCH 1/6] Add transaction op default --- CHANGELOG.md | 3 +++ src/js/measurements.ts | 6 +++++- test/measurements.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/measurements.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b656c6d7ac..b3b88476d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ - Add `lastEventId` method to the API ([#2675](https://github.com/getsentry/sentry-react-native/pull/2675)) +### Fix +- `Sentry.startTransaction` doesn't require `op` ([#](https://github.com/getsentry/sentry-react-native/pull/)) + ### Dependencies - Bump Cocoa SDK from v7.31.2 to v7.31.3 ([#2647](https://github.com/getsentry/sentry-react-native/pull/2647)) diff --git a/src/js/measurements.ts b/src/js/measurements.ts index 242d7b3add..c54a2e2923 100644 --- a/src/js/measurements.ts +++ b/src/js/measurements.ts @@ -28,7 +28,7 @@ export function _addTracingExtensions(): void { } } -type StartTransactionFunction = ( +export type StartTransactionFunction = ( this: Hub, transactionContext: TransactionContext, customSamplingContext?: CustomSamplingContext @@ -48,6 +48,10 @@ const _patchStartTransaction = ( transactionContext: TransactionContext, customSamplingContext?: CustomSamplingContext ): Transaction { + if (!transactionContext.op) { + transactionContext.op = 'default'; + } + const transaction: Transaction = originalStartTransaction.apply(this, [ transactionContext, customSamplingContext, diff --git a/test/measurements.test.ts b/test/measurements.test.ts new file mode 100644 index 0000000000..4dc9b185c8 --- /dev/null +++ b/test/measurements.test.ts @@ -0,0 +1,26 @@ +import { getMainCarrier, getCurrentHub } from '@sentry/core'; +import { _addTracingExtensions, StartTransactionFunction } from '../src/js/measurements'; + +describe('Tracing extensions', () => { + test('transaction has default op', async () => { + _addTracingExtensions(); + const hub = getCurrentHub(); + const carrier = getMainCarrier(); + const startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined; + + const transaction = startTransaction?.apply(hub, [{}]); + + expect(transaction).toEqual(expect.objectContaining({ op: 'default' })); + }); + + test('transaction does not overwrite custom op', async () => { + _addTracingExtensions(); + const hub = getCurrentHub(); + const carrier = getMainCarrier(); + const startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined; + + const transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); + + expect(transaction).toEqual(expect.objectContaining({ op: 'custom' })); + }); +}); From 681cfe299f6648e1bf9e7eaf54d6f2faca2dd6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Mon, 12 Dec 2022 15:16:40 +0100 Subject: [PATCH 2/6] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3b88476d6..6e5e02b384 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ - Add `lastEventId` method to the API ([#2675](https://github.com/getsentry/sentry-react-native/pull/2675)) ### Fix -- `Sentry.startTransaction` doesn't require `op` ([#](https://github.com/getsentry/sentry-react-native/pull/)) +- `Sentry.startTransaction` doesn't require `op` ([#2691](https://github.com/getsentry/sentry-react-native/pull/2691)) ### Dependencies From 29cb4801b9ec3cda42047220674099d6434892c1 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Dec 2022 15:24:35 +0100 Subject: [PATCH 3/6] Fix lint --- test/measurements.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/measurements.test.ts b/test/measurements.test.ts index 4dc9b185c8..dd76436e79 100644 --- a/test/measurements.test.ts +++ b/test/measurements.test.ts @@ -1,4 +1,5 @@ -import { getMainCarrier, getCurrentHub } from '@sentry/core'; +import { getCurrentHub,getMainCarrier } from '@sentry/core'; + import { _addTracingExtensions, StartTransactionFunction } from '../src/js/measurements'; describe('Tracing extensions', () => { From b9b591ba345eac04f3eb2b4ee0deb03c4d4c9cd0 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Dec 2022 15:39:26 +0100 Subject: [PATCH 4/6] Fix missing new line in changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e5e02b384..9b86efb833 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Add `lastEventId` method to the API ([#2675](https://github.com/getsentry/sentry-react-native/pull/2675)) ### Fix + - `Sentry.startTransaction` doesn't require `op` ([#2691](https://github.com/getsentry/sentry-react-native/pull/2691)) ### Dependencies From 1f527b7d60b951098b429224569494833330460b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 12 Dec 2022 17:36:13 +0100 Subject: [PATCH 5/6] Fix startChild on transaction --- src/js/measurements.ts | 11 +++++++- test/measurements.test.ts | 53 ++++++++++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/js/measurements.ts b/src/js/measurements.ts index c54a2e2923..2909d64fd5 100644 --- a/src/js/measurements.ts +++ b/src/js/measurements.ts @@ -1,6 +1,6 @@ import { getCurrentHub, getMainCarrier, Hub } from '@sentry/core'; import { Transaction } from '@sentry/tracing'; -import { CustomSamplingContext, TransactionContext } from '@sentry/types'; +import { CustomSamplingContext, Span, SpanContext, TransactionContext } from '@sentry/types'; import { ReactNativeTracing } from './tracing'; @@ -56,6 +56,15 @@ const _patchStartTransaction = ( transactionContext, customSamplingContext, ]); + const originalStartChild: Transaction['startChild'] = transaction.startChild.bind(transaction); + transaction.startChild = ( + spanContext?: Pick>, + ): Span => { + return originalStartChild({ + ...spanContext, + op: spanContext?.op || 'default', + }); + }; const reactNativeTracing = getCurrentHub().getIntegration( ReactNativeTracing diff --git a/test/measurements.test.ts b/test/measurements.test.ts index dd76436e79..cbe6ac8d5a 100644 --- a/test/measurements.test.ts +++ b/test/measurements.test.ts @@ -1,27 +1,56 @@ -import { getCurrentHub,getMainCarrier } from '@sentry/core'; +import { Carrier, getCurrentHub, getMainCarrier } from '@sentry/core'; +import { Transaction } from '@sentry/tracing'; +import { Hub } from '@sentry/types'; import { _addTracingExtensions, StartTransactionFunction } from '../src/js/measurements'; describe('Tracing extensions', () => { - test('transaction has default op', async () => { + let hub: Hub; + let carrier: Carrier; + let startTransaction: StartTransactionFunction | undefined; + + beforeEach(() => { _addTracingExtensions(); - const hub = getCurrentHub(); - const carrier = getMainCarrier(); - const startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined; + hub = getCurrentHub(); + carrier = getMainCarrier(); + startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined; + }); - const transaction = startTransaction?.apply(hub, [{}]); + test('transaction has default op', async () => { + const transaction: Transaction = startTransaction?.apply(hub, [{}]); expect(transaction).toEqual(expect.objectContaining({ op: 'default' })); }); test('transaction does not overwrite custom op', async () => { - _addTracingExtensions(); - const hub = getCurrentHub(); - const carrier = getMainCarrier(); - const startTransaction = carrier.__SENTRY__?.extensions?.startTransaction as StartTransactionFunction | undefined; - - const transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); + const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); expect(transaction).toEqual(expect.objectContaining({ op: 'custom' })); }); + + test('transaction start span creates default op', async () => { + const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); + const span = transaction?.startChild(); + + expect(span).toEqual(expect.objectContaining({ op: 'default' })); + }); + + test('transaction start span keeps custom op', async () => { + const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); + const span = transaction?.startChild({ op: 'custom' }); + + expect(span).toEqual(expect.objectContaining({ op: 'custom' })); + }); + + test('transaction start span passes correct values to the child', async () => { + const transaction: Transaction = startTransaction?.apply(hub, [{ op: 'custom' }]); + const span = transaction?.startChild({ op: 'custom' }); + + expect(span).toEqual(expect.objectContaining({ + transaction, + parentSpanId: transaction.spanId, + sampled: transaction.sampled, + traceId: transaction.traceId, + })); + }); }); From 8a7a2bd6dbf7b0a5fca3660de19a077ade5295c1 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 13 Dec 2022 11:41:34 +0100 Subject: [PATCH 6/6] Add comments and default op const --- src/js/measurements.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/js/measurements.ts b/src/js/measurements.ts index 2909d64fd5..709a9122a0 100644 --- a/src/js/measurements.ts +++ b/src/js/measurements.ts @@ -4,6 +4,8 @@ import { CustomSamplingContext, Span, SpanContext, TransactionContext } from '@s import { ReactNativeTracing } from './tracing'; +const SPAN_OP_DEFAULT = 'default'; + /** * Adds React Native's extensions. Needs to be called after @sentry/tracing's extension methods are added */ @@ -48,8 +50,9 @@ const _patchStartTransaction = ( transactionContext: TransactionContext, customSamplingContext?: CustomSamplingContext ): Transaction { + // Native SDKs require op to be set - for JS Relay sets `default` if (!transactionContext.op) { - transactionContext.op = 'default'; + transactionContext.op = SPAN_OP_DEFAULT; } const transaction: Transaction = originalStartTransaction.apply(this, [ @@ -62,7 +65,8 @@ const _patchStartTransaction = ( ): Span => { return originalStartChild({ ...spanContext, - op: spanContext?.op || 'default', + // Native SDKs require op to be set + op: spanContext?.op || SPAN_OP_DEFAULT, }); };