-
Notifications
You must be signed in to change notification settings - Fork 94
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
ref(spans): Scrub data image descriptions #2560
Conversation
In span descriptions fetching encoded data images, the actual encoded image provides no value to the span. However, the number of `/`s is forcing the span description clusterer to misbehave.
"GET data:image/png;base64,drtfghaksjfdhaeh/blah/blah/blah", | ||
"http.client", | ||
"GET data:image/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can relax the scrubbing to include png
or base64
, but I don't think it provides any value and complicates a bit the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
@@ -2,6 +2,10 @@ | |||
|
|||
## Unreleased | |||
|
|||
**Features**: | |||
|
|||
- Scrub span descriptions with encoded data images. ([#2560](https://github.com/getsentry/relay/pull/2560)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would omit this changelog records to be honest. And it's not really a feature in my opinion, maybe something like internal fits better here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I was planning to omit it. I saw the changelog section in the README and some scrubbing related entries in both features and internal. Since this is user-visible I'll leave it under features, unless someone strongly disagrees.
@@ -1,5 +1,9 @@ | |||
# Changelog | |||
|
|||
## Unreleased | |||
|
|||
- Scrub span descriptions with encoded data images. ([#2560](https://github.com/getsentry/relay/pull/2560)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
In span descriptions fetching encoded data images, the actual encoded image provides no value to the span. However, the number of
/
s is forcing the span description clusterer to misbehave.Resolves https://github.com/getsentry/team-ingest/issues/221.