Skip to content

fix(attributes): Apply consistent scrubbing across deprecations - #692

Merged
mjq merged 2 commits into
mainfrom
mjq/deprecation-inconsistency
Oct 14, 2026
Merged

mjq merged 2 commits into
mainfrom
mjq/deprecation-inconsistency

Conversation

@mjq

@mjq mjq commented Oct 9, 2026

Copy link
Copy Markdown
Member

Description

Apply the same scrubbing rules across entire deprecation chains. This keeps the scrubbing behaviour predictable whether or not Relay is doing attribute rewriting. It also make semantic sense - if two attributes have identical semantic meanings, they should also have identical scrubbing rules.

PR Checklist

  • I have run yarn test and verified that the tests pass.
  • I have run yarn generate to generate and format code and docs.

@vercel

vercel Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sentry-conventions Ready Ready Preview Oct 14, 2026 9:00am UTC

Request Review

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • (attributes) Add http.request.duration by loewenheim in #697

Bug Fixes 🐛

  • (attributes) Apply consistent scrubbing across deprecations by mjq in #692

Internal Changes 🔧

  • (repo) Update CODEOWNERS to include @giortzisg by Lms24 in #708

🤖 This preview updates automatically when you update the PR.

@linear-code

linear-code Bot commented Oct 9, 2026

Copy link
Copy Markdown

EXP-1275

Comment on lines +6 to 7
"key": "manual",
"reason": "Tool results can contain user data"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How come we align on the weaker setting for attributes that can contain user data? Or is the reason text incorrect?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@elramen I'm trying to avoid user-visible changes. We prioritize the latest version of attributes in the product, so effectively, that's the scrubbing rule we're applying - so I took the latest rule and applied it to the older deprecated aliases.

@Lms24 Lms24 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch, thanks for cleaning this up! The changes from auto to manual look reasonable to me for non-gen_ai attributes. For the gen_ai ones, I'd appreciate a second pair of eyes from agent tracing folks if the policies of the current attributes are intentional, since some of them seem a bit more "risky".

@mjq

mjq commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

@Lms24 Thanks! Looks like @obostjancic okay'd this on Slack, so I'm going to go ahead. We were already prioritizing the latest version of attributes in Sentry which would have had the weaker scrubbing rules already, so this isn't effectively weakening anything IMO.

@mjq
mjq enabled auto-merge (squash) October 13, 2026 15:01
@Lms24

Lms24 commented Oct 14, 2026

Copy link
Copy Markdown
Member

I think this just needs another update since we merged some PRs around the same time. Gonna rebase, then this should auto merge

@Lms24
Lms24 requested a review from giortzisg as a code owner October 14, 2026 08:59
@mjq
mjq merged commit b043a22 into main Oct 14, 2026
18 checks passed
@mjq
mjq deleted the mjq/deprecation-inconsistency branch October 14, 2026 09:02

This branch was successfully deployed

1 active deployment
Preview — 2c1fdc44 Deployed Oct 14, 2026 by vercel[bot]
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.

3 participants