Skip to content

feat(vue): Deprecate configuring Vue tracing options anywhere else other than through the vueIntegration's tracingOptions option #14385

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.

Sign up for GitHub

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 4 commits into from
Nov 25, 2024
Merged

feat(vue): Deprecate configuring Vue tracing options anywhere else other than through the vueIntegration's tracingOptions option #14385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e1c8af
feat(vue): Deprecate configuring Vue tracing options anywhere else ot…
lforst Nov 20, 2024
1a730e3
Merge remote-tracking branch 'origin/develop' into lforst-vue-tracing…
lforst Nov 21, 2024
57a0cb4
test
lforst Nov 21, 2024
70b589e
Merge remote-tracking branch 'origin/develop' into lforst-vue-tracing…
lforst Nov 25, 2024
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
21 changes: 21 additions & 0 deletions docs/migration/draft-v9-migration-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@

- Deprecated `Request` in favor of `RequestEventData`.

## `@sentry/vue`

- Deprecated `tracingOptions`, `trackComponents`, `timeout`, `hooks` options everywhere other than in the `tracingOptions` option of the `vueIntegration()`.
These options should now be set as follows:

```ts
import * as Sentry from '@sentry/vue';

Sentry.init({
integrations: [
Sentry.vueIntegration({
tracingOptions: {
trackComponents: true,
Copy link
Member

Choose a reason for hiding this comment

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

the only thing we could think about elevating to root-level config IMHO is trackComponents, this seems somewhat important enough to possibly warrant this (and also should not be too confusing). So IMHO we could still allow this specifically to also be passed directly to init(), in addition to allowing to pass it to the integration, but no strong feelings.

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 also admittedly don't have hard feelings towards this. I would err towards not allowing it top-level because it opens the door for things tree-shaking in the future, and less ambiguous docs.

@Lms24 @s1gr1d do you have opinions on whether or not we should still allow trackComponents as a top-level Sentry.init() option?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer keeping it integration only because if Sentry.vueIntegration is filtered out, then trackComponents doesn't do anything.

Copy link
Member

@s1gr1d s1gr1d Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I am with Abhi on that, but in Nuxt the vueIntegration is added by the SDK not the user, so I would have to introduce an option in the Nuxt SDK init() which still allows overriding the vueIntegration options.

Something like vueOptions?

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 opened a PR to update the docs. That will maybe highlight a bit better what the config will look like: getsentry/sentry-docs#11917

Copy link
Member

Choose a reason for hiding this comment

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

Component tracking options are mostly defined outside the Sentry.init options across our SDKs. I think it's fine to only have them at integration level in Vue. For Nuxt, we probably want to figure out a way of still letting users change things but I'd argue this is a long-term goal. AFAIK, we don't have docs on component tracking in meta framework SDKs at all at the moment.

Copy link
Member

@s1gr1d s1gr1d Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I see how you envisioned it for Nuxt - users add the vueIntegration themselves. However, I think that would not work here, as the Nuxt SDK already adds the vueIntegration internally to setup the Vue router. This happens here:

sentry-javascript/packages/nuxt/src/runtime/plugins/sentry.client.ts

Line 56 in 472c228

sentryClient.addIntegration(vueIntegration({ app: vueApp, attachErrorHandler: false }));

It happens at this place in the Nuxt SDK as we need to pass the vueApp and this is the earliest place to do so (the user cannot pass it).

So we would still have to add something like a vueOptions key for the Nuxt SDK init as we cannot call the integration twice.

Copy link
Member

Choose a reason for hiding this comment

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

So we would still have to add something like a vueOptions key for the Nuxt SDK init as we cannot call the integration twice.

i think that's fine

timeout: 1000,
hooks: ['mount', 'update', 'unmount'],
},
}),
],
});
```

## Server-side SDKs (`@sentry/node` and all dependents)

- Deprecated `processThreadBreadcrumbIntegration` in favor of `childProcessIntegration`. Functionally they are the same.
Expand Down
52 changes: 21 additions & 31 deletions packages/vue/src/integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { defineIntegration, hasTracingEnabled } from '@sentry/core';
import { GLOBAL_OBJ, consoleSandbox } from '@sentry/core';
import type { Client, IntegrationFn } from '@sentry/types';

import { GLOBAL_OBJ, consoleSandbox, defineIntegration, hasTracingEnabled } from '@sentry/core';
import { DEFAULT_HOOKS } from './constants';
import { DEBUG_BUILD } from './debug-build';
import { attachErrorHandler } from './errorhandler';
Expand All @@ -22,38 +19,30 @@ const DEFAULT_CONFIG: VueOptions = {

const INTEGRATION_NAME = 'Vue';

const _vueIntegration = ((integrationOptions: Partial<VueOptions> = {}) => {
export const vueIntegration = defineIntegration((integrationOptions: Partial<VueOptions> = {}) => {
return {
name: INTEGRATION_NAME,
setup(client) {
_setupIntegration(client, integrationOptions);
const options: Options = { ...DEFAULT_CONFIG, ...client.getOptions(), ...integrationOptions };
if (!options.Vue && !options.app) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
'[@sentry/vue]: Misconfigured SDK. Vue specific errors will not be captured. Update your `Sentry.init` call with an appropriate config option: `app` (Application Instance - Vue 3) or `Vue` (Vue Constructor - Vue 2).',
);
});
return;
}

if (options.app) {
const apps = Array.isArray(options.app) ? options.app : [options.app];
apps.forEach(app => vueInit(app, options));
} else if (options.Vue) {
vueInit(options.Vue, options);
}
},
};
}) satisfies IntegrationFn;

