Skip to content

ref(tracing): Introduce trace function #3697

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

Closed
wants to merge 14 commits into from
Closed
1 change: 1 addition & 0 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/member-ordering */
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
import { Event, EventHint } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
Integration,
IntegrationClass,
Options,
Scope as ScopeType,
ScopeManager,
SessionStatus,
Severity,
} from '@sentry/types';
Expand All @@ -26,6 +28,7 @@ import {

import { Backend, BackendClass } from './basebackend';
import { IntegrationIndex, setupIntegrations } from './integration';
import { SimpleScopeManager } from './scopemanager';

/**
* Base implementation for all JavaScript SDK clients.
Expand Down Expand Up @@ -73,6 +76,9 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
/** The client Dsn, if specified in options. Without this Dsn, the SDK will be disabled. */
protected readonly _dsn?: Dsn;

/** ScopeManager used to manage scopes in current execution environment (Zone.js, Async Hooks, Domains, etc). */
protected readonly _scopeManager: ScopeManager;

/** Array of used integrations. */
protected _integrations: IntegrationIndex = {};

Expand All @@ -92,6 +98,9 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
if (options.dsn) {
this._dsn = new Dsn(options.dsn);
}

// TODO: set a default scopeManager that is the best implementation based on what we know of the execution env.
this._scopeManager = options.scopeManager || new SimpleScopeManager();
}

/**
Expand Down Expand Up @@ -224,6 +233,20 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
}
}

/**
* Returns the current scope.
*/
public getScope(): ScopeType {
return this._scopeManager.current();
}

/**
* Runs fn with a forked clone of the current scope.
*/
public withScope<T>(fn: (scope: ScopeType) => T): T {
return this._scopeManager.withScope(fn);
}

/** Updates existing session based on the provided event */
protected _updateSessionFromEvent(session: Session, event: Event): void {
let crashed = false;
Expand Down
32 changes: 32 additions & 0 deletions packages/core/src/scopemanager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Scope } from '@sentry/hub';
import { ScopeManager } from '@sentry/types';

/**
*
*/
export class SimpleScopeManager implements ScopeManager {
private _current: Scope = new Scope();

/**
*
*/
public current(): Scope {
return this._current;
}

/**
*
*/
public withScope<T>(fn: (scope: Scope) => T): T {
// public withScope<T>(scope: Scope, fn: (scope: Scope) => T): T {
// TODO: optional Scope as second argument
const oldScope = this._current;
const newScope = Scope.clone(this._current);
this._current = newScope;
try {
return fn(newScope);
} finally {
this._current = oldScope;
}
}
}
58 changes: 58 additions & 0 deletions packages/core/test/lib/scopemanager.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Scope } from '@sentry/hub';
import { Options } from '@sentry/types';

import { BaseClient } from '../../src/baseclient';
import { SimpleScopeManager } from '../../src/scopemanager';

const backendClass = Object; // we don't care about the backend

class TestClient extends BaseClient<any, Options> {
public constructor(options: Options) {
super(backendClass, options);
}
}

describe('Client + SimpleScopeManager', () => {
let client: TestClient;

beforeEach(() => {
client = new TestClient({ scopeManager: new SimpleScopeManager() });
});

test('getScope returns mutable scope', () => {
const scope = client.getScope();
scope.setTag('key', 'value');
expect(scope._tags).toEqual({ key: 'value' });
expect(client.getScope()).toEqual(scope);
});

test('withScope returns return value from wrapped function', () => {
const rv = client.withScope(() => 42);
expect(rv).toEqual(42);
});

test('withScope forks the current scope', () => {
const outerScope = client.getScope();
outerScope.setTag('outer', 'scope');
expect(outerScope._tags).toEqual({ outer: 'scope' });
let innerScope: Scope;
client.withScope(scope => {
innerScope = scope;
scope.setTag('inner', 'scope');
});
expect(innerScope._tags).toEqual({ outer: 'scope', inner: 'scope' });
expect(outerScope._tags).toEqual({ outer: 'scope' });
});

test('getScope within withScope returns current scope', () => {
const outerScope = client.getScope();
let innerScopeExplicit: Scope;
let innerScopeFromClient: Scope;
client.withScope(scope => {
innerScopeExplicit = scope;
innerScopeFromClient = client.getScope();
});
expect(innerScopeFromClient).toBe(innerScopeExplicit);
expect(innerScopeFromClient).not.toBe(outerScope);
});
});
81 changes: 81 additions & 0 deletions packages/tracing/performanceV2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# New User Friendly API

