Skip to content

Add allowList for the tracestate header #1697

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

Closed
marandaneto opened this issue Sep 2, 2021 · 8 comments · Fixed by #1698
Closed

Add allowList for the tracestate header #1697

marandaneto opened this issue Sep 2, 2021 · 8 comments · Fixed by #1698
Assignees
Labels
performance Performance API issues

Comments

@marandaneto
Copy link
Contributor

marandaneto commented Sep 2, 2021

related to #1683
reason is getsentry/develop#425

cc @bruno-garcia

it should be a List in the SentryOptions with an experimental annotation

@marandaneto marandaneto added the performance Performance API issues label Sep 2, 2021
@marandaneto
Copy link
Contributor Author

@brustolin we'd need that on getsentry/sentry-cocoa#1291 too, we need to align naming and so on

@lobsterkatie
Copy link
Member

reason is getsentry/develop#425

The PII part or the incorrect data part? Can you expand upon this a little?

@marandaneto
Copy link
Contributor Author

ups, the PII part, we don't want to send the tracestate that may contain PII data when the user is making an API call, that's why we'd like to introduce an allow list that only sends that info if the target URL is in the allow list, that's the reason IIRC, correct me if I am wrong @bruno-garcia

@lobsterkatie
Copy link
Member

Ah, got it. The equivalent to JS's tracingOrigins option.

Thanks.

@marandaneto
Copy link
Contributor Author

its prob a dup of #1585 then
in case we want to reuse the same list, wdyt @bruno-garcia ?

@maciejwalkowiak
Copy link
Contributor

So I am naming this new property tracingOrigins. Unlike Javascript, I guess it should not have ['localhost', /^\//] as a default value.

What is the expected behavior if no tracing origins are set? Attach headers to every request, or do not attach headers to any request.

If the latter, this would be a breaking change.

@marandaneto
Copy link
Contributor Author

I'd say if it's empty, send it to every request, and it's up to the user either to decide to add just a specific URL or not, otherwise, it'd not be scalable to the final user adding a new entry to tracingOrigins in case they want everything but they now have a new external call.

@bruno-garcia
Copy link
Member

I'd say if it's empty, send it to every request, and it's up to the user either to decide to add just a specific URL or not, otherwise, it'd not be scalable to the final user adding a new entry to tracingOrigins in case they want everything but they now have a new external call.

I agree. It's important to make this very clear in the docs

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

Successfully merging a pull request may close this issue.

4 participants