fix(metrics): Add group by attribute fallback#116763
Conversation
Fetch the first metric sample and use its trace item details to supplement group by options when the attributes endpoint misses fields. Keep the attributes endpoint as the primary source on duplicate keys and keep the selector disabled until the fallback flow finishes. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Add toolbar coverage for merging fallback trace item attributes, pretty labels, filtered attributes, loading state, and the equation path. Co-Authored-By: OpenAI Codex <noreply@openai.com>
github-actions
Bot
added
the
Scope: Frontend
|
@sentry review |
|
@cursor review |
JoshuaKGoldberg
left a comment
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
(talked 1:1, Nick will look at more things)
| }).filter(([key]) => !HiddenTraceMetricGroupByFields.includes(key)) | ||
| ); | ||
| }, [data?.booleanAttributes]); | ||
| }, [data?.booleanAttributes, fallbackAttributes.booleanAttributes]); |
There was a problem hiding this comment.
[Refactor] I would dedupe these three nearly-identical useMemo calls. It's just past WET and there's a fair amount of syntax.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5eb2196. Configure here.
| enabled && | ||
| (sampleResult.result.isFetching || | ||
| !sampleResult.result.isFetched || | ||
| (canFetchTraceDetails && traceDetailsResult.isLoading)), |
There was a problem hiding this comment.
Group by disables on refetch
Medium Severity
isFallbackLoading treats any in-flight metric-samples request as loading, including background refetches after data is already present. The group-by control stays disabled (and shows loading) whenever isFetching is true, so users can be blocked from changing group-bys after options were already merged.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5eb2196. Configure here.
JoshuaKGoldberg
left a comment
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
Confirmed this fixes things. Nice!
Just one suggestion, definitely not a blocker at all.
2 participants
Adding in a fallback to the metrics group by selector to pull in data from the first trace item detail.