Thoughts we had while experimenting with tracing

## Goals

1. Update `withScope` and scope management to use new `ScopeManager` ideas
2.

## Context

- ContextManager (call this for now until we figure out how to combine with Scope)
- ContextManager is set on the client
- This is ok because there is always a way to figure out the current client
- Have something like `client.withScope()`?
- We decided not to use a global Context singleton like `@opentelemetry/api` (we can take shortcuts here)
- Client makes the decision on default context manager to use on construction (for ex. If it detects Zone, it uses ZoneManager)

- `hub.withScopeV2()` -> calls `client.withScope()` (step 2)
- If there is a hub, `getCurrentHub()?.getClient()`

### Tests:

- Nested with


## OpenTelemetry

BaseTraceProvider - https://github.com/open-telemetry/opentelemetry-js/blob/7860344eca83449170bafd03fd288e1a3deebacf/packages/opentelemetry-tracing/src/BasicTracerProvider.ts#L55

Registering a global - https://github.com/open-telemetry/opentelemetry-js-api/blob/7441feae07c63f68b3d21bd95b9291aa84f77f87/src/internal/global-utils.ts#L33

Fetch exmaple - https://github.com/open-telemetry/opentelemetry-js/blob/main/examples/tracer-web/examples/fetch/index.js

Sandbox link (opentelemetry fetch js example) - https://codesandbox.io/s/opentelemetry-api-js-experiment-y9th5

getContextManager - https://github.com/open-telemetry/opentelemetry-js-api/blob/41109c83a9784d689f319f2c5d953b3874c694a3/src/api/context.ts#L90

BaseContextManager - https://github.com/open-telemetry/opentelemetry-js-api/blob/main/src/context/context.ts

- For mobile we want global map (context)
- For web service we don't want

## Open Questions

- Is the context decision stored globally or per client? (or can we find a way around it)

- How to do manual context propagation? (if not using zones and still want to get the right context in a setTimeout)

- Can we have a global scope in addition to a local scope?

- With `Sentry.trace()` how to you tell it to create a `Transaction` vs `Span`?
- What happens if you nest `Transaction`s with `Sentry.trace()`?


- Possible way forward:

1. Improve hub propagation: add Zone support just like with have support for domains in `@sentry/hub`.
2. Add `Sentry.trace` on top of the existing Hub/Scope implementation (with Zone support).


## Unrelated

- Introduce the concept of a Tracer:
- Hub becomes Tracker + Tracer, eventually that is Tracker (errors/messages/sessions) + Tracer (spans) + Meter (metrics)
- Can each hub have multiple Tracker, Tracer, Meter?
- Can we have an generalized sampler then?
- Then Transport and Client only loads in code based on if Tracker/Tracer/Meter is active or not
- Tracker, Tracer, Meter all have their own processors
- Each have their own specific "context", but share hub wide context as well
- We remove scope -> put everything inside hub?

const hub = new Hub();

hub.startSpan -> hub.tracer.startSpan

hub.captureException -> hub.tracker.captureException

- Hubs have two modes (like sessions):
- request mode
- client mode (global singleton hub)
68 changes: 67 additions & 1 deletion packages/tracing/src/hubextensions.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { getMainCarrier, Hub } from '@sentry/hub';
/* eslint-disable max-lines */
import { getMainCarrier, Hub, Scope } from '@sentry/hub';
import {
CustomSamplingContext,
Integration,
IntegrationClass,
Options,
SamplingContext,
SpanContext,
TransactionContext,
TransactionSamplingMethod,
} from '@sentry/types';
import { dynamicRequire, isNodeEnv, loadModule, logger } from '@sentry/utils';

