Skip to content

Commit

Permalink
refactor: extract client from method
Browse files Browse the repository at this point in the history
  • Loading branch information
chargome committed Jul 24, 2024
1 parent f7ebd12 commit ddae213
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 23 deletions.
2 changes: 1 addition & 1 deletion packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
registerInpInteractionListener();
}

instrumentOutgoingRequests({
instrumentOutgoingRequests(client, {
traceFetch,
traceXHR,
tracePropagationTargets: client.getOptions().tracePropagationTargets,
Expand Down
35 changes: 16 additions & 19 deletions packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions
};

/** Registers span creators for xhr and fetch requests */
export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumentationOptions>): void {
export function instrumentOutgoingRequests(client: Client, _options?: Partial<RequestInstrumentationOptions>): void {
const { traceFetch, traceXHR, shouldCreateSpanForRequest, enableHTTPTimings, tracePropagationTargets } = {
traceFetch: defaultRequestInstrumentationOptions.traceFetch,
traceXHR: defaultRequestInstrumentationOptions.traceXHR,
Expand All @@ -117,27 +117,24 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
const shouldAttachHeadersWithTargets = (url: string): boolean => shouldAttachHeaders(url, tracePropagationTargets);

const spans: Record<string, Span> = {};
const client = getClient();

if (traceFetch) {
if (client) {
// Keeping track of http requests, whose body payloads resolved later than the intial resolved request
// e.g. streaming using server sent events (SSE)
client.addEventProcessor(event => {
if (event.type === 'transaction' && event.spans) {
event.spans.forEach(span => {
if (span.op === 'http.client') {
const updatedTimestamp = spanIdToEndTimestamp.get(span.span_id);
if (updatedTimestamp) {
span.timestamp = updatedTimestamp / 1000;
spanIdToEndTimestamp.delete(span.span_id);
}
// Keeping track of http requests, whose body payloads resolved later than the intial resolved request
// e.g. streaming using server sent events (SSE)
client.addEventProcessor(event => {
if (event.type === 'transaction' && event.spans) {
event.spans.forEach(span => {
if (span.op === 'http.client') {
const updatedTimestamp = spanIdToEndTimestamp.get(span.span_id);
if (updatedTimestamp) {
span.timestamp = updatedTimestamp / 1000;
spanIdToEndTimestamp.delete(span.span_id);
}
});
}
return event;
});
}
}
});
}
return event;
});

addFetchEndInstrumentationHandler(handlerData => {
if (handlerData.response) {
Expand Down
18 changes: 15 additions & 3 deletions packages/browser/test/unit/tracing/request.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as browserUtils from '@sentry-internal/browser-utils';
import type { Client } from '@sentry/types';
import * as utils from '@sentry/utils';
import { WINDOW } from '../../../src/helpers';

Expand All @@ -10,16 +11,27 @@ beforeAll(() => {
global.Request = {};
});

class MockClient implements Partial<Client> {
public addEventProcessor: () => void;
constructor() {
// Mock addEventProcessor function
this.addEventProcessor = jest.fn();
}
}

describe('instrumentOutgoingRequests', () => {
let client: Client;

beforeEach(() => {
jest.clearAllMocks();
client = new MockClient() as unknown as Client;
});

it('instruments fetch and xhr requests', () => {
const addFetchSpy = jest.spyOn(utils, 'addFetchInstrumentationHandler');
const addXhrSpy = jest.spyOn(browserUtils, 'addXhrInstrumentationHandler');

instrumentOutgoingRequests();
instrumentOutgoingRequests(client);

expect(addFetchSpy).toHaveBeenCalledWith(expect.any(Function));
expect(addXhrSpy).toHaveBeenCalledWith(expect.any(Function));
Expand All @@ -28,15 +40,15 @@ describe('instrumentOutgoingRequests', () => {
it('does not instrument fetch requests if traceFetch is false', () => {
const addFetchSpy = jest.spyOn(utils, 'addFetchInstrumentationHandler');

instrumentOutgoingRequests({ traceFetch: false });
instrumentOutgoingRequests(client, { traceFetch: false });

expect(addFetchSpy).not.toHaveBeenCalled();
});

it('does not instrument xhr requests if traceXHR is false', () => {
const addXhrSpy = jest.spyOn(browserUtils, 'addXhrInstrumentationHandler');

instrumentOutgoingRequests({ traceXHR: false });
instrumentOutgoingRequests(client, { traceXHR: false });

expect(addXhrSpy).not.toHaveBeenCalled();
});
Expand Down

0 comments on commit ddae213

Please sign in to comment.