-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix(dashboards): use spans dataset in default overview dashboard backend #108682
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,12 @@ def test_get_prebuilt_dashboard(self) -> None: | |
| assert response.status_code == 200 | ||
| assert response.data["id"] == "default-overview" | ||
|
|
||
| def test_get_prebuilt_dashboard_with_transactions_deprecation_feature_flag(self) -> None: | ||
| with self.feature("organizations:discover-saved-queries-deprecation"): | ||
| response = self.do_request("get", self.url("default-overview")) | ||
| assert response.status_code == 200 | ||
| assert response.data["widgets"][7]["widgetType"] == "spans" | ||
|
|
||
| def test_prebuilt_dashboard_with_discover_split_feature_flag(self) -> None: | ||
| response = self.do_request("get", self.url("default-overview")) | ||
| assert response.status_code == 200, response.data | ||
|
|
@@ -2181,6 +2187,31 @@ def test_update_prebuilt_dashboard(self) -> None: | |
| assert len(queries) == 1 | ||
| assert DashboardTombstone.objects.filter(slug=slug).exists() | ||
|
|
||
| def test_update_prebuilt_dashboard_with_transactions_deprecation_feature_flag(self) -> None: | ||
| data = { | ||
| "title": "First dashboard", | ||
| "widgets": [ | ||
| { | ||
| "title": "New title", | ||
| "displayType": "line", | ||
| "widgetType": "transaction-like", | ||
| "queries": [ | ||
| { | ||
| "name": "transactions", | ||
| "fields": ["count()"], | ||
| "columns": [], | ||
| "aggregates": ["count()"], | ||
| "conditions": "event.type:transaction", | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| } | ||
| slug = "default-overview" | ||
| with self.feature("organizations:discover-saved-queries-deprecation"): | ||
| response = self.do_request("put", self.url(slug), data=data) | ||
| assert response.status_code == 400, response.data | ||
|
Comment on lines
+2212
to
+2213
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, can we confirm if we actually do It might be a bug in the frontend then if we don't try to post. Which now that I think of feels like it could be the case.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that's the case, I don't think this test is necessary because we shouldn't ever be trying to update this hardcoded dashboard
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so basically the way it works is when you go to update the |
||
|
|
||
| def test_update_unknown_prebuilt(self) -> None: | ||
| data = { | ||
| "title": "First dashboard", | ||
|
|
||
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.
Do we also need to update the query condition for the transaction widget to include
is_segment:Trueor something? because otherwise it's aggregating spans and the counts would be for spans and not necessarily transactionsThere 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.
oh true good point! i'll update that