Skip to content

Commit

Permalink
chore(svelte): Detect and report SvelteKit usage (getsentry#5594)
Browse files Browse the repository at this point in the history
Add a SvelteKit detection mechanism to our SDK and inserts a SvelteKit entry into `event.modules` if we detect that the frontend in which the SDK is used, was rendered by SvelteKit.

To check this, we check for the existence of a div with the id `svelte-announcer` that's automatically injected into the rendered HTML by SvelteKit. It is used to improve accessibility (see sveltejs/kit#307) but we can leverage it as a SvelteKit detection criterion. 

Btw, we're not the first ones who came up with this approach, as it turns out: https://twitter.com/jhewt/status/1359632380483493889 

Introduce a new utils function, `getDomElement` in which we consolidate the usage of `document.querySelector` in the SDK. So in addition to using this new function for obtaining the div described above, we now also call it in `BrowserTracing` to get `<meta>` tags.

Add some tests to test the new behaviour and the helper functions. We might want to consider writing an integration test for this feature but this first requires a Svelte SDK integration test infrastructure. 

ref: getsentry#5573
  • Loading branch information
Lms24 committed Aug 18, 2022
1 parent 8dd6743 commit 74db527
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 15 deletions.
46 changes: 44 additions & 2 deletions packages/svelte/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { BrowserOptions, init as browserInit, SDK_VERSION } from '@sentry/browser';

import { addGlobalEventProcessor, BrowserOptions, init as browserInit, SDK_VERSION } from '@sentry/browser';
import type { EventProcessor } from '@sentry/types';
import { getDomElement } from '@sentry/utils';
/**
* Inits the Svelte SDK
*/
Expand All @@ -17,4 +18,45 @@ export function init(options: BrowserOptions): void {
};

browserInit(options);

detectAndReportSvelteKit();
}

/**
* Adds a global event processor to detect if the SDK is initialized in a SvelteKit frontend,
* in which case we add SvelteKit an event.modules entry to outgoing events.
* SvelteKit detection is performed only once, when the event processor is called for the
* first time. We cannot perform this check upfront (directly when init is called) because
* at this time, the HTML element might not yet be accessible.
*/
export function detectAndReportSvelteKit(): void {
let detectedSvelteKit: boolean | undefined = undefined;

const svelteKitProcessor: EventProcessor = event => {
if (detectedSvelteKit === undefined) {
detectedSvelteKit = isSvelteKitApp();
}
if (detectedSvelteKit) {
event.modules = {
svelteKit: 'latest',
...event.modules,
};
}
return event;
};
svelteKitProcessor.id = 'svelteKitProcessor';

addGlobalEventProcessor(svelteKitProcessor);
}

/**
* To actually detect a SvelteKit frontend, we search the DOM for a special
* div that's inserted by SvelteKit when the page is rendered. It's identifyed
* by its id, 'svelte-announcer', and it's used to improve page accessibility.
* This div is not present when only using Svelte without SvelteKit.
*
* @see https://github.com/sveltejs/kit/issues/307 for more information
*/
export function isSvelteKitApp(): boolean {
return getDomElement('div#svelte-announcer') !== null;
}
62 changes: 60 additions & 2 deletions packages/svelte/test/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { init as browserInitRaw, SDK_VERSION } from '@sentry/browser';
import { addGlobalEventProcessor, init as browserInitRaw, SDK_VERSION } from '@sentry/browser';
import { EventProcessor } from '@sentry/types';

import { init as svelteInit } from '../src/sdk';
import { detectAndReportSvelteKit, init as svelteInit, isSvelteKitApp } from '../src/sdk';

const browserInit = browserInitRaw as jest.Mock;
const addGlobalEventProcessorFunction = addGlobalEventProcessor as jest.Mock;
let passedEventProcessor: EventProcessor | undefined;
addGlobalEventProcessorFunction.mockImplementation(proc => {
passedEventProcessor = proc;
});

jest.mock('@sentry/browser');

describe('Initialize Svelte SDk', () => {
Expand All @@ -29,3 +36,54 @@ describe('Initialize Svelte SDk', () => {
expect(browserInit).toHaveBeenCalledWith(expect.objectContaining(expectedMetadata));
});
});

