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(core): Add dropEvent options to moduleMetadataIntegration #11718

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Apr 22, 2024

This additional option can be used to drop events that don't match specific module metadata on any stack frames:

import { init, moduleMetadataIntegration }  from '@sentry/browser';

init({
  dsn: 'https://[email protected]/0',
  integrations: [
    moduleMetadataIntegration({
      dropEvent: {
        ifNoStackFrameMetadataMatches: metadata => metadata?.team === 'frontend'
      }
    })
  ],
});

Copy link
Contributor

github-actions bot commented Apr 22, 2024

size-limit report 📦

Path Size
@sentry/browser 21.64 KB (0%)
@sentry/browser (incl. Tracing) 32.68 KB (0%)
@sentry/browser (incl. Tracing, Replay) 68.03 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.43 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 72.07 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 84.21 KB (0%)
@sentry/browser (incl. Feedback) 37.77 KB (0%)
@sentry/browser (incl. sendFeedback) 26.43 KB (0%)
@sentry/browser (incl. FeedbackAsync) 30.9 KB (0%)
@sentry/react 24.33 KB (0%)
@sentry/react (incl. Tracing) 35.64 KB (0%)
@sentry/vue 25.47 KB (0%)
@sentry/vue (incl. Tracing) 34.47 KB (0%)
@sentry/svelte 21.77 KB (0%)
CDN Bundle 23.95 KB (0%)
CDN Bundle (incl. Tracing) 33.98 KB (0%)
CDN Bundle (incl. Tracing, Replay) 67.67 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 83.58 KB (0%)
CDN Bundle - uncompressed 70.58 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 100.94 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 210.55 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 257.02 KB (0%)
@sentry/nextjs (client) 34.86 KB (0%)
@sentry/sveltekit (client) 33.24 KB (0%)
@sentry/node 138.47 KB (-0.01% 🔽)

@timfish timfish requested a review from lforst April 22, 2024 15:59
@timfish timfish marked this pull request as ready for review April 22, 2024 16:15
@timfish timfish requested a review from AbhiPrasad April 23, 2024 19:59
enableSend: true,
stackParser,
integrations: [
moduleMetadataIntegration({ dropEvent: { ifNoStackFrameMetadataMatches: m => m?.team === 'backend' } }),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
moduleMetadataIntegration({ dropEvent: { ifNoStackFrameMetadataMatches: m => m?.team === 'backend' } }),
moduleMetadataIntegration({ dropEvent: { ifNoStackFrameMetadataMatches: m => m.team === 'backend' } }),

or can m be undefined here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've changed the types to reflect that the parameter can be undefined. This allows users to filter on whether the metadata for a frame is defined!

@lforst lforst self-assigned this May 27, 2024
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

I am gonna leave a "request changes" here because I would like to still dwell a bit on the exact API here. I think in general I would like this entire functionality to be a completely separate integration with a separate option in the bundler plugins. That would make the documentation and reasoning a tiny bit easier.

@timfish
Copy link
Collaborator Author

timfish commented May 27, 2024

Yep, no problem.

Would this flag still be stored under the same globalThis._sentryModuleMetadata or use an extra global store?

Any preferences over the integration name and corresponding bundler plugin option?

@lforst
Copy link
Member

lforst commented May 27, 2024

Would this flag still be stored under the same globalThis._sentryModuleMetadata or use an extra global store for this?

Under the hood I think it would still make use of globalThis._sentryModuleMetadata, simply because the entire architecture is already prepared for that.

Any preferences over the integration name and corresponding bundler plugin option?

The following names come to mind for the integration (happy to receive other suggestions):

  • thirdPartyErrorsFilterIntegration
  • stackTraceIntegrityFilterIntegration
  • filterGarbageErrorsIntegration

I think the option in the bundler plugins should be called something like:

  • codeIdentifier
  • codeIntegrityIdentifier
  • codeIntegrityFilterKey

The options of the integration should probably be something as follows:

interface Options {
    /**
     * Keys that have been provided in the Sentry bundler plugin as integrity keys.
     */
    filterKeys: string[],
    
    /**
     * Defines how the integration should behave:
     * 
     * - `drop-if-some-frames-not-matched`: Drops all error events that contain stack frames that did not come from files marked with a matching integrity key.
     * - `drop-if-all-frames-not-matched`: Drops all error events exclusively contain stack frames that did not come from files marked with a matching integrity key
     * - `apply-tag-if-some-frames-not-matched`: Keep all events, but apply a `not-application-code: True` tag in case some frames did not come from user code.
     * - `apply-tag-if-all-frames-not-matched`: Keep all events, but apply a `not-application-code: True` tag in case ale frames did not come from user code.
     */
    behaviour: 'drop-if-some-frames-not-matched' | 'drop-if-all-frames-not-matched' | 'apply-tag-if-some-frames-not-matched' | 'apply-tag-if-all-frames-not-matched'
}

We can also add some more behaviours that only look at the top stack frame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants