Feat(spotlight): metrics support #1265
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Deps
Other
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 1476 uncovered lines. Files with missing lines (32)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 76.31% 75.18% -1.13%
==========================================
Files 47 48 +1
Lines 5690 5946 +256
Branches 611 614 +3
==========================================
+ 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}
packages/spotlight/src/ui/telemetry/components/metrics/MetricDetail.tsx
Outdated
Show resolved
Hide resolved
packages/spotlight/src/ui/telemetry/components/metrics/MetricsList.tsx
Outdated
Show resolved
Hide resolved
|
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)
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.
|
|
||
| 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.
#1145
reference: https://develop.sentry.dev/sdk/telemetry/metrics/
Screen.Recording.2026-01-25.at.10.40.19.PM.mov