-
-
Notifications
You must be signed in to change notification settings - Fork 32
Feat(spotlight): metrics support #1265
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
base: main
Are you sure you want to change the base?
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 🔧
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 |
| > | ||
| {type} | ||
| </span> | ||
| ); |
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.
Duplicate MetricTypeBadge component in two files
Low Severity
The MetricTypeBadge component is identically defined in both MetricDetail.tsx and MetricsList.tsx. Both implementations have the same props, the same color mappings for counter/gauge/distribution, and the same rendering logic. This duplication increases maintenance burden and risks inconsistent bug fixes if either copy needs to be modified.
Additional Locations (1)
| min: Math.min(...values), | ||
| max: Math.max(...values), | ||
| count: metrics.length, | ||
| }; |
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.
| getMetricsByName: (name: string) => { | ||
| return get().metricsByName.get(name) || []; | ||
| }, | ||
| getMetricNames: () => Array.from(get().metricsByName.keys()), |
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.
Unused getMetricsByName and getMetricNames functions
Low Severity
The getMetricsByName and getMetricNames functions are defined in the metrics slice and typed in the store types, but they are never called anywhere in the codebase. These are dead code that add unnecessary complexity and maintenance burden without providing value.
Additional Locations (1)
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| {type} | ||
| </span> | ||
| ); | ||
| } |
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.
Duplicated MetricTypeBadge component across two files
Low Severity
The MetricTypeBadge component is defined identically in both MetricsList.tsx and MetricDetail.tsx. Both implementations have the same props, color mappings, and JSX structure. This duplicated logic increases maintenance burden and risks inconsistent bug fixes if changes are needed in the future.
Additional Locations (1)
| const payload = itemData as { items?: SentryMetricPayload[] }; | ||
| if (payload.items && Array.isArray(payload.items)) { | ||
| allMetrics.push(...payload.items); | ||
| } |
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.
Server-side metrics extraction omits required id field
Medium Severity
The extractMetricsFromEnvelopes function casts raw envelope items to SentryMetricPayload[], but the raw data from envelopes lacks the required id field. The type contract specifies id: string is mandatory, yet raw metric items don't include this field. The UI-side processMetricItems correctly adds IDs, but the server-side MCP tool doesn't, creating an inconsistency.
|
This looks awesome 🤩 |
#1145
reference: https://develop.sentry.dev/sdk/telemetry/metrics/
Screen.Recording.2026-01-25.at.10.40.19.PM.mov