Skip to content

Commit

Permalink
feat(core): Add parentSpan option to startSpan* APIs (#12567)
Browse files Browse the repository at this point in the history 
With this PR, you can now pass a `parentSpan` optionally to `startSpan*`
APIs to create a span as a child of a specific span:


```js
const span = Sentry.startInactiveSpan({ name: 'xxx', parentSpan: parent });

Sentry.startSpan({ name: 'xxx', parentSpan: parent }, () => {});

Sentry.startSpanManual({ name: 'xxx', parentSpan: parent }, () => {});
```

With this, it should be easier to understand how you can manually work
with spans in v8.

Closes #12539

ref #12566, which I
noticed while working on this. For now I just cast the span which should
be fine, but is not ideal.
  • Loading branch information
@mydea
mydea authored Jun 20, 2024
1 parent 424937f commit 0019309
Show file tree
Hide file tree
Showing 5 changed files with 287 additions and 139 deletions.
211 changes: 122 additions & 89 deletions packages/core/src/tracing/trace.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable max-lines */

import type { ClientOptions, Scope, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '@sentry/types';
import { generatePropagationContext, logger, propagationContextFromHeaders } from '@sentry/utils';
import type { AsyncContextStrategy } from '../asyncContext/types';
Expand Down Expand Up @@ -32,40 +34,47 @@ const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__';
* You'll always get a span passed to the callback,
* it may just be a non-recording span if the span is not sampled or if tracing is disabled.
*/
export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) => T): T {
export function startSpan<T>(options: StartSpanOptions, callback: (span: Span) => T): T {
const acs = getAcs();
if (acs.startSpan) {
return acs.startSpan(context, callback);
return acs.startSpan(options, callback);
}

const spanContext = normalizeContext(context);

return withScope(context.scope, scope => {
const parentSpan = getParentSpan(scope);

const shouldSkipSpan = context.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
spanContext,
forceTransaction: context.forceTransaction,
scope,
});

_setSpanForScope(scope, activeSpan);

return handleCallbackErrors(
() => callback(activeSpan),
() => {
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
const { status } = spanToJSON(activeSpan);
if (activeSpan.isRecording() && (!status || status === 'ok')) {
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
}
},
() => activeSpan.end(),
);
const spanArguments = parseSentrySpanArguments(options);
const { forceTransaction, parentSpan: customParentSpan } = options;

return withScope(options.scope, () => {
// If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan`
const wrapper = getActiveSpanWrapper<T>(customParentSpan);

return wrapper(() => {
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope);

const shouldSkipSpan = options.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
spanArguments,
forceTransaction,
scope,
});

_setSpanForScope(scope, activeSpan);

return handleCallbackErrors(
() => callback(activeSpan),
() => {
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
const { status } = spanToJSON(activeSpan);
if (activeSpan.isRecording() && (!status || status === 'ok')) {
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
}
},
() => activeSpan.end(),
);
});
});
}

Expand All @@ -79,43 +88,50 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) =
* You'll always get a span passed to the callback,
* it may just be a non-recording span if the span is not sampled or if tracing is disabled.
*/
export function startSpanManual<T>(context: StartSpanOptions, callback: (span: Span, finish: () => void) => T): T {
export function startSpanManual<T>(options: StartSpanOptions, callback: (span: Span, finish: () => void) => T): T {
const acs = getAcs();
if (acs.startSpanManual) {
return acs.startSpanManual(context, callback);
return acs.startSpanManual(options, callback);
}

const spanContext = normalizeContext(context);

return withScope(context.scope, scope => {
const parentSpan = getParentSpan(scope);

const shouldSkipSpan = context.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
spanContext,
forceTransaction: context.forceTransaction,
scope,
});

_setSpanForScope(scope, activeSpan);

function finishAndSetSpan(): void {
activeSpan.end();
}

return handleCallbackErrors(
() => callback(activeSpan, finishAndSetSpan),
() => {
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
const { status } = spanToJSON(activeSpan);
if (activeSpan.isRecording() && (!status || status === 'ok')) {
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
}
},
);
const spanArguments = parseSentrySpanArguments(options);
const { forceTransaction, parentSpan: customParentSpan } = options;

return withScope(options.scope, () => {
// If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan`
const wrapper = getActiveSpanWrapper<T>(customParentSpan);

return wrapper(() => {
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope);

const shouldSkipSpan = options.onlyIfParent && !parentSpan;
const activeSpan = shouldSkipSpan
? new SentryNonRecordingSpan()
: createChildOrRootSpan({
parentSpan,
spanArguments,
forceTransaction,
scope,
});

_setSpanForScope(scope, activeSpan);

function finishAndSetSpan(): void {
activeSpan.end();
}

return handleCallbackErrors(
() => callback(activeSpan, finishAndSetSpan),
() => {
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
const { status } = spanToJSON(activeSpan);
if (activeSpan.isRecording() && (!status || status === 'ok')) {
activeSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
}
},
);
});
});
}

