Skip to content
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

feat(node-experimental): Keep breadcrumbs on transaction #8967

Merged
merged 1 commit into from
Sep 12, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/node-experimental/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export { INTEGRATIONS as Integrations };
export { getAutoPerformanceIntegrations } from './integrations/getAutoPerformanceIntegrations';
export * as Handlers from './sdk/handlers';
export * from './sdk/trace';
export { getCurrentHub, getHubFromCarrier } from './sdk/hub';

export {
makeNodeTransport,
Expand All @@ -33,8 +34,6 @@ export {
extractTraceparentData,
flush,
getActiveTransaction,
getHubFromCarrier,
getCurrentHub,
Hub,
lastEventId,
makeMain,
Expand Down
2 changes: 1 addition & 1 deletion packages/node-experimental/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { _INTERNAL_getSentrySpan } from '@sentry/opentelemetry-node';
import type { EventProcessor, Hub, Integration } from '@sentry/types';
import type { ClientRequest, IncomingMessage, ServerResponse } from 'http';

import type { NodeExperimentalClient } from '../sdk/client';
import type { NodeExperimentalClient } from '../types';
import { getRequestSpanData } from '../utils/getRequestSpanData';

interface TracingOptions {
Expand Down
26 changes: 24 additions & 2 deletions packages/node-experimental/src/sdk/client.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import type { Tracer } from '@opentelemetry/api';
import { trace } from '@opentelemetry/api';
import type { EventHint, Scope } from '@sentry/node';
import { NodeClient, SDK_VERSION } from '@sentry/node';
import type { Event } from '@sentry/types';

import type { NodeExperimentalClientOptions } from '../types';
import type {
NodeExperimentalClient as NodeExperimentalClientInterface,
NodeExperimentalClientOptions,
} from '../types';
import { OtelScope } from './scope';

/**
* A client built on top of the NodeClient, which provides some otel-specific things on top.
*/
export class NodeExperimentalClient extends NodeClient {
export class NodeExperimentalClient extends NodeClient implements NodeExperimentalClientInterface {
private _tracer: Tracer | undefined;

public constructor(options: ConstructorParameters<typeof NodeClient>[0]) {
Expand Down Expand Up @@ -47,4 +53,20 @@ export class NodeExperimentalClient extends NodeClient {
// Just a type-cast, basically
return super.getOptions();
}

/**
* Extends the base `_prepareEvent` so that we can properly handle `captureContext`.
* This uses `Scope.clone()`, which we need to replace with `OtelScope.clone()` for this client.
*/
protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
let actualScope = scope;

// Remove `captureContext` hint and instead clone already here
if (hint && hint.captureContext) {
actualScope = OtelScope.clone(scope);
delete hint.captureContext;
}

return super._prepareEvent(event, hint, actualScope);
}
}
140 changes: 140 additions & 0 deletions packages/node-experimental/src/sdk/hub.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import type { Carrier, Scope } from '@sentry/core';
import { Hub } from '@sentry/core';
import type { Client } from '@sentry/types';
import { getGlobalSingleton, GLOBAL_OBJ } from '@sentry/utils';

import { OtelScope } from './scope';

/** A custom hub that ensures we always creat an OTEL scope. */

class OtelHub extends Hub {
public constructor(client?: Client, scope: Scope = new OtelScope()) {
super(client, scope);
}

/**
* @inheritDoc
*/
public pushScope(): Scope {
// We want to clone the content of prev scope
const scope = OtelScope.clone(this.getScope());
this.getStack().push({
client: this.getClient(),
scope,
});
return scope;
}
}

/**
* *******************************************************************************
* Everything below here is a copy of the stuff from core's hub.ts,
* only that we make sure to create our custom OtelScope instead of the default Scope.
* This is necessary to get the correct breadcrumbs behavior.
*
* Basically, this overwrites all places that do `new Scope()` with `new OtelScope()`.
* Which in turn means overwriting all places that do `new Hub()` and make sure to pass in a OtelScope instead.
* *******************************************************************************
*/

/**
* API compatibility version of this hub.
*
* WARNING: This number should only be increased when the global interface
* changes and new methods are introduced.
*
* @hidden
*/
const API_VERSION = 4;

/**
* Returns the default hub instance.
*
* If a hub is already registered in the global carrier but this module
* contains a more recent version, it replaces the registered version.
* Otherwise, the currently registered hub will be returned.
*/
export function getCurrentHub(): Hub {
// Get main carrier (global for every environment)
const registry = getMainCarrier();

if (registry.__SENTRY__ && registry.__SENTRY__.acs) {
const hub = registry.__SENTRY__.acs.getCurrentHub();

if (hub) {
return hub;
}
}

// Return hub that lives on a global object
return getGlobalHub(registry);
}

/**
* This will create a new {@link Hub} and add to the passed object on
* __SENTRY__.hub.
* @param carrier object
* @hidden
*/
export function getHubFromCarrier(carrier: Carrier): Hub {
return getGlobalSingleton<Hub>('hub', () => new OtelHub(), carrier);
}

/**
* @private Private API with no semver guarantees!
*
* If the carrier does not contain a hub, a new hub is created with the global hub client and scope.
*/
export function ensureHubOnCarrier(carrier: Carrier, parent: Hub = getGlobalHub()): void {
// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(carrier) || getHubFromCarrier(carrier).isOlderThan(API_VERSION)) {
const globalHubTopStack = parent.getStackTop();
setHubOnCarrier(carrier, new OtelHub(globalHubTopStack.client, OtelScope.clone(globalHubTopStack.scope)));
}
}

function getGlobalHub(registry: Carrier = getMainCarrier()): Hub {
// If there's no hub, or its an old API, assign a new one
if (!hasHubOnCarrier(registry) || getHubFromCarrier(registry).isOlderThan(API_VERSION)) {
setHubOnCarrier(registry, new OtelHub());
}

// Return hub that lives on a global object
return getHubFromCarrier(registry);
}

/**
* This will tell whether a carrier has a hub on it or not
* @param carrier object
*/
function hasHubOnCarrier(carrier: Carrier): boolean {
return !!(carrier && carrier.__SENTRY__ && carrier.__SENTRY__.hub);
}

/**
* Returns the global shim registry.
*
* FIXME: This function is problematic, because despite always returning a valid Carrier,
* it has an optional `__SENTRY__` property, which then in turn requires us to always perform an unnecessary check
* at the call-site. We always access the carrier through this function, so we can guarantee that `__SENTRY__` is there.
**/
function getMainCarrier(): Carrier {
GLOBAL_OBJ.__SENTRY__ = GLOBAL_OBJ.__SENTRY__ || {
extensions: {},
hub: undefined,
};
return GLOBAL_OBJ;
}

/**
* This will set passed {@link Hub} on the passed object's __SENTRY__.hub attribute
* @param carrier object
* @param hub Hub
* @returns A boolean indicating success or failure
*/
function setHubOnCarrier(carrier: Carrier, hub: Hub): boolean {
if (!carrier) return false;
const __SENTRY__ = (carrier.__SENTRY__ = carrier.__SENTRY__ || {});
__SENTRY__.hub = hub;
return true;
}
2 changes: 1 addition & 1 deletion packages/node-experimental/src/sdk/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getCurrentHub } from '@sentry/core';
import { SentryPropagator, SentrySpanProcessor } from '@sentry/opentelemetry-node';
import { logger } from '@sentry/utils';

import type { NodeExperimentalClient } from './client';
import type { NodeExperimentalClient } from '../types';
import { SentryContextManager } from './otelContextManager';

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/node-experimental/src/sdk/otelContextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import type { Context } from '@opentelemetry/api';
import * as api from '@opentelemetry/api';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import type { Carrier, Hub } from '@sentry/core';
import { ensureHubOnCarrier, getCurrentHub, getHubFromCarrier } from '@sentry/core';

import { ensureHubOnCarrier, getCurrentHub, getHubFromCarrier } from './hub';

export const OTEL_CONTEXT_HUB_KEY = api.createContextKey('sentry_hub');

Expand Down
138 changes: 138 additions & 0 deletions packages/node-experimental/src/sdk/scope.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { Scope } from '@sentry/core';
import type { Breadcrumb, Transaction } from '@sentry/types';
import { dateTimestampInSeconds } from '@sentry/utils';

import { getActiveSpan } from './trace';

const DEFAULT_MAX_BREADCRUMBS = 100;

/**
* This is a fork of the base Transaction with OTEL specific stuff added.
* Note that we do not solve this via an actual subclass, but by wrapping this in a proxy when we need it -
* as we can't easily control all the places a transaction may be created.
*/
interface TransactionWithBreadcrumbs extends Transaction {
_breadcrumbs: Breadcrumb[];

/** Get all breadcrumbs added to this transaction. */
getBreadcrumbs(): Breadcrumb[];

/** Add a breadcrumb to this transaction. */
addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): void;
}

/** A fork of the classic scope with some otel specific stuff. */
export class OtelScope extends Scope {
/**
* @inheritDoc
*/
public static clone(scope?: Scope): Scope {
const newScope = new OtelScope();
if (scope) {
newScope._breadcrumbs = [...scope['_breadcrumbs']];
newScope._tags = { ...scope['_tags'] };
newScope._extra = { ...scope['_extra'] };
newScope._contexts = { ...scope['_contexts'] };
newScope._user = scope['_user'];
newScope._level = scope['_level'];
newScope._span = scope['_span'];
newScope._session = scope['_session'];
newScope._transactionName = scope['_transactionName'];
newScope._fingerprint = scope['_fingerprint'];
newScope._eventProcessors = [...scope['_eventProcessors']];
newScope._requestSession = scope['_requestSession'];
newScope._attachments = [...scope['_attachments']];
newScope._sdkProcessingMetadata = { ...scope['_sdkProcessingMetadata'] };
newScope._propagationContext = { ...scope['_propagationContext'] };
}
return newScope;
}

/**
* @inheritDoc
*/
public addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): this {
const transaction = getActiveTransaction();

if (transaction) {
transaction.addBreadcrumb(breadcrumb, maxBreadcrumbs);
return this;
}

return super.addBreadcrumb(breadcrumb, maxBreadcrumbs);
}

/**
* @inheritDoc
*/
protected _getBreadcrumbs(): Breadcrumb[] {
const transaction = getActiveTransaction();
const transactionBreadcrumbs = transaction ? transaction.getBreadcrumbs() : [];

return this._breadcrumbs.concat(transactionBreadcrumbs);
}
}

