Skip to content
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

feat(nuxt): Filter out Nuxt build assets #13148

Merged
merged 6 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"build": "nuxt build",
"dev": "nuxt dev",
"generate": "nuxt generate",
"preview": "nuxt preview",
"preview": "NODE_OPTIONS='--import ./public/instrument.server.mjs' nuxt preview",
"clean": "npx nuxi cleanup",
"test": "playwright test",
"test:build": "pnpm install && npx playwright install && pnpm build",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import * as Sentry from '@sentry/nuxt';

Sentry.init({
dsn: 'https://[email protected]/1337',
environment: 'qa', // dynamic sampling bias to keep transactions
tracesSampleRate: 1.0, // Capture 100% of the transactions
tunnel: 'http://localhost:3031/', // proxy server
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';

test('sends a server action transaction on pageload', async ({ page }) => {
const transactionPromise = waitForTransaction('nuxt-3', transactionEvent => {
return transactionEvent.transaction.includes('GET /test-param/');
});

await page.goto('/test-param/1234');

const transaction = await transactionPromise;

expect(transaction.contexts.trace).toEqual(
expect.objectContaining({
data: expect.objectContaining({
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.http',
}),
}),
);
});

test('does not send transactions for build asset folder "_nuxt"', async ({ page }) => {
let buildAssetFolderOccurred = false;

waitForTransaction('nuxt-3', transactionEvent => {
if (transactionEvent.transaction.includes('/_nuxt/')) {
buildAssetFolderOccurred = true;
}
return false; // expects to return a boolean (but not relevant here)
});

const transactionEventPromise = waitForTransaction('nuxt-3', transactionEvent => {
return transactionEvent.transaction.includes('GET /test-param/');
});

await page.goto('/test-param/1234');

const transactionEvent = await transactionEventPromise;

expect(buildAssetFolderOccurred).toBe(false);

// todo: url not yet parametrized
expect(transactionEvent.transaction).toBe('GET /test-param/1234');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this is the case here - it shows /test-param/* in the UI but 1234 here. Relay seems to pick that up, but shouldn't the SDK as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that because we aren't hitting relay, but our tunnel?

expect(buildAssetFolderOccurred).toBe(false);
});
30 changes: 27 additions & 3 deletions packages/nuxt/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { applySdkMetadata } from '@sentry/core';
import { applySdkMetadata, getGlobalScope } from '@sentry/core';
import { init as initNode } from '@sentry/node';
import type { Client } from '@sentry/types';
import type { Client, EventProcessor } from '@sentry/types';
import type { SentryNuxtOptions } from '../common/types';

/**
Expand All @@ -15,5 +15,29 @@ export function init(options: SentryNuxtOptions): Client | undefined {

applySdkMetadata(sentryOptions, 'nuxt', ['nuxt', 'node']);

return initNode(sentryOptions);
const client = initNode(sentryOptions);

getGlobalScope().addEventProcessor(
Object.assign(
(event => {
if (event.type === 'transaction') {
// Filter out transactions for Nuxt build assets
// This regex matches the default path to the nuxt-generated build assets (`_nuxt`).
if (event.transaction?.match(/^GET \/_nuxt\//)) {
options.debug &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Maybe let's add a small TODO comment here to dynamically match this path based on the nuxt build output folder so that we don't forget.

// eslint-disable-next-line no-console
console.log('[Sentry] NuxtLowQualityTransactionsFilter filtered transaction: ', event.transaction);
return null;
}

return event;
} else {
return event;
}
}) satisfies EventProcessor,
{ id: 'NuxtLowQualityTransactionsFilter' },
),
);

return client;
}
42 changes: 42 additions & 0 deletions packages/nuxt/test/server/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as SentryNode from '@sentry/node';
import { SDK_VERSION } from '@sentry/node';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { init } from '../../src/server';

const nodeInit = vi.spyOn(SentryNode, 'init');

describe('Nuxt Server SDK', () => {
describe('init', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('Adds Nuxt metadata to the SDK options', () => {
expect(nodeInit).not.toHaveBeenCalled();

init({
dsn: 'https://[email protected]/1337',
});

const expectedMetadata = {
_metadata: {
sdk: {
name: 'sentry.javascript.nuxt',
version: SDK_VERSION,
packages: [
{ name: 'npm:@sentry/nuxt', version: SDK_VERSION },
{ name: 'npm:@sentry/node', version: SDK_VERSION },
],
},
},
};

expect(nodeInit).toHaveBeenCalledTimes(1);
expect(nodeInit).toHaveBeenLastCalledWith(expect.objectContaining(expectedMetadata));
});

it('returns client from init', () => {
expect(init({})).not.toBeUndefined();
});
});
});
Loading