feat(attributes): add search alias metadata - #492
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Breaking Changes 🛠
New Features ✨Attributes
Op
Other
Internal Changes 🔧Attribute
Attributes
Deps
Other
🤖 This preview updates automatically when you update the PR. |
| { | ||
| "public_alias": "span.op", | ||
| "internal_name": "sentry.op", | ||
| "search_type": "string", |
There was a problem hiding this comment.
Can likely make string the default, so not required.
| "internal_name": { | ||
| "description": "The internal attribute name used by the EAP search resolver", | ||
| "type": "string" | ||
| }, |
There was a problem hiding this comment.
Will this always duplicate the attribute's key? It feels like it ought to, in which case I'd prefer we make mismatches impossible by removing this property.
| "private": { | ||
| "description": "Whether the internal attribute should be hidden from public search results", | ||
| "type": "boolean" | ||
| }, |
There was a problem hiding this comment.
Is this already covered by visibility? cc @nsdeschenes
| "spans": { | ||
| "description": "The column expression in the indexed spans dataset", | ||
| "type": "string" | ||
| }, |
There was a problem hiding this comment.
This dataset has been removed. Only EAP exists now.
| "eap": { | ||
| "description": "The column expression in the Events Analytics Platform dataset", | ||
| "type": "string" | ||
| } |
There was a problem hiding this comment.
This seems better left to the Sentry backend to compute. (What's sent to EAP isn't a string either, so I don't think this is helpful here anyway). We do need a way to record when the search types and internal types of an attribute mismatch, though. I'd rather expose an (optional) internal_type property for this (one of string, boolean, double, integer, or array) so that the backend can construct the correct RPC proto.
| "description": "Whether the internal attribute should be hidden from public search results", | ||
| "type": "boolean" | ||
| }, | ||
| "dataset_mappings": { |
There was a problem hiding this comment.
Given the two other two comments re: spans and eap I think the whole dataset_mappings can be removed.
|
this closes #85 right? |
Simplify the search alias model from a complex array of objects (with internal_name, search_type, dataset_mappings, etc.) to a single top-level public_alias string field on attributes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # python/src/sentry_conventions/attributes.py # scripts/generate_attributes.ts
Add public_alias metadata from the Sentry EAP search resolver to all convention attributes that have a corresponding search alias, including span.* aliases, user.* aliases, mobile frame aliases, and more. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the flat `public_alias` string with a structured `search_alias` object that supports `name`, optional `type` (defaults to string), and optional `aliases` array for additional search query names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Refactor `public_alias` into a structured `search_alias` object with `name`, optional `type`, and optional `aliases` fields. Add non-string search types from the EAP search resolver (number, integer, second) and add search_alias with aliases for db.system. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add search_alias with non-string types from the EAP search resolver: - ai.total_cost (currency), ai.total_tokens.used (integer) - cache.hit (boolean), cache.item_size (byte) - code.lineno (number) - http.decoded_response_content_length (byte), http.response_content_length (byte), http.response_transfer_size (byte) - messaging.message.body.size (byte), messaging.message.receive.latency (millisecond), messaging.message.retry.count (number) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| "is_in_otel": false, | ||
| "example": true, | ||
| "visibility": "public", | ||
| "search_alias": { |
There was a problem hiding this comment.
q: Does it make sense to add a search_alias entry if neither type nor name differ from the primary definition? like in this case but probably in a lot of others, too
| "visibility": "public", | ||
| "search_alias": { | ||
| "name": "db.system", | ||
| "aliases": ["span.system"] |
There was a problem hiding this comment.
side-note/no action required: I guess we can't break this, since we have such a ResolvedAttribute but IMHO the aliases mapping makes very little sense 🤔
| { | ||
| "key": "sentry.thread.id", | ||
| "brief": "Current “managed” thread ID.", | ||
| "brief": "Current \u201cmanaged\u201d thread ID.", |
There was a problem hiding this comment.
super-l:
| "brief": "Current \u201cmanaged\u201d thread ID.", | |
| "brief": "Current \"managed\" thread ID.", |
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } |
There was a problem hiding this comment.
m: what's the difference between search_alias.name and search_alias.aliases? Is the intention purely to have more than one alias or are there more differences?
If there are no differences, should we simplify to names which is always an Array? (I likely lack some context in the product but I checked ResolvedAttribute and I don't see more than one alias per instance. I do see the secondary_alias boolean though which might play a role here 🤔 )
If there is a difference, let's point this out in the schema description. Also let's consider changing the name because we already have a top-level alias field and this might get confusing, especially, because the top-level alias get's generated to aliases in JS and Python code. So if these are "secondary" aliases, let's maybe call it secondary_aliases?
|
I will rebase #508 on this PR and probably repurpose it slightly: Instead of exposing purely a deprecation chain, the chains will include:
|
4 participants
No description provided.