/**
* This gets the currently active transaction,
* and ensures to wrap it so that we can store breadcrumbs on it.
*/
function getActiveTransaction(): TransactionWithBreadcrumbs | undefined {
const activeSpan = getActiveSpan();
const transaction = activeSpan && activeSpan.transaction;

if (!transaction) {
return undefined;
}

if (transactionHasBreadcrumbs(transaction)) {
return transaction;
}

return new Proxy(transaction as TransactionWithBreadcrumbs, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: we can proxy patch startTransaction (hub extension) so that we don't have to rely on getActiveTransaction being used all the time.

Can skip this for now just to get it merged.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, wrote something up here: #9010

get(target, prop, receiver) {
if (prop === 'addBreadcrumb') {
return addBreadcrumb;
}
if (prop === 'getBreadcrumbs') {
return getBreadcrumbs;
}
if (prop === '_breadcrumbs') {
const breadcrumbs = Reflect.get(target, prop, receiver);
return breadcrumbs || [];
}
return Reflect.get(target, prop, receiver);
},
});
}

function transactionHasBreadcrumbs(transaction: Transaction): transaction is TransactionWithBreadcrumbs {
return (
typeof (transaction as TransactionWithBreadcrumbs).getBreadcrumbs === 'function' &&
typeof (transaction as TransactionWithBreadcrumbs).addBreadcrumb === 'function'
);
}

