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.

Sign up for GitHub

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

Jump to bottom

Wrap parse_url calls in capture_internal_exceptions #2162

Merged
merged 11 commits into from Jun 13, 2023
Merged

Wrap parse_url calls in capture_internal_exceptions #2162

merged 11 commits into from Jun 13, 2023

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Jun 7, 2023 •
edited

Follow up to #2160

Our URL parsing and sanitizing can fail. If it does, the error must never break the user's application.

This PR:

  • wraps all calls to parse_url in capture_internal_exceptions and skips setting any parsed URL related data on the span (affects the stdlib, boto3, httpx integrations)
  • adds more test cases for the sanitize_url function with split=True

@sentrivana sentrivana marked this pull request as ready for review June 9, 2023 10:24
antonpirker
antonpirker approved these changes Jun 13, 2023
View reviewed changes
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Fine to merge if we use SENSITIVE_DATA_SUBSTITUTE as placeholder. But it is up to you, if you think thats a stupid idea also fine to merge right away.

@sentrivana sentrivana merged commit d991be7 into master Jun 13, 2023
242 of 243 checks passed
@sentrivana sentrivana deleted the ivana/wrap-parse-url branch June 13, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonpirker antonpirker antonpirker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sentrivana @antonpirker