Skip to content

Commit

Permalink
fix: make it possible to disable internal SDK integrations (#655)
Browse files Browse the repository at this point in the history
  • Loading branch information
rchl committed Nov 27, 2023
1 parent 7dbeffb commit fd32a7e
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 19 deletions.
15 changes: 15 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const tsParser = require('@typescript-eslint/parser')

module.exports = {
extends: [
'@nuxtjs/eslint-config',
Expand All @@ -15,6 +17,19 @@ module.exports = {
'vue/multi-word-component-names': 'off',
},
overrides: [
{
files: ['*.vue'],
parserOptions: {
parser: {
// Overrides script parser for `<script lang="ts">`
ts: tsParser,
},
},
rules: {
'no-undef': 'off',
'no-unused-vars': 'off',
},
},
{
files: ['*.ts', '*.tsx'],
extends: [
Expand Down
24 changes: 20 additions & 4 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,14 @@ const SERVER_PLUGGABLE_INTEGRATIONS = ['CaptureConsole', 'Debug', 'Dedupe', 'Ext
// External and optional Node.js integration - https://docs.sentry.io/platforms/node/profiling/
export const SERVER_PROFILING_INTEGRATION: keyof ProfilingIntegration = 'ProfilingIntegration'

function filterDisabledIntegrations<T extends AllIntegrations> (integrations: T): (keyof T)[] {
function getEnabledIntegrations<T extends AllIntegrations> (integrations: T): (keyof T)[] {
return getIntegrationsKeys(integrations).filter(key => integrations[key])
}

function getDisabledIntegrationKeys<T extends AllIntegrations> (integrations: T): string[] {
return getIntegrationsKeys(integrations).filter(key => integrations[key] === false) as string[]
}

function getIntegrationsKeys<T extends AllIntegrations> (integrations: T): (keyof T)[] {
return Object.keys(integrations) as (keyof T)[]
}
Expand Down Expand Up @@ -158,6 +162,7 @@ export type ResolvedClientOptions = {
BROWSER_PLUGGABLE_INTEGRATIONS: string[]
BROWSER_VUE_INTEGRATIONS: string[]
dev: boolean
DISABLED_INTEGRATION_KEYS: string[]
runtimeConfigKey: string
config: Options
lazy: boolean | LazyConfiguration
Expand Down Expand Up @@ -213,13 +218,14 @@ export async function resolveClientOptions (nuxt: Nuxt, moduleOptions: Readonly<
...options.config,
},
clientConfigPath,
DISABLED_INTEGRATION_KEYS: getDisabledIntegrationKeys(options.clientIntegrations),
lazy: options.lazy,
apiMethods,
customClientIntegrations,
logMockCalls: options.logMockCalls, // for mocked only
tracing: options.tracing,
initialize: canInitialize(options),
integrations: filterDisabledIntegrations(options.clientIntegrations)
integrations: getEnabledIntegrations(options.clientIntegrations)
.reduce((res, key) => {
res[key] = options.clientIntegrations[key]
return res
Expand Down Expand Up @@ -290,10 +296,10 @@ export async function resolveServerOptions (nuxt: Nuxt, moduleOptions: Readonly<
}
}

options.config.integrations = [
const resolvedIntegrations = [
// Automatically instrument Node.js libraries and frameworks
...(options.tracing ? autoDiscoverNodePerformanceMonitoringIntegrations() : []),
...filterDisabledIntegrations(options.serverIntegrations)
...getEnabledIntegrations(options.serverIntegrations)
.map((name) => {
const opt = options.serverIntegrations[name]
try {
Expand All @@ -314,6 +320,16 @@ export async function resolveServerOptions (nuxt: Nuxt, moduleOptions: Readonly<
...customIntegrations,
]

const disabledIntegrationKeys = getDisabledIntegrationKeys(options.serverIntegrations)

// Use a function to be able to filter out default integrations.
options.config.integrations = (defaultIntegrations) => {
return [
...defaultIntegrations.filter(integration => !disabledIntegrationKeys.includes(integration.name)),
...resolvedIntegrations,
]
}

return {
config: {
dsn: options.dsn,
Expand Down
17 changes: 12 additions & 5 deletions src/templates/client.shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ if (integrations.length) {%>import { <%= integrations.join(', ') %> } from '~@se
export { init }
export const SentrySdk = { ...CoreSdk, ...BrowserSdk }

/* eslint-disable object-curly-spacing, quote-props, quotes, key-spacing, comma-spacing */
const DISABLED_INTEGRATION_KEYS = <%= serialize(options.DISABLED_INTEGRATION_KEYS) %>

export<%= (options.clientConfigPath || options.customClientIntegrations) ? ' async' : '' %> function getConfig (ctx) {
/* eslint-disable object-curly-spacing, quote-props, quotes, key-spacing, comma-spacing */
const config = {
<%= Object
.entries(options.config)
Expand All @@ -39,7 +41,7 @@ export<%= (options.clientConfigPath || options.customClientIntegrations) ? ' asy
<% if (browserIntegrations.length) {%>
const { <%= browserIntegrations.join(', ') %> } = Integrations
<%}%>
config.integrations = [
const resolvedIntegrations = [
<%= Object
.entries(options.integrations)
.filter(([name]) => name !== 'Vue')
Expand All @@ -57,7 +59,7 @@ export<%= (options.clientConfigPath || options.customClientIntegrations) ? ' asy
]
<% if (options.tracing) { %>
const { browserTracing, vueOptions, vueRouterInstrumentationOptions, ...tracingOptions } = <%= serialize(options.tracing) %>
config.integrations.push(new BrowserTracing({
resolvedIntegrations.push(new BrowserTracing({
...(ctx.app.router ? { routingInstrumentation: vueRouterInstrumentation(ctx.app.router, vueRouterInstrumentationOptions) } : {}),
...browserTracing,
}))
Expand All @@ -72,12 +74,17 @@ export<%= (options.clientConfigPath || options.customClientIntegrations) ? ' asy
<% if (options.customClientIntegrations) { %>
const customIntegrations = await getCustomIntegrations(ctx)
if (Array.isArray(customIntegrations)) {
config.integrations.push(...customIntegrations)
resolvedIntegrations.push(...customIntegrations)
} else {
console.error(`[@nuxtjs/sentry] Invalid value returned from customClientIntegrations plugin. Expected an array, got "${typeof customIntegrations}".`)
}
<% } %>

config.integrations = (defaultIntegrations) => {
return [
...defaultIntegrations.filter(integration => !DISABLED_INTEGRATION_KEYS.includes(integration.name)),
...resolvedIntegrations,
]
}
const runtimeConfigKey = <%= serialize(options.runtimeConfigKey) %>
if (ctx.$config && runtimeConfigKey && ctx.$config[runtimeConfigKey]) {
merge(config, ctx.$config[runtimeConfigKey].config, ctx.$config[runtimeConfigKey].clientConfig)
Expand Down
2 changes: 1 addition & 1 deletion test/default.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('Smoke test (default)', () => {

beforeAll(async () => {
await localServer.start(TEST_DSN)
const dsn = localServer.getDsn()
const dsn = localServer.getDsn() ?? undefined
nuxt = (await setup(loadConfig(__dirname, 'default', { sentry: { dsn } }, { merge: true }))).nuxt
browser = await createBrowser()
})
Expand Down
4 changes: 4 additions & 0 deletions test/fixture/typescript/nuxt.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const config: NuxtConfig = {
// Integration from @Sentry/browser package.
TryCatch: { eventTarget: false },
Replay: {},
Dedupe: false,
},
serverIntegrations: {
Modules: false,
},
clientConfig: {
// This sets the sample rate to be 10%. You may want this to be 100% while
Expand Down
38 changes: 38 additions & 0 deletions test/fixture/typescript/pages/disabled-integrations.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<template>
<div>
<p>Integrations</p>
<p id="client">
Dedupe: {{ clientDedupeDisabled ? 'DISABLED' : 'ENABLED' }}
</p>
<p id="server">
Modules: {{ serverModulesDisabled ? 'DISABLED' : 'ENABLED' }}
</p>
</div>
</template>

<script lang="ts">
import { defineComponent } from 'vue'
import type { BrowserClient } from '@sentry/browser'
import type { NodeClient } from '@sentry/node'
export default defineComponent({
asyncData ({ $sentry }) {
if (process.server) {
return {
serverModulesDisabled: $sentry.getCurrentHub().getClient<NodeClient>()!.getIntegrationById('Modules') === undefined,
}
}
},
data () {
return {
clientDedupeDisabled: false,
serverModulesDisabled: false,
}
},
created () {
if (process.client) {
this.clientDedupeDisabled = this.$sentry.getCurrentHub().getClient<BrowserClient>()!.getIntegrationById('Dedupe') === undefined
}
},
})
</script>
4 changes: 2 additions & 2 deletions test/lazy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { $$, createBrowser, loadConfig } from './utils'
const __filename = fileURLToPath(import.meta.url)
const __dirname = dirname(__filename)

const { testkit, localServer } = sentryTestkit.default()
const { localServer } = sentryTestkit.default()
const TEST_DSN = 'http://acacaeaccacacacabcaacdacdacadaca@sentry.io/000001'

describe('Smoke test (lazy)', () => {
Expand All @@ -19,7 +19,7 @@ describe('Smoke test (lazy)', () => {

beforeAll(async () => {
await localServer.start(TEST_DSN)
const dsn = localServer.getDsn()
const dsn = localServer.getDsn() ?? undefined
nuxt = (await setup(loadConfig(__dirname, 'lazy', { sentry: { dsn } }, { merge: true }))).nuxt
browser = await createBrowser()
})
Expand Down
11 changes: 9 additions & 2 deletions test/options.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { describe, test, expect } from 'vitest'
import { defu } from 'defu'
import { ResolvedClientOptions, ResolvedServerOptions, resolveClientOptions, resolveServerOptions } from '../src/options'
Expand Down Expand Up @@ -115,7 +116,10 @@ describe('Resolve Server Options', () => {
logMockCalls: true,
tracing: false,
})
const integrations = Array.isArray(resolvedOptions.config.integrations) ? resolvedOptions.config.integrations : null
expect(resolvedOptions.config.integrations).not.toBeUndefined()
const integrations = Array.isArray(resolvedOptions.config.integrations)
? resolvedOptions.config.integrations
: resolvedOptions.config.integrations!([])
expect(integrations).toBeTruthy()
expect(integrations?.map(integration => integration.name)).toEqual(expect.arrayContaining(['Dedupe', 'ExtraErrorData', 'RewriteFrames', 'Transaction']))
})
Expand Down Expand Up @@ -158,7 +162,10 @@ describe('Resolve Server Options', () => {
},
},
})
const integrations = Array.isArray(resolvedOptions.config.integrations) ? resolvedOptions.config.integrations : null
expect(resolvedOptions.config.integrations).not.toBeUndefined()
const integrations = Array.isArray(resolvedOptions.config.integrations)
? resolvedOptions.config.integrations
: resolvedOptions.config.integrations!([])
expect(integrations).toBeTruthy()
expect(integrations?.map(integration => integration.name)).toEqual(expect.arrayContaining(['Http', 'Dedupe', 'ExtraErrorData', 'RewriteFrames', 'Transaction']))
})
Expand Down
17 changes: 14 additions & 3 deletions test/typescript.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { dirname } from 'path'
import { describe, afterAll, beforeAll, beforeEach, test, expect } from 'vitest'
import type { Browser } from 'playwright-chromium'
import sentryTestkit from 'sentry-testkit'
// TODO: Until sentry-kit types are fixed
import type { Stacktrace } from '@sentry/node'
import type { NuxtConfig } from '@nuxt/types'
import { generatePort, setup, url } from '@nuxtjs/module-test-utils'
import type { Nuxt } from '../src/kit-shim'
Expand Down Expand Up @@ -82,7 +80,7 @@ describe('Smoke test (typescript)', () => {
const reports = testkit.reports()
expect(reports).toHaveLength(1)
expect(reports[0].error?.message).toContain('apiCrash is not defined')
expect(reports[0].error?.stacktrace as Stacktrace).toMatchObject({
expect(reports[0].error?.stacktrace).toMatchObject({
frames: expect.arrayContaining([
expect.objectContaining({
filename: 'app:///api/index.ts',
Expand All @@ -102,5 +100,18 @@ describe('Smoke test (typescript)', () => {
expect(reports[0].error?.message).toContain('crash_me is not a function')
})

test('can disable integrations that SDK enables by default', async () => {
const page = await browser.newPage()
await page.goto(url('/disabled-integrations'))

const clientParagraph = await $$('#client', page)
expect(clientParagraph).not.toBeNull()
expect(clientParagraph!.trim()).toBe('Dedupe: DISABLED')

const serverParagraph = await $$('#server', page)
expect(serverParagraph).not.toBeNull()
expect(serverParagraph!.trim()).toBe('Modules: DISABLED')
})

// TODO: Add tests for custom integration. Blocked by various sentry-kit bugs reported in its repo.
})
3 changes: 2 additions & 1 deletion test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { fileURLToPath } from 'node:url'
import initJiti from 'jiti'
import { defu } from 'defu'
import { join } from 'pathe'
import { NuxtConfig } from '@nuxt/types'
import { chromium, Browser, Page } from 'playwright-chromium'

const jitiImport = initJiti(fileURLToPath(import.meta.url))
Expand All @@ -22,7 +23,7 @@ export async function $$ (selector: string, page: Page): Promise<string | null>
return null
}

export function loadConfig (dir: string, fixture: string | null = null, override: Record<string, unknown> = {}, { merge = false } = {}): Record<string, unknown> {
export function loadConfig (dir: string, fixture: string | null = null, override: NuxtConfig = {}, { merge = false } = {}): NuxtConfig {
const fixtureConfig = jitiImport(join(dir, 'fixture', fixture ?? '', 'nuxt.config'))
const config = Object.assign({}, fixtureConfig.default || fixtureConfig)

Expand Down
2 changes: 1 addition & 1 deletion test/with-lazy-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Smoke test (lazy config)', () => {

beforeAll(async () => {
await localServer.start(TEST_DSN)
const dsn = localServer.getDsn()
const dsn = localServer.getDsn() ?? undefined
nuxt = (await setup(loadConfig(__dirname, 'with-lazy-config', { sentry: { dsn } }, { merge: true }))).nuxt
browser = await createBrowser()
})
Expand Down

0 comments on commit fd32a7e

Please sign in to comment.