-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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(apis): Pass query source to snuba - phase 2 #73497
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 |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
from sentry.api.base import region_silo_endpoint | ||
from sentry.api.bases import OrganizationEventsV2EndpointBase | ||
from sentry.constants import MAX_TOP_EVENTS | ||
from sentry.middleware import is_frontend_request | ||
from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetTypes | ||
from sentry.models.organization import Organization | ||
from sentry.snuba import ( | ||
|
@@ -25,6 +26,7 @@ | |
transactions, | ||
) | ||
from sentry.snuba.metrics.extraction import MetricSpecType | ||
from sentry.snuba.query_sources import QuerySource | ||
from sentry.snuba.referrer import Referrer | ||
from sentry.utils.snuba import SnubaError, SnubaTSResult | ||
|
||
|
@@ -75,6 +77,9 @@ | |
ALLOWED_EVENTS_STATS_REFERRERS: set[str] = { | ||
Referrer.API_ALERTS_ALERT_RULE_CHART.value, | ||
Referrer.API_ALERTS_CHARTCUTERIE.value, | ||
Referrer.API_ENDPOINT_REGRESSION_ALERT_CHARTCUTERIE.value, | ||
Referrer.API_FUNCTION_REGRESSION_ALERT_CHARTCUTERIE.value, | ||
Referrer.DISCOVER_SLACK_UNFURL.value, | ||
Referrer.API_DASHBOARDS_WIDGET_AREA_CHART.value, | ||
Referrer.API_DASHBOARDS_WIDGET_BAR_CHART.value, | ||
Referrer.API_DASHBOARDS_WIDGET_LINE_CHART.value, | ||
|
@@ -108,6 +113,14 @@ | |
} | ||
|
||
|
||
SENTRY_BACKEND_REFERRERS = [ | ||
Referrer.API_ALERTS_CHARTCUTERIE.value, | ||
Referrer.API_ENDPOINT_REGRESSION_ALERT_CHARTCUTERIE.value, | ||
Referrer.API_FUNCTION_REGRESSION_ALERT_CHARTCUTERIE.value, | ||
Referrer.DISCOVER_SLACK_UNFURL.value, | ||
] | ||
|
||
|
||
@region_silo_endpoint | ||
class OrganizationEventsStatsEndpoint(OrganizationEventsV2EndpointBase): | ||
publish_status = { | ||
|
@@ -167,6 +180,7 @@ def check_if_results_have_data(self, results: SnubaTSResult | dict[str, SnubaTSR | |
return has_data | ||
|
||
def get(self, request: Request, organization: Organization) -> Response: | ||
query_source = QuerySource.FRONTEND if is_frontend_request(request) else QuerySource.API | ||
with sentry_sdk.start_span(op="discover.endpoint", description="filter_params") as span: | ||
span.set_data("organization", organization) | ||
|
||
|
@@ -205,6 +219,8 @@ def get(self, request: Request, organization: Organization) -> Response: | |
if referrer in ALLOWED_EVENTS_STATS_REFERRERS.union(METRICS_ENHANCED_REFERRERS) | ||
else Referrer.API_ORGANIZATION_EVENT_STATS.value | ||
) | ||
if referrer in SENTRY_BACKEND_REFERRERS: | ||
query_source = QuerySource.SENTRY_BACKEND | ||
Comment on lines
+222
to
+223
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. Can this logic be moved into 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.
I don't think it's a good idea to move this to is_frontend_request because that's really independent of Snuba. Let me create a method in |
||
batch_features = self.get_features(organization, request) | ||
has_chart_interpolation = batch_features.get( | ||
"organizations:performance-chart-interpolation", False | ||
|
@@ -281,6 +297,7 @@ def _get_event_stats( | |
on_demand_metrics_enabled=use_on_demand_metrics, | ||
on_demand_metrics_type=on_demand_metrics_type, | ||
include_other=include_other, | ||
query_source=query_source, | ||
) | ||
|
||
return scoped_dataset.timeseries_query( | ||
|
@@ -306,6 +323,7 @@ def _get_event_stats( | |
) | ||
), | ||
on_demand_metrics_type=on_demand_metrics_type, | ||
query_source=query_source, | ||
) | ||
|
||
def get_event_stats_factory(scoped_dataset): | ||
|
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.
Did you mean to leave this commented out?
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.
Yes, not all datasets currently expect query_source as a param and they're not typed to safely take care of that. So I'll uncomment it after my last PR.