Skip to content

Commit

Permalink
feat(core): Extract scope application to util (#9804)
Browse files Browse the repository at this point in the history
This is a try to move the application of scope data to events out of the
scope class into a util function.

This changes the flow to be:

1. The scope has a `getScopeData()` method that returns data that should
be applied to events.
2. The `prepareEvent` method uses that to actually apply scope data to
the event etc.
3. This also makes it much easier to do the experimentation in
node-experimental
(#9799) because we
only need to overwrite this, and can leave the rest of the event
processing the same.

I _think_ this makes sense but would appreciate some more eyes on this
as well. For me the separation makes also more sense, as e.g. the
application of event processors etc. should not really be tied to the
scope at all.

This is also is a first step to then allow us to add global scopes more
easily.

---------

Co-authored-by: Lukas Stracke <[email protected]>
  • Loading branch information
mydea and Lms24 authored Dec 19, 2023
1 parent d8eba90 commit afb900c
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 197 deletions.
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export {
convertIntegrationFnToClass,
} from './integration';
export { FunctionToString, InboundFilters, LinkedErrors } from './integrations';
export { applyScopeDataToEvent } from './utils/applyScopeDataToEvent';
export { prepareEvent } from './utils/prepareEvent';
export { createCheckInEnvelope } from './checkin';
export { hasTracingEnabled } from './utils/hasTracingEnabled';
Expand Down
133 changes: 48 additions & 85 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
RequestSession,
Scope as ScopeInterface,
ScopeContext,
ScopeData,
Session,
Severity,
SeverityLevel,
Expand All @@ -26,6 +27,7 @@ import { arrayify, dateTimestampInSeconds, isPlainObject, uuid4 } from '@sentry/

import { getGlobalEventProcessors, notifyEventProcessors } from './eventProcessors';
import { updateSession } from './session';
import { applyScopeDataToEvent } from './utils/applyScopeDataToEvent';

/**
* Default value for maximum number of breadcrumbs added to an event.
Expand Down Expand Up @@ -466,78 +468,65 @@ export class Scope implements ScopeInterface {
return this;
}

/** @inheritDoc */
public getScopeData(): ScopeData {
const {
_breadcrumbs,
_attachments,
_contexts,
_tags,
_extra,
_user,
_level,
_fingerprint,
_eventProcessors,
_propagationContext,
_sdkProcessingMetadata,
_transactionName,
_span,
} = this;

return {
breadcrumbs: _breadcrumbs,
attachments: _attachments,
contexts: _contexts,
tags: _tags,
extra: _extra,
user: _user,
level: _level,
fingerprint: _fingerprint || [],
eventProcessors: _eventProcessors,
propagationContext: _propagationContext,
sdkProcessingMetadata: _sdkProcessingMetadata,
transactionName: _transactionName,
span: _span,
};
}

/**
* Applies data from the scope to the event and runs all event processors on it.
*
* @param event Event
* @param hint Object containing additional information about the original exception, for use by the event processors.
* @hidden
* @deprecated Use `applyScopeDataToEvent()` directly
*/
public applyToEvent(
event: Event,
hint: EventHint = {},
additionalEventProcessors?: EventProcessor[],
additionalEventProcessors: EventProcessor[] = [],
): PromiseLike<Event | null> {
if (this._extra && Object.keys(this._extra).length) {
event.extra = { ...this._extra, ...event.extra };
}
if (this._tags && Object.keys(this._tags).length) {
event.tags = { ...this._tags, ...event.tags };
}
if (this._user && Object.keys(this._user).length) {
event.user = { ...this._user, ...event.user };
}
if (this._contexts && Object.keys(this._contexts).length) {
event.contexts = { ...this._contexts, ...event.contexts };
}
if (this._level) {
event.level = this._level;
}
if (this._transactionName) {
event.transaction = this._transactionName;
}

// We want to set the trace context for normal events only if there isn't already
// a trace context on the event. There is a product feature in place where we link
// errors with transaction and it relies on that.
if (this._span) {
event.contexts = { trace: this._span.getTraceContext(), ...event.contexts };
const transaction = this._span.transaction;
if (transaction) {
event.sdkProcessingMetadata = {
dynamicSamplingContext: transaction.getDynamicSamplingContext(),
...event.sdkProcessingMetadata,
};
const transactionName = transaction.name;
if (transactionName) {
event.tags = { transaction: transactionName, ...event.tags };
}
}
}

this._applyFingerprint(event);

const scopeBreadcrumbs = this._getBreadcrumbs();
const breadcrumbs = [...(event.breadcrumbs || []), ...scopeBreadcrumbs];
event.breadcrumbs = breadcrumbs.length > 0 ? breadcrumbs : undefined;

event.sdkProcessingMetadata = {
...event.sdkProcessingMetadata,
...this._sdkProcessingMetadata,
propagationContext: this._propagationContext,
};
applyScopeDataToEvent(event, this.getScopeData());

// TODO (v8): Update this order to be: Global > Client > Scope
return notifyEventProcessors(
[
...(additionalEventProcessors || []),
// eslint-disable-next-line deprecation/deprecation
...getGlobalEventProcessors(),
...this._eventProcessors,
],
event,
hint,
);
const eventProcessors: EventProcessor[] = [
...additionalEventProcessors,
// eslint-disable-next-line deprecation/deprecation
...getGlobalEventProcessors(),
...this._eventProcessors,
];

return notifyEventProcessors(eventProcessors, event, hint);
}

/**
Expand All @@ -564,13 +553,6 @@ export class Scope implements ScopeInterface {
return this._propagationContext;
}

/**
* Get the breadcrumbs for this scope.
*/
protected _getBreadcrumbs(): Breadcrumb[] {
return this._breadcrumbs;
}

/**
* This will be called on every set call.
*/
Expand All @@ -586,25 +568,6 @@ export class Scope implements ScopeInterface {
this._notifyingListeners = false;
}
}

/**
* Applies fingerprint from the scope to the event if there's one,
* uses message if there's one instead or get rid of empty fingerprint
*/
private _applyFingerprint(event: Event): void {
// Make sure it's an array first and we actually have something in place
event.fingerprint = event.fingerprint ? arrayify(event.fingerprint) : [];

// If we have something on the scope, then merge it with event
if (this._fingerprint) {
event.fingerprint = event.fingerprint.concat(this._fingerprint);
}

// If we have no data at all, remove empty array default
if (event.fingerprint && !event.fingerprint.length) {
delete event.fingerprint;
}
}
}

