diff --git a/src/sentry/api/endpoints/project_trace_item_details.py b/src/sentry/api/endpoints/project_trace_item_details.py index dc62ea7520b7..6efbba3419db 100644 --- a/src/sentry/api/endpoints/project_trace_item_details.py +++ b/src/sentry/api/endpoints/project_trace_item_details.py @@ -1,5 +1,6 @@ import time import uuid +from datetime import timedelta from typing import Any, Literal import sentry_sdk @@ -17,8 +18,10 @@ from sentry.api.base import cell_silo_endpoint from sentry.api.bases.project import ProjectEndpoint from sentry.api.exceptions import BadRequest +from sentry.api.utils import get_date_range_from_params from sentry.auth.staff import is_active_staff from sentry.auth.superuser import is_active_superuser +from sentry.exceptions import InvalidParams from sentry.models.project import Project from sentry.search.eap import constants from sentry.search.eap.types import ( @@ -36,8 +39,10 @@ translate_search_type_for_internal_column, translate_to_sentry_conventions, ) +from sentry.search.utils import InvalidQuery, parse_datetime_string from sentry.snuba.referrer import Referrer -from sentry.utils import json, snuba_rpc +from sentry.utils import json +from sentry.utils.snuba_rpc import trace_item_details_rpc _NUMERIC_COERCIONS: dict[str, type] = {"valFloat": float, "valDouble": float} _VAL_TYPE_TO_COLUMN_TYPE: dict[str, ColumnType] = { @@ -362,9 +367,31 @@ def get(request: Request, project: Project, item_id: str) -> Response: if not serializer.is_valid(): return Response(serializer.errors, status=400) + try: + start, end = get_date_range_from_params(request.GET, optional=True) + except InvalidParams: + return Response("date range parameters invalid", status=400) + if "timestamp" in request.GET: + try: + example_timestamp = parse_datetime_string(request.GET["timestamp"]) + except InvalidQuery: + return Response("timestamp parameter invalid", status=400) + time_buffer = 1.5 + example_start = example_timestamp - timedelta(days=time_buffer) + example_end = example_timestamp + timedelta(days=time_buffer) + if start is not None: + start = max(start, example_start) + else: + start = example_start + if end is not None: + end = min(end, example_end) + else: + end = example_end + serialized = serializer.validated_data trace_id = serialized.get("trace_id") item_type = serialized.get("item_type") + sentry_sdk.set_tag("trace_item_details.item_type", item_type) referrer = serialized.get("referrer", Referrer.API_ORGANIZATION_TRACE_ITEM_DETAILS.value) trace_item_type = None @@ -377,12 +404,14 @@ def get(request: Request, project: Project, item_id: str) -> Response: raise BadRequest(detail=f"Unknown trace item type: {item_type}") start_timestamp_proto = ProtoTimestamp() - start_timestamp_proto.FromSeconds(0) - end_timestamp_proto = ProtoTimestamp() - - # due to clock drift, the end time can be in the future - add a week to be safe - end_timestamp_proto.FromSeconds(int(time.time()) + 60 * 60 * 24 * 7) + if start is not None and end is not None: + start_timestamp_proto.FromDatetime(start) + end_timestamp_proto.FromDatetime(end) + else: + start_timestamp_proto.FromSeconds(0) + # due to clock drift, the end time can be in the future - add a week to be safe + end_timestamp_proto.FromSeconds(int(time.time()) + 60 * 60 * 24 * 7) trace_id = request.GET.get("trace_id") if not trace_id: @@ -403,7 +432,7 @@ def get(request: Request, project: Project, item_id: str) -> Response: trace_id=trace_id, ) - resp = MessageToDict(snuba_rpc.trace_item_details_rpc(req)) + resp = MessageToDict(trace_item_details_rpc(req)) use_sentry_conventions = features.has( "organizations:performance-sentry-conventions-fields", diff --git a/tests/snuba/api/endpoints/test_project_trace_item_details.py b/tests/snuba/api/endpoints/test_project_trace_item_details.py index 7694cabfbad3..dc29c5c6f079 100644 --- a/tests/snuba/api/endpoints/test_project_trace_item_details.py +++ b/tests/snuba/api/endpoints/test_project_trace_item_details.py @@ -1,4 +1,5 @@ import uuid +from datetime import timedelta from unittest import mock import pytest @@ -32,7 +33,7 @@ def setUp(self) -> None: self.one_min_ago = before_now(minutes=1) self.trace_uuid = str(uuid.uuid4()).replace("-", "") - def do_request(self, event_type: str, item_id: str, features=None): + def do_request(self, event_type: str, item_id: str, extra_data=None, features=None): item_details_url = reverse( "sentry-api-0-project-trace-item-details", kwargs={ @@ -43,13 +44,16 @@ def do_request(self, event_type: str, item_id: str, features=None): ) if features is None: features = self.features + data = { + "item_type": event_type, + "trace_id": self.trace_uuid, + } + if extra_data is not None: + data.update(extra_data) with self.feature(features): return self.client.get( item_details_url, - { - "item_type": event_type, - "trace_id": self.trace_uuid, - }, + data, ) def test_simple(self) -> None: @@ -639,3 +643,124 @@ def test_attachment(self) -> None: "meta": {}, "timestamp": mock.ANY, } + + def test_with_timestamp(self) -> None: + log = self.create_ourlog( + { + "body": "foo", + "trace_id": self.trace_uuid, + }, + attributes={ + "str_attr": { + "string_value": "1", + }, + "int_attr": {"int_value": 2}, + "float_attr": { + "double_value": 3.0, + }, + "bool_attr": { + "bool_value": True, + }, + }, + timestamp=self.one_min_ago, + ) + self.store_eap_items([log]) + item_id = log.item_id.hex() + + for extra_data in [ + {"timestamp": self.one_min_ago.isoformat()}, + {"statsPeriod": "24h"}, + ]: + trace_details_response = self.do_request("logs", item_id, extra_data=extra_data) + + assert trace_details_response.status_code == 200, trace_details_response.content + + timestamp_nanos = int(self.one_min_ago.timestamp() * 1_000_000_000) + assert trace_details_response.data["attributes"] == [ + {"name": "tags[bool_attr,boolean]", "type": "bool", "value": True}, + {"name": "tags[float_attr,number]", "type": "float", "value": 3.0}, + { + "name": "observed_timestamp", + "type": "int", + "value": str(timestamp_nanos), + }, + {"name": "project_id", "type": "int", "value": str(self.project.id)}, + {"name": "severity_number", "type": "int", "value": "0"}, + {"name": "tags[int_attr,number]", "type": "int", "value": "2"}, + { + "name": "timestamp_precise", + "type": "int", + "value": str(timestamp_nanos), + }, + {"name": "message", "type": "str", "value": "foo"}, + {"name": "severity", "type": "str", "value": "INFO"}, + {"name": "str_attr", "type": "str", "value": "1"}, + {"name": "trace", "type": "str", "value": self.trace_uuid}, + ] + assert trace_details_response.data["itemId"] == item_id + assert ( + trace_details_response.data["timestamp"] + == self.one_min_ago.replace(microsecond=0, tzinfo=None).isoformat() + "Z" + ) + + def test_with_incorrect_timestamp(self) -> None: + log = self.create_ourlog( + { + "body": "foo", + "trace_id": self.trace_uuid, + }, + attributes={ + "str_attr": { + "string_value": "1", + }, + "int_attr": {"int_value": 2}, + "float_attr": { + "double_value": 3.0, + }, + "bool_attr": { + "bool_value": True, + }, + }, + timestamp=self.one_min_ago, + ) + self.store_eap_items([log]) + item_id = log.item_id.hex() + + for extra_data in [ + {"timestamp": (self.one_min_ago - timedelta(days=30)).isoformat()}, + {"statsPeriodEnd": "24h", "statsPeriodStart": "48h"}, + ]: + trace_details_response = self.do_request("logs", item_id, extra_data=extra_data) + + assert trace_details_response.status_code == 404, trace_details_response.content + + def test_with_invalid_timestamp(self) -> None: + log = self.create_ourlog( + { + "body": "foo", + "trace_id": self.trace_uuid, + }, + attributes={ + "str_attr": { + "string_value": "1", + }, + "int_attr": {"int_value": 2}, + "float_attr": { + "double_value": 3.0, + }, + "bool_attr": { + "bool_value": True, + }, + }, + timestamp=self.one_min_ago, + ) + self.store_eap_items([log]) + item_id = log.item_id.hex() + + for extra_data in [ + {"timestamp": "beepboop"}, + {"statsPeriod": "hello"}, + ]: + trace_details_response = self.do_request("logs", item_id, extra_data=extra_data) + + assert trace_details_response.status_code == 400, trace_details_response.content