Skip to content

feat(core): Ensure trace APIs always return a span #10942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
typeof serveOptions.fetch
>);
if (response && response.status) {
if (span) {
setHttpStatus(span, response.status);
}
setHttpStatus(span, response.status);
if (span instanceof Transaction) {
const scope = getCurrentScope();
scope.setContext('response', {
Expand Down
9 changes: 4 additions & 5 deletions packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
setHttpStatus,
startInactiveSpan,
} from './tracing';
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
import { hasTracingEnabled } from './utils/hasTracingEnabled';
import { spanToTraceHeader } from './utils/spanUtils';

Expand Down Expand Up @@ -92,12 +93,10 @@ export function instrumentFetchRequest(
},
op: 'http.client',
})
: undefined;
: new SentryNonRecordingSpan();

if (span) {
handlerData.fetchData.__span = span.spanContext().spanId;
spans[span.spanContext().spanId] = span;
}
handlerData.fetchData.__span = span.spanContext().spanId;
spans[span.spanContext().spanId] = span;

if (shouldAttachHeaders(handlerData.fetchData.url) && client) {
const request: string | Request = handlerData.args[0];
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export {
spanToJSON,
spanIsSampled,
spanToTraceContext,
getSpanDescendants,
} from './utils/spanUtils';
export { getRootSpan } from './utils/getRootSpan';
export { applySdkMetadata } from './utils/sdkMetadata';
Expand Down
31 changes: 10 additions & 21 deletions packages/core/src/tracing/idleSpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { logger, timestampInSeconds } from '@sentry/utils';
import { getClient, getCurrentScope } from '../currentScopes';

import { DEBUG_BUILD } from '../debug-build';
import { spanToJSON } from '../utils/spanUtils';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
import { getSpanDescendants, removeChildSpanFromSpan, spanToJSON } from '../utils/spanUtils';
import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
import { SPAN_STATUS_ERROR } from './spanstatus';
import { startInactiveSpan } from './trace';
import { getActiveSpan, getSpanDescendants, removeChildSpanFromSpan } from './utils';
import { getActiveSpan } from './utils';

export const TRACING_DEFAULTS = {
idleTimeout: 1_000,
Expand Down Expand Up @@ -71,10 +73,7 @@ interface IdleSpanOptions {
* An idle span is a span that automatically finishes. It does this by tracking child spans as activities.
* An idle span is always the active span.
*/
export function startIdleSpan(
startSpanOptions: StartSpanOptions,
options: Partial<IdleSpanOptions> = {},
): Span | undefined {
export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Partial<IdleSpanOptions> = {}): Span {
// Activities store a list of active spans
const activities = new Map<string, boolean>();

Expand All @@ -101,21 +100,13 @@ export function startIdleSpan(

const client = getClient();

if (!client) {
return;
if (!client || !hasTracingEnabled()) {
return new SentryNonRecordingSpan();
}

const scope = getCurrentScope();
const previousActiveSpan = getActiveSpan();
const _span = _startIdleSpan(startSpanOptions);

// Span _should_ always be defined here, but TS does not know that...
if (!_span) {
return;
}

// For TS, so that we know everything below here has a span
const span = _span;
const span = _startIdleSpan(startSpanOptions);

/**
* Cancels the existing idle timeout, if there is one.
Expand Down Expand Up @@ -319,15 +310,13 @@ export function startIdleSpan(
return span;
}

function _startIdleSpan(options: StartSpanOptions): Span | undefined {
function _startIdleSpan(options: StartSpanOptions): Span {
const span = startInactiveSpan(options);

// eslint-disable-next-line deprecation/deprecation
getCurrentScope().setSpan(span);

if (span) {
DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);
}
DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);

return span;
}
3 changes: 2 additions & 1 deletion packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ export { startIdleTransaction, addTracingExtensions } from './hubextensions';
export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction';
export type { BeforeFinishCallback } from './idletransaction';
export { SentrySpan } from './sentrySpan';
export { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
export { Transaction } from './transaction';
// eslint-disable-next-line deprecation/deprecation
export { getActiveTransaction, getActiveSpan, getSpanDescendants } from './utils';
export { getActiveTransaction, getActiveSpan } from './utils';
export {
setHttpStatus,
getSpanStatusFromHttpCode,
Expand Down
62 changes: 62 additions & 0 deletions packages/core/src/tracing/sentryNonRecordingSpan.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type {
Span,
SpanAttributeValue,
SpanAttributes,
SpanContext,
SpanContextData,
SpanStatus,
SpanTimeInput,
} from '@sentry/types';
import { uuid4 } from '@sentry/utils';
import { TRACE_FLAG_NONE } from '../utils/spanUtils';

/**
* A Sentry Span that is non-recording, meaning it will not be sent to Sentry.
*/
export class SentryNonRecordingSpan implements Span {
private _traceId: string;
private _spanId: string;

public constructor(spanContext: SpanContext = {}) {
this._traceId = spanContext.traceId || uuid4();
this._spanId = spanContext.spanId || uuid4().substring(16);
}

/** @inheritdoc */
public spanContext(): SpanContextData {
return {
spanId: this._spanId,
traceId: this._traceId,
traceFlags: TRACE_FLAG_NONE,
};
}

/** @inheritdoc */
// eslint-disable-next-line @typescript-eslint/no-empty-function
public end(_timestamp?: SpanTimeInput): void {}

/** @inheritdoc */
public setAttribute(_key: string, _value: SpanAttributeValue | undefined): this {
return this;
}

/** @inheritdoc */
public setAttributes(_values: SpanAttributes): this {
return this;
}

/** @inheritdoc */
public setStatus(_status: SpanStatus): this {
return this;
}

/** @inheritdoc */
public updateName(_name: string): this {
return this;
}

/** @inheritdoc */
public isRecording(): boolean {
return false;
}
}
2 changes: 1 addition & 1 deletion packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import { getRootSpan } from '../utils/getRootSpan';
import {
TRACE_FLAG_NONE,
TRACE_FLAG_SAMPLED,
addChildSpanToSpan,
getStatusMessage,
spanTimeInputToSeconds,
spanToJSON,
spanToTraceContext,
} from '../utils/spanUtils';
import { addChildSpanToSpan } from './utils';

/**
* Keeps track of finished spans for a given transaction
Expand Down
42 changes: 16 additions & 26 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import { DEBUG_BUILD } from '../debug-build';
import { getCurrentHub } from '../hub';
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
import { addChildSpanToSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
import type { SentrySpan } from './sentrySpan';
import { SPAN_STATUS_ERROR } from './spanstatus';
import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './utils';
import { getActiveSpan, setCapturedScopesOnSpan } from './utils';

/**
* Wraps a function with a transaction/span and finishes the span after the function is done.
Expand All @@ -24,7 +25,7 @@ import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './ut
* or you didn't set `tracesSampleRate`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startSpan<T>(context: StartSpanOptions, callback: (span: Span | undefined) => T): T {
export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) => T): T {
const spanContext = normalizeContext(context);

return withScope(context.scope, scope => {
Expand All @@ -35,18 +36,16 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |

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

if (activeSpan) {
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);
}
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);

return handleCallbackErrors(
() => callback(activeSpan),
Expand Down Expand Up @@ -75,10 +74,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
* or you didn't set `tracesSampleRate`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startSpanManual<T>(
context: StartSpanOptions,
callback: (span: Span | undefined, finish: () => void) => T,
): T {
export function startSpanManual<T>(context: StartSpanOptions, callback: (span: Span, finish: () => void) => T): T {
const spanContext = normalizeContext(context);

return withScope(context.scope, scope => {
Expand All @@ -89,18 +85,16 @@ export function startSpanManual<T>(

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

if (activeSpan) {
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);
}
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);

function finishAndSetSpan(): void {
activeSpan && activeSpan.end();
Expand Down Expand Up @@ -131,11 +125,7 @@ export function startSpanManual<T>(
* or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
if (!hasTracingEnabled()) {
return undefined;
}

export function startInactiveSpan(context: StartSpanOptions): Span {
const spanContext = normalizeContext(context);
// eslint-disable-next-line deprecation/deprecation
const hub = getCurrentHub();
Expand All @@ -147,7 +137,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
const shouldSkipSpan = context.onlyIfParent && !parentSpan;

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

const scope = context.scope || getCurrentScope();
Expand Down Expand Up @@ -275,14 +265,14 @@ function createChildSpanOrTransaction(
forceTransaction?: boolean;
scope: Scope;
},
): Span | undefined {
): Span {
if (!hasTracingEnabled()) {
return undefined;
return new SentryNonRecordingSpan();
}

const isolationScope = getIsolationScope();

let span: Span | undefined;
let span: Span;
if (parentSpan && !forceTransaction) {
// eslint-disable-next-line deprecation/deprecation
span = parentSpan.startChild(spanContext);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import { DEBUG_BUILD } from '../debug-build';
import { getCurrentHub } from '../hub';
import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
import { spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
import { getSpanDescendants, spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
import { SentrySpan, SpanRecorder } from './sentrySpan';
import { getCapturedScopesOnSpan, getSpanDescendants } from './utils';
import { getCapturedScopesOnSpan } from './utils';

/** JSDoc */
export class Transaction extends SentrySpan implements TransactionInterface {
Expand Down
48 changes: 0 additions & 48 deletions packages/core/src/tracing/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,54 +31,6 @@ export function getActiveSpan(): Span | undefined {
return getCurrentScope().getSpan();
}

const CHILD_SPANS_FIELD = '_sentryChildSpans';

type SpanWithPotentialChildren = Span & {
[CHILD_SPANS_FIELD]?: Set<Span>;
};

/**
* Adds an opaque child span reference to a span.
*/
export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
if (span[CHILD_SPANS_FIELD] && span[CHILD_SPANS_FIELD].size < 1000) {
span[CHILD_SPANS_FIELD].add(childSpan);
} else {
span[CHILD_SPANS_FIELD] = new Set([childSpan]);
}
}

/** This is only used internally by Idle Spans. */
export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
if (span[CHILD_SPANS_FIELD]) {
span[CHILD_SPANS_FIELD].delete(childSpan);
}
}

/**
* Returns an array of the given span and all of its descendants.
*/
export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] {
const resultSet = new Set<Span>();

function addSpanChildren(span: SpanWithPotentialChildren): void {
// This exit condition is required to not infinitely loop in case of a circular dependency.
if (resultSet.has(span)) {
return;
} else {
resultSet.add(span);
const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : [];
for (const childSpan of childSpans) {
addSpanChildren(childSpan);
}
}
}

addSpanChildren(span);

return Array.from(resultSet);
}

const SCOPE_ON_START_SPAN_FIELD = '_sentryScope';
const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope';

Expand Down
Loading