feat(attributes): Deprecate http.query and http.fragment in favour of url.query and url.fragment - #539
feat(attributes): Deprecate http.query and http.fragment in favour of url.query and url.fragment#539Lms24 wants to merge 2 commits into
http.query and http.fragment in favour of url.query and url.fragment#539Conversation
…r of `url.query` and `url.fragment`
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Attributes
Other
Internal Changes 🔧Attribute
Attributes
Deps
Other
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }, | ||
| "deprecation": { | ||
| "_status": "backfill", | ||
| "replacement": "url.fragment" |
There was a problem hiding this comment.
Backfill ignores value format mismatch
High Severity
http.fragment and http.query use deprecation _status of backfill, which copies values as-is to url.fragment and url.query. Those pairs are not value-compatible: the http.* attributes include a leading # or ?, while the url.* attributes do not. Per CONTRIBUTING, incompatible values need _status: "transform" and an attribute transformation, or ingestion will write incorrect url.* values.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ef645c3. Configure here.
There was a problem hiding this comment.
yes, that's correct. Still need to decide what to do
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b25bbcc. Configure here.
| "_status": "backfill", | ||
| "replacement": "url.query", | ||
| "reason": "`url.query` is supported by OTel and conceptually more fitting." | ||
| }, |
There was a problem hiding this comment.
Backfill ignores value format mismatch
High Severity
deprecation._status is set to backfill for http.query and http.fragment, but those values include a leading ?/# while url.query and url.fragment must not. Backfill copies values as-is, so ingest would write OTel-invalid replacements. Per CONTRIBUTING, incompatible values need transform or _status: null.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b25bbcc. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| "deprecation": { | ||
| "_status": "backfill", | ||
| "replacement": "url.query", | ||
| "reason": "`url.query` is supported by OTel and conceptually more fitting." | ||
| }, |
There was a problem hiding this comment.
Bug: The deprecation status for http.query and http.fragment should be "transform" instead of "backfill" to handle semantic differences with their url.* replacements.
Severity: MEDIUM
Suggested Fix
Change the _status from "backfill" to "transform" for http.query and http.fragment. Create a corresponding transformation document to strip the leading ? and # characters from the values before assigning them to url.query and url.fragment.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: model/attributes/http/http__query.json#L9-L13
Potential issue: The deprecation status for `http.query` and `http.fragment` is
incorrectly set to `"backfill"`. This status implies a direct copy of the value to the
new attributes, `url.query` and `url.fragment`. However, the old attributes contain a
leading `?` or `#` character, while the new attributes should not. This direct copy will
result in semantically incorrect data for the new attributes. The contribution
guidelines state that a `"transform"` status should be used when a value cannot be
copied directly, which is the case here.
Also affects:
model/attributes/http/http__fragment.json:8~12
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Description
These attributes are ambiguous and have slightly different semantics:
http.queryandhttp.fragmentrequire a leading?/#and are not defined by OTelurl.queryandurl.fragmentrequire no leading delimiters and are defined by OTel.This PR deprecates the
http.*variants.Open to decide: Should we backfill or transform the attributes? The change in semantics makes this a bit more cumbersome. A simpler option would be to set status
null.PR Checklist
yarn testand verified that the tests pass.yarn generateto generate and format code and docs.If an attribute was added:
nextjs.function_id, notfunction_id)apply_scrubbing(i.e.manualorauto. Useneveronly for values that should never be scrubbed such as IDs)If an attribute was deprecated: