Specify level for sentry enrichments

[alesso-x]

trafficstars

I would like to scope certain sentry enrichments to only appear on error. It would be useful for includeFetchResult, includeError, includeVariables, setTransaction, setFingerprint, etc to be included when there is an error from a query. includeQuery is useful for all sentry issues in my case.

Possible solution

@spawnia suggested to add optional strings for each level i.e. includeFetchResult: 'on-error'. Building off that idea, I would turn it into a list and my ideal config would look like

const sentryLink = new SentryLink({
    setFingerprint: ['on-error'],
    setTransaction: ['on-error'],
    attachBreadcrumbs: {
      includeQuery: ['on-error', 'on-success'],
      includeVariables: ['on-error'],
      includeFetchResult: ['on-error'],
      includeError:  ['on-error'],
    },
  })

Prior thread https://github.com/DiederikvandenB/apollo-link-sentry/issues/278#issuecomment-783517465

[spawnia]

After using this for a while, i found the use of setTransaction to be a complete semantic mismatch. I am using routes as transactions for now and turned it off. I am starting to think this option makes little sense at all, given that requests can happen concurrently.

Setting setFingerprint has a similar issue when concurrent requests are involved, it randomly split some errors I had that were unrelated to the current queries. Again, concurrency makes the results totally unpredictable. That said, including it on errors seems sensible, perhaps the only sensible option.

I do like the conditional includes for breadcrumbs. Data volume and size do play a role here, it makes sense to surface as much as possible when errors happen and just have minimal info in other cases.

My proposal to go ahead would be to:

• turn off setTransaction by default, perhaps deprecate it
• add on-error as the new default for setFingerprint
• add on-error to options within attachBreadcrumbs, perhaps also as default for some.

[DiederikvandenB]

Thanks for both of your efforts in improving this package.

With regards to setTransaction, I agree that it should be disabled by default. To give a little context, when I was writing this package, the app I was writing it for had no concurrent requests. Or maybe it did, but it never happened that two requests failed. Anyway, I do still think the option is useful for the scenario in which I applied it to. We'll want to reference this discussion in the docs, once we change the default.

I think updating the options with an on-error makes sense, but I'm not too comfortable with the notation yet. I will think about this some more tomorrow or Wednesday.

[watadarkstar]

I think I'm asking for the same thing in issue https://github.com/DiederikvandenB/apollo-link-sentry/issues/301 as this issue is asking for, right?