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

fix(tracing): add new transaction source #6068

Closed

Conversation

ys-zhifu
Copy link

In my opinion, the source value of customer should not be used directly, which will result in the user being unable to customize the value, so a new initialization value is required to distinguish whether it is modified or not.

Finally, the user can use the source of customer when he wants to modify context.name.

new Integrations.BrowserTracing({
        beforeNavigate: context => {
          context.name = 'test-transaction-name'
          context.metadata = {
            source: 'custom'
          }
          return context
        },
        routingInstrumentation: Sentry.vueRouterInstrumentation(router)
}),

@ys-zhifu
Copy link
Author

This may be a good result, of course, if the changes are not significant.

@Lms24
Copy link
Member

Lms24 commented Oct 27, 2022

Hi @ys-zhifu and thanks for opening this PR!

I am sorry but I don't think we can merge this in because our transaction source specification (and here) is very strict and we cannot just add another source type (initialize).

Let me explain what source is to us and why we use it:

We use the transaction source to internally differentiate and describe how the name of the transaction was set. For instance, transaction source url means that the name of the transaction is a raw URL (e.g. /users/123/details, /users/456/details). If we manage to parameterize a URL, we set the source to route, meaning that we now have a proper, parameterized route as transaction name (e.g. /users/:userId/details). This is what happens in our Vue routing instrumentation without user intervention.

In case users give their transaction a custom name by using beforeNavigate or transaction.setName, we'll set the source to custom, meaning that the transaction name was changed by the user. In that case, we will no longer update it.

The reason why we introduced transaction source was because we needed a way to distinguish between these name types for trace propagation and dynamic sampling. We never intended to let users change the transaction source themselves.

I'm seeing that this is your second PR for the Vue router instrumentation. Would you mind telling me if the problem we fixed in #6060 still persists for you after our update? Our latest release (7.17.0) unfortunately is broken but we'll have a fix out soon. Then you can give it a try.

@AbhiPrasad
Copy link
Member

We should have released the fix with 7.17.1. As such, closing this PR.

@AbhiPrasad AbhiPrasad closed this Oct 27, 2022
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