Skip to content

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
  • Loading branch information
edwardgou-sentry committed Nov 13, 2024
1 parent bbda268 commit b3b7a3f
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 38 deletions.
12 changes: 5 additions & 7 deletions src/sentry/snuba/entity_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
from sentry.models.environment import Environment
from sentry.models.organization import Organization
from sentry.models.project import Project
from sentry.search.eap.types import SearchResolverConfig
from sentry.search.events.builder.base import BaseQueryBuilder
from sentry.search.events.builder.discover import DiscoverQueryBuilder
from sentry.search.events.builder.metrics import AlertMetricsQueryBuilder
from sentry.search.events.builder.spans_indexed import SpansEAPQueryBuilder
from sentry.search.events.types import ParamsType, QueryBuilderConfig
from sentry.search.events.types import ParamsType, QueryBuilderConfig, SnubaParams
from sentry.sentry_metrics.use_case_id_registry import UseCaseID
from sentry.sentry_metrics.utils import (
resolve,
Expand All @@ -29,12 +30,11 @@
resolve_tag_values,
)
from sentry.snuba.dataset import Dataset, EntityKey
from sentry.snuba.discover import SnubaParams
from sentry.snuba.metrics.extraction import MetricSpecType
from sentry.snuba.metrics.naming_layer.mri import SessionMRI
from sentry.snuba.models import SnubaQuery, SnubaQueryEventType
from sentry.snuba.referrer import Referrer
from sentry.snuba.spans_rpc import TraceItemTableRequest, get_timeseries_query
from sentry.snuba.spans_rpc import get_timeseries_query
from sentry.utils import metrics

# TODO: If we want to support security events here we'll need a way to
Expand Down Expand Up @@ -161,7 +161,7 @@ def build_rpc_request(
environment: Environment | None,
params: ParamsType | None = None,
skip_field_validation_for_entity_subscription_deletion: bool = False,
) -> TraceItemTableRequest:
) -> TimeSeriesRequest:
raise NotImplementedError


Expand Down Expand Up @@ -317,9 +317,7 @@ def build_rpc_request(
y_axes=[self.aggregate],
groupby=[],
referrer=Referrer.API_ALERTS_ALERT_RULE_CHART.value,
config=QueryBuilderConfig(
skip_time_conditions=True,
),
config=SearchResolverConfig(),
granularity_secs=self.time_window,
)

Expand Down
12 changes: 10 additions & 2 deletions src/sentry/snuba/spans_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,16 @@ def get_timeseries_query(
return TimeSeriesRequest(
meta=meta,
filter=query,
aggregations=[agg.proto_definition for agg in aggregations],
group_by=[groupby.proto_definition for groupby in groupbys],
aggregations=[
agg.proto_definition
for agg in aggregations
if isinstance(agg.proto_definition, AttributeAggregation)
],
group_by=[
groupby.proto_definition
for groupby in groupbys
if isinstance(groupby.proto_definition, AttributeKey)
],
granularity_secs=granularity_secs,
)

Expand Down
7 changes: 1 addition & 6 deletions src/sentry/snuba/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
get_entity_subscription_from_snuba_query,
)
from sentry.snuba.models import QuerySubscription, SnubaQuery
from sentry.snuba.referrer import Referrer
from sentry.snuba.utils import build_query_strings
from sentry.tasks.base import instrumented_task
from sentry.utils import metrics, snuba_rpc
Expand Down Expand Up @@ -295,11 +294,7 @@ def _create_rpc_in_snuba(
)

