From 29a1628941763a24bd5b2da16bd80a1ff41088cb Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 8 Nov 2022 15:09:18 +0100 Subject: [PATCH 1/4] feat(tracing): Add transaction.setContext method This is needed for the OpenTelemetry Span Processor. --- packages/tracing/src/transaction.ts | 18 ++++ packages/tracing/test/transaction.test.ts | 126 ++++++++++++++++++++++ packages/types/src/transaction.ts | 6 ++ 3 files changed, 150 insertions(+) diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 83d5d14e419f..39571d0238f8 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -1,5 +1,7 @@ import { getCurrentHub, Hub } from '@sentry/core'; import { + Context, + Contexts, DynamicSamplingContext, Event, Measurements, @@ -25,6 +27,8 @@ export class Transaction extends SpanClass implements TransactionInterface { private _measurements: Measurements = {}; + private _contexts: Contexts = {}; + private _trimEnd?: boolean; private _frozenDynamicSamplingContext: Readonly> | undefined = undefined; @@ -105,6 +109,18 @@ export class Transaction extends SpanClass implements TransactionInterface { this.spanRecorder.add(this); } + /** + * @inheritDoc + */ + public setContext(key: string, context: Context | null): void { + if (context === null) { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete this._contexts[key]; + } else { + this._contexts = { ...this._contexts, [key]: context }; + } + } + /** * @inheritDoc */ @@ -163,6 +179,8 @@ export class Transaction extends SpanClass implements TransactionInterface { const transaction: Event = { contexts: { + ...this._contexts, + // We don't want to override trace context trace: this.getTraceContext(), }, spans: finishedSpans, diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index ebf8ddc41533..1df6bc10f9e6 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -1,6 +1,13 @@ import { Transaction } from '../src/transaction'; +import { addExtensionMethods } from '../src'; +import { getDefaultBrowserClientOptions } from './testutils'; +import { BrowserClient, Hub } from '@sentry/browser'; describe('`Transaction` class', () => { + beforeAll(() => { + addExtensionMethods(); + }); + describe('transaction name source', () => { it('sets source in constructor if provided', () => { const transaction = new Transaction({ name: 'dogpark', metadata: { source: 'route' } }); @@ -180,4 +187,123 @@ describe('`Transaction` class', () => { }); }); }); + + describe('setContext', () => { + it('sets context', () => { + const transaction = new Transaction({ name: 'dogpark' }); + transaction.setContext('foo', { + key: 'val', + key2: 'val2', + }); + + // @ts-ignore accessing private property + expect(transaction._contexts).toEqual({ + foo: { + key: 'val', + key2: 'val2', + }, + }); + }); + + it('overwrites context', () => { + const transaction = new Transaction({ name: 'dogpark' }); + transaction.setContext('foo', { + key: 'val', + key2: 'val2', + }); + transaction.setContext('foo', { + key3: 'val3', + }); + + // @ts-ignore accessing private property + expect(transaction._contexts).toEqual({ + foo: { + key3: 'val3', + }, + }); + }); + + it('merges context', () => { + const transaction = new Transaction({ name: 'dogpark' }); + transaction.setContext('foo', { + key: 'val', + key2: 'val2', + }); + transaction.setContext('bar', { + anotherKey: 'anotherVal', + }); + + // @ts-ignore accessing private property + expect(transaction._contexts).toEqual({ + foo: { + key: 'val', + key2: 'val2', + }, + bar: { + anotherKey: 'anotherVal', + }, + }); + }); + + it('deletes context', () => { + const transaction = new Transaction({ name: 'dogpark' }); + transaction.setContext('foo', { + key: 'val', + key2: 'val2', + }); + transaction.setContext('foo', null); + + // @ts-ignore accessing private property + expect(transaction._contexts).toEqual({}); + }); + + it('sets contexts on the event', () => { + const options = getDefaultBrowserClientOptions({ tracesSampleRate: 1 }); + const client = new BrowserClient(options); + const hub = new Hub(client); + + jest.spyOn(hub, 'captureEvent'); + + const transaction = hub.startTransaction({ name: 'dogpark' }); + transaction.setContext('foo', { key: 'val' }); + transaction.finish(); + + expect(hub.captureEvent).toHaveBeenCalledTimes(1); + expect(hub.captureEvent).toHaveBeenLastCalledWith( + expect.objectContaining({ + contexts: { + foo: { key: 'val' }, + trace: { + span_id: transaction.spanId, + trace_id: transaction.traceId, + }, + }, + }), + ); + }); + + it('does not override trace context', () => { + const options = getDefaultBrowserClientOptions({ tracesSampleRate: 1 }); + const client = new BrowserClient(options); + const hub = new Hub(client); + + jest.spyOn(hub, 'captureEvent'); + + const transaction = hub.startTransaction({ name: 'dogpark' }); + transaction.setContext('trace', { key: 'val' }); + transaction.finish(); + + expect(hub.captureEvent).toHaveBeenCalledTimes(1); + expect(hub.captureEvent).toHaveBeenLastCalledWith( + expect.objectContaining({ + contexts: { + trace: { + span_id: transaction.spanId, + trace_id: transaction.traceId, + }, + }, + }), + ); + }); + }); }); diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index 3859438866d0..a3a922eff9e2 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -1,3 +1,4 @@ +import { Context } from './context'; import { DynamicSamplingContext } from './envelope'; import { MeasurementUnit } from './measurement'; import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc'; @@ -76,6 +77,11 @@ export interface Transaction extends TransactionContext, Span { */ setName(name: string, source?: TransactionMetadata['source']): void; + /** + * Set the context of a transaction event + */ + setContext(key: string, context: Context): void; + /** * Set observed measurement for this transaction. * From e56c870c7eec424b002077d8a421975f13c4f43d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 8 Nov 2022 15:35:22 +0100 Subject: [PATCH 2/4] fix linting --- packages/opentelemetry-node/test/spanprocessor.test.ts | 8 +++----- packages/tracing/test/transaction.test.ts | 9 +++++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 306396d49376..c2124570b329 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -55,21 +55,19 @@ describe('SentrySpanProcessor', () => { } function getContext(transaction: Transaction) { - const transactionWithContext = transaction as unknown as Transaction & { _contexts: Contexts }; + const transactionWithContext = transaction as unknown as Transaction; return transactionWithContext._contexts; } // monkey-patch finish to store the context at finish time function monkeyPatchTransactionFinish(transaction: Transaction) { - const monkeyPatchedTransaction = transaction as Transaction & { _contexts: Contexts }; + const monkeyPatchedTransaction = transaction as Transaction; // eslint-disable-next-line @typescript-eslint/unbound-method const originalFinish = monkeyPatchedTransaction.finish; monkeyPatchedTransaction._contexts = {}; monkeyPatchedTransaction.finish = function (endTimestamp?: number | undefined) { - monkeyPatchedTransaction._contexts = ( - transaction._hub.getScope() as unknown as Scope & { _contexts: Contexts } - )._contexts; + monkeyPatchedTransaction._contexts = (transaction._hub.getScope() as unknown as Scope)._contexts; return originalFinish.apply(monkeyPatchedTransaction, [endTimestamp]); }; diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index 1df6bc10f9e6..c9d1f3fdabb2 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -1,7 +1,8 @@ -import { Transaction } from '../src/transaction'; +import { BrowserClient, Hub } from '@sentry/browser'; + import { addExtensionMethods } from '../src'; +import { Transaction } from '../src/transaction'; import { getDefaultBrowserClientOptions } from './testutils'; -import { BrowserClient, Hub } from '@sentry/browser'; describe('`Transaction` class', () => { beforeAll(() => { @@ -268,7 +269,9 @@ describe('`Transaction` class', () => { transaction.setContext('foo', { key: 'val' }); transaction.finish(); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(hub.captureEvent).toHaveBeenCalledTimes(1); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(hub.captureEvent).toHaveBeenLastCalledWith( expect.objectContaining({ contexts: { @@ -293,7 +296,9 @@ describe('`Transaction` class', () => { transaction.setContext('trace', { key: 'val' }); transaction.finish(); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(hub.captureEvent).toHaveBeenCalledTimes(1); + // eslint-disable-next-line @typescript-eslint/unbound-method expect(hub.captureEvent).toHaveBeenLastCalledWith( expect.objectContaining({ contexts: { From 536dca6f03fcd11d16385ab85c0764a5270c9991 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 8 Nov 2022 15:47:18 +0100 Subject: [PATCH 3/4] dont create new object --- packages/tracing/src/transaction.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 39571d0238f8..78a33fd17313 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -117,7 +117,7 @@ export class Transaction extends SpanClass implements TransactionInterface { // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete this._contexts[key]; } else { - this._contexts = { ...this._contexts, [key]: context }; + this._contexts[key] = context; } } From 0b6a461ec0034fdaf3ad40a21f1578d2d3e088ab Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 8 Nov 2022 16:09:37 +0100 Subject: [PATCH 4/4] ts-ignore stuff --- packages/opentelemetry-node/test/spanprocessor.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index c2124570b329..d2ce33847504 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -7,7 +7,7 @@ import { SemanticAttributes, SemanticResourceAttributes } from '@opentelemetry/s import { createTransport, Hub, makeMain } from '@sentry/core'; import { NodeClient } from '@sentry/node'; import { addExtensionMethods, Span as SentrySpan, SpanStatusType, Transaction } from '@sentry/tracing'; -import { Contexts, Scope } from '@sentry/types'; +import { Scope } from '@sentry/types'; import { resolvedSyncPromise } from '@sentry/utils'; import { SENTRY_SPAN_PROCESSOR_MAP, SentrySpanProcessor } from '../src/spanprocessor'; @@ -56,6 +56,7 @@ describe('SentrySpanProcessor', () => { function getContext(transaction: Transaction) { const transactionWithContext = transaction as unknown as Transaction; + // @ts-ignore accessing private property return transactionWithContext._contexts; } @@ -65,8 +66,10 @@ describe('SentrySpanProcessor', () => { // eslint-disable-next-line @typescript-eslint/unbound-method const originalFinish = monkeyPatchedTransaction.finish; + // @ts-ignore accessing private property monkeyPatchedTransaction._contexts = {}; monkeyPatchedTransaction.finish = function (endTimestamp?: number | undefined) { + // @ts-ignore accessing private property monkeyPatchedTransaction._contexts = (transaction._hub.getScope() as unknown as Scope)._contexts; return originalFinish.apply(monkeyPatchedTransaction, [endTimestamp]);