Skip to content

feat(tracing): Add transaction name to tracestate value #3937

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

Conversation

lobsterkatie
Copy link
Member

This adds transaction name to the tracetate value.

Doing this has two known limitations, which, though we've discussed them, I'm adding here for posterity:

  1. Adding this data puts us over the character limit for tracestate values listed in the W3C spec (256 characters). To be fair, depending on the data, we might already be over this limit with the previous addition of user . For reference:

    Given the tracestate data:

    {
      trace_id: "12312012123120121231201212312012",
      environment: dogpark,
      release: off.leash.park,
      public_key: dogsarebadatkeepingsecrets,
      user: {id: 12312013, segment: "bigs"},
      transaction: /interactions/other-dogs/new-dog/
    }
    

    tracestate with without either: 196 characters
    tracestate with user data: 256 characters
    tracestate with transaction: 264 characters
    tracestate with both: 324 characters

  2. This data may change and/or get added to the scope after the tracestate value has been calculated, making it impossible to sample based on those attributes. This is especially a problem for transaction name, which in some frameworks isn't set to its final value until the transaction ends. This poses the added problem that the transaction name in its raw, un-finalized form may contain PII, because it is often the raw URL as opposed to the parameterized one (so, /users/maisey/tricks/ rather than /users/:username/tricks/). More work needs to be done to investigate whether the final transaction name can be set earlier in any/all of the frameworks where this poses a problem. (For instance, it is a known problem in our Express integration, but not yet clear if it is a problem in any Python frameworks.)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 30, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.9 KB (+0.01% 🔺)
@sentry/browser - Webpack 22.86 KB (0%)
@sentry/react - Webpack 22.89 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 30.07 KB (+0.05% 🔺)

@lobsterkatie lobsterkatie force-pushed the kmclb-add-tracestate-header-handling branch 2 times, most recently from 26bdee1 to c1f86c4 Compare August 31, 2021 20:28
@lobsterkatie lobsterkatie force-pushed the kmclb-tracestate-add-transaction-name branch from 2a3e800 to 096b19f Compare August 31, 2021 22:21
@lobsterkatie lobsterkatie merged commit c25a312 into kmclb-add-tracestate-header-handling Sep 1, 2021
@lobsterkatie lobsterkatie deleted the kmclb-tracestate-add-transaction-name branch September 1, 2021 04:48
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.

2 participants