Feat(spotlight): metrics support#1265
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Internal Changes 🔧Deps
Deps Dev
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊❌ Patch coverage is 50.58%. Project has 1476 uncovered lines. Files with missing lines (3)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 76.31% 75.18% -1.13%
==========================================
Files 47 48 +1
Lines 5690 5946 +256
Branches 611 611 —
==========================================
+ Hits 4342 4470 +128
- Misses 1348 1476 +128
- Partials 5 5 —Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
This looks awesome 🤩 |
| <span className="text-primary-400">P95:</span> | ||
| <span className="text-primary-200 ml-1">{getFormattedNumber(percentiles.get(95) ?? 0)}</span> | ||
| </div> | ||
| </> |
There was a problem hiding this comment.
P99 percentile calculated but not displayed in UI
High Severity
The calculatePercentiles call at line 69 explicitly calculates P99 alongside P50, P90, and P95. However, the AggregateSummary component only renders P50, P90, and P95 - omitting P99 entirely. The PR description explicitly lists "percentiles (P50, P90, P95, P99)" as a feature, making this a missing functionality bug.
Additional Locations (1)
| for (const [key, attr] of Object.entries(metric.attributes)) { | ||
| data[`attr.${key}`] = attr.value; | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent null/undefined handling across metric formatters
Low Severity
The human formatter defensively checks attr.value !== undefined && attr.value !== null before including attribute values, but the logfmt and md formatters access attr.value directly without any null/undefined guards. If an attribute has a missing or null value, logfmt will include undefined values in the output, and md will render literal "undefined" or "null" strings in the markdown table.
Additional Locations (1)
|
|
||
| const metricsWithName = newMetricsByName.get(metric.name) || []; | ||
| metricsWithName.push(metric); | ||
| newMetricsByName.set(metric.name, metricsWithName); |
There was a problem hiding this comment.
State mutation via shared array reference in metrics store
Medium Severity
The metricsByName Map is shallow-copied with new Map(metricsByName), but its array values maintain the same references. When newMetricsByName.get(metric.name) returns an existing array, calling push on it mutates the original array that's still referenced by the previous state. This violates immutability principles and can cause stale state issues, incorrect memoization, or React components not re-rendering when they should.
|
this would be great! just started looking into leveraging metrics for our app but not being able to see them in spotlight atm is a bummer |
|
What's missing for this to land? Super eager to have this. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f483f7b. Configure here.
| } | ||
| } | ||
| } | ||
| }, [metricId, metricGroups]); |
There was a problem hiding this comment.
Infinite render loop when viewing metric details
High Severity
When metricId is set (user clicks a metric sample), this useEffect causes an infinite render loop. allMetrics is computed directly in the render body via getMetrics() which calls Array.from(...), producing a new array reference every render. This makes the useMemo for allGroups and metricGroups recalculate every render (new references). Since metricGroups is in the effect's dependency array, the effect fires every render. It then calls setExpandedSections with new Set(...) which always creates a new reference, triggering another re-render — repeating the cycle indefinitely.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f483f7b. Configure here.
4 participants
#1145
reference: https://develop.sentry.dev/sdk/telemetry/metrics/
Screen.Recording.2026-01-25.at.10.40.19.PM.mov