export const vueIntegration = defineIntegration(_vueIntegration);

function _setupIntegration(client: Client, integrationOptions: Partial<VueOptions>): void {
const options: Options = { ...DEFAULT_CONFIG, ...client.getOptions(), ...integrationOptions };
if (!options.Vue && !options.app) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[@sentry/vue]: Misconfigured SDK. Vue specific errors will not be captured.
Update your \`Sentry.init\` call with an appropriate config option:
\`app\` (Application Instance - Vue 3) or \`Vue\` (Vue Constructor - Vue 2).`,
);
});
return;
}

if (options.app) {
const apps = Array.isArray(options.app) ? options.app : [options.app];
apps.forEach(app => vueInit(app, options));
} else if (options.Vue) {
vueInit(options.Vue, options);
}
}
});

const vueInit = (app: Vue, options: Options): void => {
if (DEBUG_BUILD) {
Expand Down Expand Up @@ -85,6 +74,7 @@ const vueInit = (app: Vue, options: Options): void => {
app.mixin(
createTracingMixins({
...options,
// eslint-disable-next-line deprecation/deprecation
...options.tracingOptions,
}),
);
Expand Down
59 changes: 57 additions & 2 deletions packages/vue/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export type ViewModel = {
};
};

export interface VueOptions extends TracingOptions {
export interface VueOptions {
/** Vue constructor to be used inside the integration (as imported by `import Vue from 'vue'` in Vue2) */
Vue?: Vue;

Expand Down Expand Up @@ -60,9 +60,64 @@ export interface VueOptions extends TracingOptions {

/** {@link TracingOptions} */
tracingOptions?: Partial<TracingOptions>;

/**
* Decides whether to track components by hooking into its lifecycle methods.
* Can be either set to `boolean` to enable/disable tracking for all of them.
* Or to an array of specific component names (case-sensitive).
*
* @deprecated Use tracingOptions
*/
trackComponents: boolean | string[];

/**
* How long to wait until the tracked root activity is marked as finished and sent of to Sentry
*
* @deprecated Use tracingOptions
*/
timeout: number;

/**
* List of hooks to keep track of during component lifecycle.
* Available hooks: 'activate' | 'create' | 'destroy' | 'mount' | 'unmount' | 'update'
* Based on https://vuejs.org/v2/api/#Options-Lifecycle-Hooks
*
* @deprecated Use tracingOptions
*/
hooks: Operation[];
}

export interface Options extends BrowserOptions, VueOptions {}
export interface Options extends BrowserOptions, VueOptions {
/**
* @deprecated Use `vueIntegration` tracingOptions
*/
tracingOptions?: Partial<TracingOptions>;

/**
* Decides whether to track components by hooking into its lifecycle methods.
* Can be either set to `boolean` to enable/disable tracking for all of them.
* Or to an array of specific component names (case-sensitive).
*
* @deprecated Use `vueIntegration` tracingOptions
*/
trackComponents: boolean | string[];

/**
* How long to wait until the tracked root activity is marked as finished and sent of to Sentry
*
* @deprecated Use `vueIntegration` tracingOptions
*/
timeout: number;

/**
* List of hooks to keep track of during component lifecycle.
* Available hooks: 'activate' | 'create' | 'destroy' | 'mount' | 'unmount' | 'update'
* Based on https://vuejs.org/v2/api/#Options-Lifecycle-Hooks
*
* @deprecated Use `vueIntegration` tracingOptions
*/
hooks: Operation[];
}

/** Vue specific configuration for Tracing Integration */
export interface TracingOptions {
Expand Down
4 changes: 1 addition & 3 deletions packages/vue/test/integration/init.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ describe('Sentry.init', () => {
app.mount(el);

expect(warnings).toEqual([
`[@sentry/vue]: Misconfigured SDK. Vue specific errors will not be captured.
Update your \`Sentry.init\` call with an appropriate config option:
\`app\` (Application Instance - Vue 3) or \`Vue\` (Vue Constructor - Vue 2).`,
'[@sentry/vue]: Misconfigured SDK. Vue specific errors will not be captured. Update your `Sentry.init` call with an appropriate config option: `app` (Application Instance - Vue 3) or `Vue` (Vue Constructor - Vue 2).',
]);
});

Expand Down
Loading