feat(attributes): Add calculated performance scores#355
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Attributes
Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
| @@ -0,0 +1,18 @@ | |||
| { | |||
| "key": "score.ratio.<key>", | |||
| "brief": "For a measured web vital value, this is the ratio of values that fall below that value.", | |||
There was a problem hiding this comment.
Score.ratio.<key> is the actually the performance score for the individual web vital, normalized from 0-1. While Score.<key> is the preweighted scores, i belive the preweighted scores are legecy. (ratio is more valuable because we can adjust weights at query time)
For more context, When we calculate the total performance score we do a weighted average, something like (score.ratio.lcp * lcpWeight + score.ratio.inp + inpWeight ...) / sumOfWeights.
Here is the exact place we do the division, there a little more too it with things like omitting unseen vitals, but the formula above is the general idea
https://github.com/getsentry/sentry/blob/d4da43fea1f6ce8f521f3b6a1b8dac4c3e5c9a04/src/sentry/search/eap/spans/formulas.py#L486-L492
There was a problem hiding this comment.
To add to this, the legacy way (before we had score.ratio.<key>) was to use the score.weights to calculate the normalized score (i.e lcp score = score.lcp / score.weight.lcp.
There was a problem hiding this comment.
Oh interesting, I thought this only happened in Relay. I based my explanations of these attributes on Relay's calculation logic. Does this mean that everything except score.ratio is strictly speaking optional for Relay to compute?
There was a problem hiding this comment.
Does this mean that everything except score.ratio is strictly speaking optional for Relay to compute?
Eventually yes, all though a quick search in the sentry backend proves that we are still doing things the legacy way in some areas (i.e still using score.weight.* and score.*. I would be hesitant to make non ratios optional without digging into the usages further
There was a problem hiding this comment.
Oh, I definitely wasn't suggesting removing that right now.
What do you think of the rewritten description for score.ratio?
Description
This adds attributes for the results of performance score calculation in Relay, cf. https://github.com/getsentry/relay/blob/4b2a15909edaec2b95bd623d15923abbadb18b19/relay-event-normalization/src/event.rs#L869. The added attributes are exactly the measurements that Relay inserts in that function.
The reason to do this is that for V2 spans we use attributes to store the measurements, and we want every attribute that Relay writes to be defined here.
I hope my descriptions of these attributes make some sense. Please feel free to suggest improvements.
Closes INGEST-896.
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)pii(i.e.maybeortrue. Usefalseonly for values that should never be scrubbed such as IDs)