-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(starfish): Replaces usage of p95 in the Database Module with avg #53515
feat(starfish): Replaces usage of p95 in the Database Module with avg #53515
Conversation
Some of these changes also affect parts of the WSV workflow due to shared components. We can complete the move from p95 in the WSV to avg in a separate future pr since WSV is still undergoing redesign. |
…tarfish/replace-p95-with-avg
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.
Awesome!
@@ -191,7 +191,9 @@ export function TransactionSamplesTable({queryConditions, sampleFilter}: Props) | |||
return ( | |||
<DurationComparisonCell | |||
duration={row['transaction.duration']} | |||
p95={(aggregatesData?.['p95(transaction.duration)'] as number) ?? 0} | |||
compareToDuration={ | |||
(aggregatesData?.['p95(transaction.duration)'] as number) ?? 0 |
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.
Should this also be avg()
or is this out of scope?
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 think I'm going to consider this to be out of scope for this pr. This component is part of the WSV/Endpoint overview so I'll follow up with updating this in a separate pr when it becomes a necessity.
Replaces usage of p95 in the Database Module with avg in the frontend. Also cleans ups some types and column related constants.