/** Add a breadcrumb to a transaction. */
function addBreadcrumb(this: TransactionWithBreadcrumbs, breadcrumb: Breadcrumb, maxBreadcrumbs?: number): void {
const maxCrumbs = typeof maxBreadcrumbs === 'number' ? maxBreadcrumbs : DEFAULT_MAX_BREADCRUMBS;

// No data has been changed, so don't notify scope listeners
if (maxCrumbs <= 0) {
return;
}

const mergedBreadcrumb = {
timestamp: dateTimestampInSeconds(),
...breadcrumb,
};

const breadcrumbs = this._breadcrumbs;
breadcrumbs.push(mergedBreadcrumb);
this._breadcrumbs = breadcrumbs.length > maxCrumbs ? breadcrumbs.slice(-maxCrumbs) : breadcrumbs;
}

/** Get all breadcrumbs from a transaction. */
function getBreadcrumbs(this: TransactionWithBreadcrumbs): Breadcrumb[] {
return this._breadcrumbs;
}
2 changes: 1 addition & 1 deletion packages/node-experimental/src/sdk/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { _INTERNAL_getSentrySpan } from '@sentry/opentelemetry-node';
import type { Span, TransactionContext } from '@sentry/types';
import { isThenable } from '@sentry/utils';

import type { NodeExperimentalClient } from './client';
import type { NodeExperimentalClient } from '../types';

/**
* Wraps a function with a transaction/span and finishes the span after the function is done.
Expand Down
6 changes: 6 additions & 0 deletions packages/node-experimental/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import type { Tracer } from '@opentelemetry/api';
import type { NodeClient, NodeOptions } from '@sentry/node';

export type NodeExperimentalOptions = NodeOptions;
export type NodeExperimentalClientOptions = ConstructorParameters<typeof NodeClient>[0];

export interface NodeExperimentalClient extends NodeClient {
tracer: Tracer;
getOptions(): NodeExperimentalClientOptions;
}