try:
response = snuba_rpc.rpc(
subscription_request,
CreateSubscriptionResponse,
Referrer.API_ALERTS_ALERT_RULE_CHART.value,
)
response = snuba_rpc.rpc(subscription_request, CreateSubscriptionResponse)
except snuba_rpc.SnubaRPCError:
metrics.incr("snuba.snql.subscription.http.error", tags={"dataset": snuba_query.dataset})
raise
Expand Down
26 changes: 17 additions & 9 deletions src/sentry/utils/snuba_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
import sentry_sdk
import sentry_sdk.scope
from google.protobuf.message import Message as ProtobufMessage
from sentry_protos.snuba.v1.endpoint_create_subscription_pb2 import CreateSubscriptionRequest
from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import (
TraceItemTableRequest,
TraceItemTableResponse,
)
from sentry_protos.snuba.v1.error_pb2 import Error as ErrorProto
from urllib3.response import BaseHTTPResponse

from sentry.snuba.referrer import Referrer
from sentry.utils.snuba import SnubaError, _snuba_pool

RPCResponseType = TypeVar("RPCResponseType", bound=ProtobufMessage)
Expand Down Expand Up @@ -59,7 +59,8 @@ def table_rpc(req: TraceItemTableRequest) -> TraceItemTableResponse:


def rpc(
req: SnubaRPCRequest, resp_type: type[RPCResponseType], referrer: Referrer | None = None
req: SnubaRPCRequest | CreateSubscriptionRequest,
resp_type: type[RPCResponseType],
) -> RPCResponseType:
"""
You want to call a snuba RPC. Here's how you do it:
Expand Down Expand Up @@ -95,29 +96,36 @@ def rpc(
cls = req.__class__
endpoint_name = cls.__name__
class_version = cls.__module__.split(".", 3)[2]
http_resp = _make_rpc_request(endpoint_name, class_version, req, referrer)
http_resp = _make_rpc_request(endpoint_name, class_version, req)
resp = resp_type()
resp.ParseFromString(http_resp.data)
return resp


def _make_rpc_request(
endpoint_name: str, class_version: str, req: SnubaRPCRequest, referrer: Referrer | None = None
endpoint_name: str,
class_version: str,
req: SnubaRPCRequest | CreateSubscriptionRequest,
) -> BaseHTTPResponse:
referrer = referrer if referrer else req.meta.referrer
referrer = req.meta.referrer if hasattr(req, "meta") else None
if SNUBA_INFO:
from google.protobuf.json_format import MessageToJson

log_snuba_info(f"{referrer}.body:\n{MessageToJson(req)}") # type: ignore[arg-type]
with sentry_sdk.start_span(op="snuba_rpc.run", name=req.__class__.__name__) as span:
span.set_tag("snuba.referrer", referrer)
if referrer:
span.set_tag("snuba.referrer", referrer)
http_resp = _snuba_pool.urlopen(
"POST",
f"/rpc/{endpoint_name}/{class_version}",
body=req.SerializeToString(),
headers={
"referer": referrer,
},
headers=(
{
"referer": referrer,
}
if referrer
else {}
),
)
if http_resp.status != 200 and http_resp.status != 202:
error = ErrorProto()
Expand Down
17 changes: 3 additions & 14 deletions tests/sentry/snuba/test_entity_subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from unittest.mock import patch

import pytest
from google.protobuf.json_format import MessageToDict
from snuba_sdk import And, Column, Condition, Entity, Function, Join, Op, Relationship

from sentry.exceptions import (
Expand Down Expand Up @@ -677,19 +676,9 @@ def test_get_entity_subscription_for_eap_rpc_query(self) -> None:
query, [self.project.id], None
)

time_series_request_dict = MessageToDict(rpc_timeseries_request)

assert time_series_request_dict.get("granularitySecs") == "3600"
assert (
time_series_request_dict.get("filter")
.get("comparisonFilter")
.get("value")
.get("valStr")
== "http.client"
)
assert (
time_series_request_dict.get("aggregations")[0].get("label") == "count(span.duration)"
)
assert rpc_timeseries_request.granularity_secs == 3600
assert rpc_timeseries_request.filter.comparison_filter.value.val_str == "http.client"
assert rpc_timeseries_request.aggregations[0].label == "count(span.duration)"


class GetEntitySubscriptionFromSnubaQueryTest(TestCase):
Expand Down

0 comments on commit b3b7a3f

Please sign in to comment.