Skip to content

ref(browser): Refactor sentry breadcrumb to use hook #8892

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 1 commit into from
Aug 29, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { createClientReportEnvelope, dsnToString, getSDKSource, logger } from '@

import { eventFromException, eventFromMessage } from './eventbuilder';
import { WINDOW } from './helpers';
import type { Breadcrumbs } from './integrations';
import { BREADCRUMB_INTEGRATION_ID } from './integrations/breadcrumbs';
import type { BrowserTransportOptions } from './transports/types';
import { createUserFeedbackEnvelope } from './userfeedback';

Expand Down Expand Up @@ -91,26 +89,6 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
return eventFromMessage(this._options.stackParser, message, level, hint, this._options.attachStacktrace);
}

/**
* @inheritDoc
*/
public sendEvent(event: Event, hint?: EventHint): void {
// We only want to add the sentry event breadcrumb when the user has the breadcrumb integration installed and
// activated its `sentry` option.
// We also do not want to use the `Breadcrumbs` class here directly, because we do not want it to be included in
// bundles, if it is not used by the SDK.
// This all sadly is a bit ugly, but we currently don't have a "pre-send" hook on the integrations so we do it this
// way for now.
const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | undefined;
// We check for definedness of `addSentryBreadcrumb` in case users provided their own integration with id
// "Breadcrumbs" that does not have this function.
if (breadcrumbIntegration && breadcrumbIntegration.addSentryBreadcrumb) {
breadcrumbIntegration.addSentryBreadcrumb(event);
}

super.sendEvent(event, hint);
}

/**
* Sends user feedback to Sentry.
*/
Expand Down
40 changes: 20 additions & 20 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ interface BreadcrumbsOptions {
/** maxStringLength gets capped to prevent 100 breadcrumbs exceeding 1MB event payload size */
const MAX_ALLOWED_STRING_LENGTH = 1024;

export const BREADCRUMB_INTEGRATION_ID = 'Breadcrumbs';

/**
* Default Breadcrumbs instrumentations
* TODO: Deprecated - with v6, this will be renamed to `Instrument`
Expand All @@ -51,7 +49,7 @@ export class Breadcrumbs implements Integration {
/**
* @inheritDoc
*/
public static id: string = BREADCRUMB_INTEGRATION_ID;
public static id: string = 'Breadcrumbs';

/**
* @inheritDoc
Expand Down Expand Up @@ -104,28 +102,30 @@ export class Breadcrumbs implements Integration {
if (this.options.history) {
addInstrumentationHandler('history', _historyBreadcrumb);
}
}

/**
* Adds a breadcrumb for Sentry events or transactions if this option is enabled.
*/
public addSentryBreadcrumb(event: SentryEvent): void {
if (this.options.sentry) {
getCurrentHub().addBreadcrumb(
{
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
event_id: event.event_id,
level: event.level,
message: getEventDescription(event),
},
{
event,
},
);
const client = getCurrentHub().getClient();
client && client.on && client.on('beforeSendEvent', addSentryBreadcrumb);
}
}
}

/**
* Adds a breadcrumb for Sentry events or transactions if this option is enabled.
*/
function addSentryBreadcrumb(event: SentryEvent): void {
getCurrentHub().addBreadcrumb(
{
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
event_id: event.event_id,
level: event.level,
message: getEventDescription(event),
},
{
event,
},
);
}

/**
* A HOC that creaes a function that creates breadcrumbs from DOM API calls.
* This is a HOC so that we get access to dom options in the closure.
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @inheritDoc
*/
public sendEvent(event: Event, hint: EventHint = {}): void {
this.emit('beforeSendEvent', event, hint);

if (this._dsn) {
let env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel);

Expand Down Expand Up @@ -381,6 +383,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;

/** @inheritdoc */
public on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint) => void): void;

/** @inheritdoc */
public on(
hook: 'afterSendEvent',
Expand Down Expand Up @@ -412,6 +417,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public emit(hook: 'beforeEnvelope', envelope: Envelope): void;

/** @inheritdoc */
public emit(hook: 'beforeSendEvent', event: Event, hint?: EventHint): void;

/** @inheritdoc */
public emit(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void;

Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
on?(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;

/**
* Register a callback for before an event is sent.
*/
on?(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint | void) => void): void;

/**
* Register a callback for when an event has been sent.
*/
Expand Down Expand Up @@ -212,6 +217,12 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
emit?(hook: 'beforeEnvelope', envelope: Envelope): void;

/*
* Fire a hook event before sending an event. Expects to be given an Event & EventHint as the
* second/third argument.
*/
emit?(hook: 'beforeSendEvent', event: Event, hint?: EventHint): void;

/*
* Fire a hook event after sending an event. Expects to be given an Event as the
* second argument.
Expand Down