function generatePropagationContext(): PropagationContext {
Expand Down
97 changes: 97 additions & 0 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import type { Breadcrumb, Event, PropagationContext, ScopeData, Span } from '@sentry/types';
import { arrayify } from '@sentry/utils';

/**
* Applies data from the scope to the event and runs all event processors on it.
*/
export function applyScopeDataToEvent(event: Event, data: ScopeData): void {
const { fingerprint, span, breadcrumbs, sdkProcessingMetadata, propagationContext } = data;

// Apply general data
applyDataToEvent(event, data);

// We want to set the trace context for normal events only if there isn't already
// a trace context on the event. There is a product feature in place where we link
// errors with transaction and it relies on that.
if (span) {
applySpanToEvent(event, span);
}

applyFingerprintToEvent(event, fingerprint);
applyBreadcrumbsToEvent(event, breadcrumbs);
applySdkMetadataToEvent(event, sdkProcessingMetadata, propagationContext);
}

function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

if (extra && Object.keys(extra).length) {
event.extra = { ...extra, ...event.extra };
}
if (tags && Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}
if (user && Object.keys(user).length) {
event.user = { ...user, ...event.user };
}
if (contexts && Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}
if (level) {
event.level = level;
}
if (transactionName) {
event.transaction = transactionName;
}
}

function applyBreadcrumbsToEvent(event: Event, breadcrumbs: Breadcrumb[]): void {
const mergedBreadcrumbs = [...(event.breadcrumbs || []), ...breadcrumbs];
event.breadcrumbs = mergedBreadcrumbs.length ? mergedBreadcrumbs : undefined;
}