describe('detectAndReportSvelteKit()', () => {
const originalHtmlBody = document.body.innerHTML;
afterEach(() => {
jest.clearAllMocks();
document.body.innerHTML = originalHtmlBody;
passedEventProcessor = undefined;
});

it('registers a global event processor', async () => {
detectAndReportSvelteKit();

expect(addGlobalEventProcessorFunction).toHaveBeenCalledTimes(1);
expect(passedEventProcessor?.id).toEqual('svelteKitProcessor');
});

it('adds "SvelteKit" as a module to the event, if SvelteKit was detected', () => {
document.body.innerHTML += '<div id="svelte-announcer">Home</div>';
detectAndReportSvelteKit();

const processedEvent = passedEventProcessor && passedEventProcessor({} as unknown as any, {});

expect(processedEvent).toBeDefined();
expect(processedEvent).toEqual({ modules: { svelteKit: 'latest' } });
});

it("doesn't add anything to the event, if SvelteKit was not detected", () => {
document.body.innerHTML = '';
detectAndReportSvelteKit();

const processedEvent = passedEventProcessor && passedEventProcessor({} as unknown as any, {});

expect(processedEvent).toBeDefined();
expect(processedEvent).toEqual({});
});

describe('isSvelteKitApp()', () => {
it('returns true if the svelte-announcer div is present', () => {
document.body.innerHTML += '<div id="svelte-announcer">Home</div>';
expect(isSvelteKitApp()).toBe(true);
});
it('returns false if the svelte-announcer div is not present (but similar elements)', () => {
document.body.innerHTML += '<div id="svelte-something">Home</div>';
expect(isSvelteKitApp()).toBe(false);
});
it('returns false if no div is present', () => {
document.body.innerHTML = '';
expect(isSvelteKitApp()).toBe(false);
});
});
});
13 changes: 3 additions & 10 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable max-lines */
import { Hub } from '@sentry/hub';
import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, logger, parseBaggageSetMutability } from '@sentry/utils';
import { getDomElement, getGlobalObject, logger, parseBaggageSetMutability } from '@sentry/utils';

import { startIdleTransaction } from '../hubextensions';
import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT } from '../idletransaction';
Expand Down Expand Up @@ -294,13 +294,6 @@ export function extractTraceDataFromMetaTags(): Partial<TransactionContext> | un

/** Returns the value of a meta tag */
export function getMetaContent(metaName: string): string | null {
const globalObject = getGlobalObject<Window>();

// DOM/querySelector is not available in all environments
if (globalObject.document && globalObject.document.querySelector) {
const el = globalObject.document.querySelector(`meta[name=${metaName}]`);
return el ? el.getAttribute('content') : null;
} else {
return null;
}
const metaTag = getDomElement(`meta[name=${metaName}]`);
return metaTag ? metaTag.getAttribute('content') : null;
}
18 changes: 18 additions & 0 deletions packages/utils/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,21 @@ export function getLocationHref(): string {
return '';
}
}

/**
* Gets a DOM element by using document.querySelector.
*
* This wrapper will first check for the existance of the function before
* actually calling it so that we don't have to take care of this check,
* every time we want to access the DOM.
* Reason: DOM/querySelector is not available in all environments
*
* @param selector the selector string passed on to document.querySelector
*/
export function getDomElement(selector: string): Element | null {
const global = getGlobalObject<Window>();
if (global.document && global.document.querySelector) {
return global.document.querySelector(selector);
}
return null;
}
12 changes: 11 additions & 1 deletion packages/utils/test/browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { JSDOM } from 'jsdom';

import { htmlTreeAsString } from '../src/browser';
import { getDomElement, htmlTreeAsString } from '../src/browser';

beforeAll(() => {
const dom = new JSDOM();
Expand Down Expand Up @@ -45,3 +45,13 @@ describe('htmlTreeAsString', () => {
);
});
});

describe('getDomElement', () => {
it('returns the element for a given query selector', () => {
document.head.innerHTML = '<div id="mydiv">Hello</div>';
const el = getDomElement('div#mydiv');
expect(el).toBeDefined();
expect(el?.tagName).toEqual('DIV');
expect(el?.id).toEqual('mydiv');
});
});

0 comments on commit 74db527

Please sign in to comment.