Expand All @@ -128,28 +144,39 @@ export function startSpanManual<T>(context: StartSpanOptions, callback: (span: S
* This function will always return a span,
* it may just be a non-recording span if the span is not sampled or if tracing is disabled.
*/
export function startInactiveSpan(context: StartSpanOptions): Span {
export function startInactiveSpan(options: StartSpanOptions): Span {
const acs = getAcs();
if (acs.startInactiveSpan) {
return acs.startInactiveSpan(context);
return acs.startInactiveSpan(options);
}

const spanContext = normalizeContext(context);
const spanArguments = parseSentrySpanArguments(options);
const { forceTransaction, parentSpan: customParentSpan } = options;

const scope = context.scope || getCurrentScope();
const parentSpan = getParentSpan(scope);
// If `options.scope` is defined, we use this as as a wrapper,
// If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan`
const wrapper = options.scope
? (callback: () => Span) => withScope(options.scope, callback)
: customParentSpan
? (callback: () => Span) => withActiveSpan(customParentSpan, callback)
: (callback: () => Span) => callback();

const shouldSkipSpan = context.onlyIfParent && !parentSpan;
return wrapper(() => {
const scope = getCurrentScope();
const parentSpan = getParentSpan(scope);

if (shouldSkipSpan) {
return new SentryNonRecordingSpan();
}
const shouldSkipSpan = options.onlyIfParent && !parentSpan;

if (shouldSkipSpan) {
return new SentryNonRecordingSpan();
}

return createChildOrRootSpan({
parentSpan,
spanContext,
forceTransaction: context.forceTransaction,
scope,
return createChildOrRootSpan({
parentSpan,
spanArguments,
forceTransaction,
scope,
});
});
}

Expand Down Expand Up @@ -239,12 +266,12 @@ export function startNewTrace<T>(callback: () => T): T {

function createChildOrRootSpan({
parentSpan,
spanContext,
spanArguments,
forceTransaction,
scope,
}: {
parentSpan: SentrySpan | undefined;
spanContext: SentrySpanArguments;
spanArguments: SentrySpanArguments;
forceTransaction?: boolean;
scope: Scope;
}): Span {
Expand All @@ -256,7 +283,7 @@ function createChildOrRootSpan({

let span: Span;
if (parentSpan && !forceTransaction) {
span = _startChildSpan(parentSpan, scope, spanContext);
span = _startChildSpan(parentSpan, scope, spanArguments);
addChildSpanToSpan(parentSpan, span);
} else if (parentSpan) {
// If we forced a transaction but have a parent span, make sure to continue from the parent span, not the scope
Expand All @@ -268,7 +295,7 @@ function createChildOrRootSpan({
{
traceId,
parentSpanId,
...spanContext,
...spanArguments,
},
scope,
parentSampled,
Expand All @@ -290,7 +317,7 @@ function createChildOrRootSpan({
{
traceId,
parentSpanId,
...spanContext,
...spanArguments,
},
scope,
parentSampled,
Expand All @@ -312,19 +339,17 @@ function createChildOrRootSpan({
* This converts StartSpanOptions to SentrySpanArguments.
* For the most part (for now) we accept the same options,
* but some of them need to be transformed.
*
* Eventually the StartSpanOptions will be more aligned with OpenTelemetry.
*/
function normalizeContext(context: StartSpanOptions): SentrySpanArguments {
const exp = context.experimental || {};
function parseSentrySpanArguments(options: StartSpanOptions): SentrySpanArguments {
const exp = options.experimental || {};
const initialCtx: SentrySpanArguments = {
isStandalone: exp.standalone,
...context,
...options,
};

if (context.startTime) {
if (options.startTime) {
const ctx: SentrySpanArguments & { startTime?: SpanTimeInput } = { ...initialCtx };
ctx.startTimestamp = spanTimeInputToSeconds(context.startTime);
ctx.startTimestamp = spanTimeInputToSeconds(options.startTime);
delete ctx.startTime;
return ctx;
}
Expand Down Expand Up @@ -419,3 +444,11 @@ function getParentSpan(scope: Scope): SentrySpan | undefined {

return span;
}

function getActiveSpanWrapper<T>(parentSpan?: Span): (callback: () => T) => T {
return parentSpan
? (callback: () => T) => {
return withActiveSpan(parentSpan, callback);
}
: (callback: () => T) => callback();
}
41 changes: 39 additions & 2 deletions packages/core/test/lib/tracing/trace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ describe('startSpan', () => {
expect(getActiveSpan()).toBe(undefined);
});

it('allows to pass a parentSpan', () => {
const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true, name: 'parent-span' });

startSpan({ name: 'GET users/[id]', parentSpan }, span => {
expect(getActiveSpan()).toBe(span);
expect(spanToJSON(span).parent_span_id).toBe('parent-span-id');
});

expect(getActiveSpan()).toBe(undefined);
});

it('allows to force a transaction with forceTransaction=true', async () => {
const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
client = new TestClient(options);
Expand Down Expand Up @@ -653,13 +664,13 @@ describe('startSpanManual', () => {
const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true });
_setSpanForScope(manualScope, parentSpan);

startSpanManual({ name: 'GET users/[id]', scope: manualScope }, (span, finish) => {
startSpanManual({ name: 'GET users/[id]', scope: manualScope }, span => {
expect(getCurrentScope()).not.toBe(initialScope);
expect(getCurrentScope()).toBe(manualScope);
expect(getActiveSpan()).toBe(span);
expect(spanToJSON(span).parent_span_id).toBe('parent-span-id');

finish();
span.end();

// Is still the active span
expect(getActiveSpan()).toBe(span);
Expand All @@ -669,6 +680,19 @@ describe('startSpanManual', () => {
expect(getActiveSpan()).toBe(undefined);
});

it('allows to pass a parentSpan', () => {
const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true, name: 'parent-span' });

startSpanManual({ name: 'GET users/[id]', parentSpan }, span => {
expect(getActiveSpan()).toBe(span);
expect(spanToJSON(span).parent_span_id).toBe('parent-span-id');

span.end();
});

expect(getActiveSpan()).toBe(undefined);
});

it('allows to force a transaction with forceTransaction=true', async () => {
const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
client = new TestClient(options);
Expand Down Expand Up @@ -977,6 +1001,19 @@ describe('startInactiveSpan', () => {
expect(getActiveSpan()).toBeUndefined();
});

it('allows to pass a parentSpan', () => {
const parentSpan = new SentrySpan({ spanId: 'parent-span-id', sampled: true, name: 'parent-span' });

const span = startInactiveSpan({ name: 'GET users/[id]', parentSpan });

expect(spanToJSON(span).parent_span_id).toBe('parent-span-id');
expect(getActiveSpan()).toBe(undefined);

span.end();

expect(getActiveSpan()).toBeUndefined();
});

it('allows to force a transaction with forceTransaction=true', async () => {
const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
client = new TestClient(options);
Expand Down
Loading

0 comments on commit 0019309

Please sign in to comment.