function applySdkMetadataToEvent(
event: Event,
sdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
propagationContext: PropagationContext,
): void {
event.sdkProcessingMetadata = {
...event.sdkProcessingMetadata,
...sdkProcessingMetadata,
propagationContext: propagationContext,
};
}

function applySpanToEvent(event: Event, span: Span): void {
event.contexts = { trace: span.getTraceContext(), ...event.contexts };
const transaction = span.transaction;
if (transaction) {
event.sdkProcessingMetadata = {
dynamicSamplingContext: transaction.getDynamicSamplingContext(),
...event.sdkProcessingMetadata,
};
const transactionName = transaction.name;
if (transactionName) {
event.tags = { transaction: transactionName, ...event.tags };
}
}
}

/**
* Applies fingerprint from the scope to the event if there's one,
* uses message if there's one instead or get rid of empty fingerprint
*/
function applyFingerprintToEvent(event: Event, fingerprint: ScopeData['fingerprint'] | undefined): void {
// Make sure it's an array first and we actually have something in place
event.fingerprint = event.fingerprint ? arrayify(event.fingerprint) : [];

// If we have something on the scope, then merge it with event
if (fingerprint) {
event.fingerprint = event.fingerprint.concat(fingerprint);
}

// If we have no data at all, remove empty array default
if (event.fingerprint && !event.fingerprint.length) {
delete event.fingerprint;
}
}
41 changes: 15 additions & 26 deletions packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,12 @@ import type {
StackFrame,
StackParser,
} from '@sentry/types';
import {
GLOBAL_OBJ,
addExceptionMechanism,
dateTimestampInSeconds,
normalize,
resolvedSyncPromise,
truncate,
uuid4,
} from '@sentry/utils';
import { GLOBAL_OBJ, addExceptionMechanism, dateTimestampInSeconds, normalize, truncate, uuid4 } from '@sentry/utils';

import { DEFAULT_ENVIRONMENT } from '../constants';
import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors';
import { Scope } from '../scope';
import { applyScopeDataToEvent } from './applyScopeDataToEvent';

/**
* This type makes sure that we get either a CaptureContext, OR an EventHint.
Expand Down Expand Up @@ -80,10 +73,13 @@ export function prepareEvent(
addExceptionMechanism(prepared, hint.mechanism);
}

// We prepare the result here with a resolved Event.
let result = resolvedSyncPromise<Event | null>(prepared);

const clientEventProcessors = client && client.getEventProcessors ? client.getEventProcessors() : [];
// TODO (v8): Update this order to be: Global > Client > Scope
const eventProcessors = [
...clientEventProcessors,
// eslint-disable-next-line deprecation/deprecation
...getGlobalEventProcessors(),
];

// This should be the last thing called, since we want that
// {@link Hub.addEventProcessor} gets the finished prepared event.
Expand All @@ -102,22 +98,15 @@ export function prepareEvent(
}
}

// In case we have a hub we reassign it.
result = finalScope.applyToEvent(prepared, hint, clientEventProcessors);
} else {
// Apply client & global event processors even if there is no scope
// TODO (v8): Update the order to be Global > Client
result = notifyEventProcessors(
[
...clientEventProcessors,
// eslint-disable-next-line deprecation/deprecation
...getGlobalEventProcessors(),
],
prepared,
hint,
);
const scopeData = finalScope.getScopeData();
applyScopeDataToEvent(prepared, scopeData);

// Run scope event processors _after_ all other processors
eventProcessors.push(...scopeData.eventProcessors);
}

const result = notifyEventProcessors(eventProcessors, prepared, hint);

return result.then(evt => {
if (evt) {
// We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Unit | util | prepareFeedbackEvent', () => {
hub.bindClient(client);

client = hub.getClient()!;
scope = hub.getScope()!;
scope = hub.getScope();
});

afterEach(() => {
Expand Down
Loading

0 comments on commit afb900c

Please sign in to comment.