-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(insights): register query module prebuilt dashboards #103531
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(insights): register query module prebuilt dashboards #103531
Conversation
| { | ||
| "prebuilt_id": PrebuiltDashboardId.BACKEND_QUERIES, | ||
| "title": "Queries Overview", | ||
| }, | ||
| { | ||
| "prebuilt_id": PrebuiltDashboardId.BACKEND_QUERIES_SUMMARY, | ||
| "title": "Query Summary", | ||
| }, |
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.
Bug: Prebuilt dashboard widgets contain id attributes, violating a documented constraint.
Severity: HIGH | Confidence: 1.00
🔍 Detailed Analysis
The frontend configuration files queries.ts and querySummary.ts define widgets with id attributes (e.g., id: 'throughput', id: 'duration'). This violates a documented constraint in src/sentry/models/dashboard.py which requires that "All widgets and queries in prebuilt dashboards must not have id attributes defined". This will prevent users from properly updating forked versions of these prebuilt dashboards.
💡 Suggested Fix
Remove all id attributes from widgets defined in queries.ts and querySummary.ts to comply with the documented constraint.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/dashboards/endpoints/organization_dashboards.py#L88-L95
Potential issue: The frontend configuration files `queries.ts` and `querySummary.ts`
define widgets with `id` attributes (e.g., `id: 'throughput'`, `id: 'duration'`). This
violates a documented constraint in `src/sentry/models/dashboard.py` which requires that
"All widgets and queries in prebuilt dashboards must not have id attributes defined".
This will prevent users from properly updating forked versions of these prebuilt
dashboards.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2778025
| { | ||
| "prebuilt_id": PrebuiltDashboardId.BACKEND_QUERIES_SUMMARY, | ||
| "title": "Query Summary", | ||
| }, |
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.
Bug: Dashboard Title Uniqueness Blocks Prebuilt Deployment
The new prebuilt dashboard titles "Queries Overview" and "Query Summary" can conflict with existing user-created dashboards due to the unique constraint on (organization, title). When sync_prebuilt_dashboards tries to create these prebuilt dashboards, it raises an IntegrityError if a custom dashboard with the same title already exists, preventing the prebuilt dashboard from being created. This creates inconsistent behavior across organizations where some get the prebuilt dashboards and others silently fail.
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.
This is a good point, we should probably not allow people to create dashboards that match a prebuilt title 🤔 , but it's possible one with the same title already exists. For now we can merge this to continue to development. the prebuilt dashboards are deleted then recreated so we can make changes to the title without some migration script.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #103531 +/- ##
===========================================
+ Coverage 80.39% 80.58% +0.18%
===========================================
Files 9255 9255
Lines 395186 395140 -46
Branches 25201 25181 -20
===========================================
+ Hits 317728 318406 +678
+ Misses 77028 76305 -723
+ Partials 430 429 -1 |
…ry-summary-and-overview-prebuilt-dashboards-to
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.
Bug: Prebuilt Dashboards Consume User Quota
The dashboard count includes prebuilt dashboards when checking against the organization's quota limit. Since prebuilt dashboards are system-created (with created_by_id=None) and auto-registered for organizations, they reduce the number of custom dashboards users can create. The count should likely exclude prebuilt dashboards by filtering for prebuilt_id__isnull=True.
src/sentry/dashboards/endpoints/organization_dashboards.py#L445-L447
sentry/src/sentry/dashboards/endpoints/organization_dashboards.py
Lines 445 to 447 in 30c3996
| "organization": organization, | |
| "request": request, | |
| "projects": self.get_projects(request, organization), |
…ry-summary-and-overview-prebuilt-dashboards-to
This allows prebuilt dashboards to be auto created for orgs