import { registerErrorInstrumentation } from './errors';
import { IdleTransaction } from './idletransaction';
import { SpanStatus } from './spanstatus';
import { Transaction } from './transaction';
import { hasTracingEnabled } from './utils';

Expand Down Expand Up @@ -146,6 +149,52 @@ function isValidSampleRate(rate: unknown): boolean {
return true;
}

/**
*
* @param this The Hub starting the transaction
* @param ctx The
* @param callback
*/
function _trace<T>(hub: Hub, ctx: SpanContext | TransactionContext, callback: (scope: Scope) => T): void {
// TODO: withScope returns different scope managers depending on what is registered on the hub
return hub.withScope(scope => {
const spanContext = { ...ctx };

const parentSpan = scope.getSpan();

// TODO: What should happen in this case?
const span = parentSpan
? parentSpan.startChild(spanContext)
: _startTransaction(hub, spanContext as TransactionContext);

scope.setSpan(span);
try {
const rv = callback(scope);
if (isPromise(rv)) {
return rv.then(
r => {
span.finish();
return r;
},
e => {
// TODO: Capture error?
span.setStatus(SpanStatus.InternalError);
span.finish();
return e;
},
);
}
span.finish();
return rv;
} catch (e) {
// TODO: Capture error?
span.setStatus(SpanStatus.InternalError);
span.finish();
return e;
}
});
}

/**
* Creates a new transaction and adds a sampling decision if it doesn't yet have one.
*
Expand Down Expand Up @@ -219,6 +268,23 @@ export function _addTracingExtensions(): void {
if (!carrier.__SENTRY__.extensions.traceHeaders) {
carrier.__SENTRY__.extensions.traceHeaders = traceHeaders;
}
if (!carrier.__SENTRY__.extensions.trace) {
carrier.__SENTRY__.extensions.trace = _trace;
}
}

/** JSDOC */
function isObject(value: unknown): boolean {
return value !== null && (typeof value === 'object' || typeof value === 'function');
}

/** JSDOC */
function isPromise<T>(value: any): value is Promise<T> {
return (
value instanceof Promise ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(isObject(value) && typeof value.then === 'function' && typeof value.catch === 'function')
);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions packages/tracing/test/withtrace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
describe('withTrace', () => {
it('should ', () => {
//withTrace;
});
});
1 change: 1 addition & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ export { Thread } from './thread';
export { Transport, TransportOptions, TransportClass } from './transport';
export { User } from './user';
export { WrappedFunction } from './wrappedfunction';
export { ScopeManager } from './scopemanager';
11 changes: 11 additions & 0 deletions packages/types/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Event, EventHint } from './event';
import { Integration } from './integration';
import { LogLevel } from './loglevel';
import { CaptureContext } from './scope';
import { ScopeManager } from './scopemanager';
import { SdkMetadata } from './sdkmetadata';
import { SamplingContext } from './transaction';
import { Transport, TransportClass, TransportOptions } from './transport';
Expand Down Expand Up @@ -137,6 +138,16 @@ export interface Options {
*/
initialScope?: CaptureContext;

/**
* The scope manager defines the strategy for scope management.
*
* Scope management is related to the ability of determining the current scope.
*
* A default scope manager is automatically chosen based on the current execution environment.
* This option serves to override the default.
*/
scopeManager?: ScopeManager;

/**
* Set of metadata about the SDK that can be internally used to enhance envelopes and events,
* and provide additional data about every request.
Expand Down
6 changes: 6 additions & 0 deletions packages/types/src/scopemanager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Scope } from './scope';

export interface ScopeManager {
current(): Scope;
withScope<T>(fn: (scope: Scope) => T): T;
}