Skip to content

Commit

Permalink
feat(otel): Handle inconsistent instrumenter option (#6153)
Browse files Browse the repository at this point in the history
* feat(otel): Set `otel` instrumenter in otel integration on spans

* feat(tracing): Error when using inconsistent `instrumenter`
  • Loading branch information
mydea committed Nov 9, 2022
1 parent a99d894 commit 9452a1d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/opentelemetry-node/src/spanprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
if (sentryParentSpan) {
const sentryChildSpan = sentryParentSpan.startChild({
description: otelSpan.name,
// instrumentor: 'otel',
instrumenter: 'otel',
startTimestamp: convertOtelTimeToSeconds(otelSpan.startTime),
spanId: otelSpanId,
});
Expand All @@ -56,7 +56,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
const transaction = hub.startTransaction({
name: otelSpan.name,
...traceCtx,
// instrumentor: 'otel',
instrumenter: 'otel',
startTimestamp: convertOtelTimeToSeconds(otelSpan.startTime),
spanId: otelSpanId,
});
Expand Down
13 changes: 13 additions & 0 deletions packages/tracing/src/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ function _startTransaction(
const client = this.getClient();
const options: Partial<ClientOptions> = (client && client.getOptions()) || {};

const configInstrumenter = options.instrumenter || 'sentry';
const transactionInstrumenter = transactionContext.instrumenter || 'sentry';

if (configInstrumenter !== transactionInstrumenter) {
__DEBUG_BUILD__ &&
logger.error(
`A transaction was started with instrumenter=\`${transactionInstrumenter}\`, but the SDK is configured with the \`${configInstrumenter}\` instrumenter.
The transaction will not be sampled. Please use the ${configInstrumenter} instrumentation to start transactions.`,
);

transactionContext.sampled = false;
}

let transaction = new Transaction(transactionContext, this);
transaction = sample(transaction, options, {
parentSampled: transactionContext.parentSampled,
Expand Down
22 changes: 22 additions & 0 deletions packages/tracing/test/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const mathRandom = jest.spyOn(Math, 'random');
jest.spyOn(Transaction.prototype, 'setMetadata');
jest.spyOn(logger, 'warn');
jest.spyOn(logger, 'log');
jest.spyOn(logger, 'error');
jest.spyOn(utilsModule, 'isNodeEnv');

addDOMPropertiesToGlobal(['XMLHttpRequest', 'Event', 'location', 'document']);
Expand Down Expand Up @@ -377,6 +378,27 @@ describe('Hub', () => {
expect(client.captureEvent).not.toBeCalled();
});

it('should drop transactions when using wrong instrumenter', () => {
const options = getDefaultBrowserClientOptions({ tracesSampleRate: 1, instrumenter: 'otel' });
const client = new BrowserClient(options);
jest.spyOn(client, 'captureEvent');

const hub = new Hub(client);
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

jest.spyOn(transaction, 'finish');
transaction.finish();

expect(transaction.sampled).toBe(false);
expect(transaction.finish).toReturnWith(undefined);
expect(client.captureEvent).not.toBeCalled();
expect(logger.error).toHaveBeenCalledWith(
`A transaction was started with instrumenter=\`sentry\`, but the SDK is configured with the \`otel\` instrumenter.
The transaction will not be sampled. Please use the otel instrumentation to start transactions.`,
);
});

describe('sampling inheritance', () => {
it('should propagate sampling decision to child spans', () => {
const options = getDefaultBrowserClientOptions({ tracesSampleRate: Math.random() });
Expand Down

0 comments on commit 9452a1d

Please sign in to comment.