Skip to content

Commit

Permalink
feat(visibility): Clamp date range for TagStore queries (#81363)
Browse files Browse the repository at this point in the history
Follow-up and complement to
#80332. In short, `SAMPLE`
wasn't enough. For longer ranges (i.e., >30d) fetching a list of project
tags still times out.

In this PR, we're adding time range clamping for fetching tag keys. We
will only ever query a maximum of N days (14 for now, but it's
configurable). If someone chooses to get tags for the last 90 days, they
will get tags from the last 14 days. If they need tags from November 5th
- November 10th, we will fetch the range they asked for.

We think this is a reasonable compromise. When tags don't load at all,
autocomplete stops working, which is very bad UX. Limiting the date
range to 14 days is a tradeoff. The tags will succeed more often, but
some tags might be missing.

Also, limiting the range should improve the cache hit ratio a bit, but
we'll see.

---------

Co-authored-by: Tony Xiao <[email protected]>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 27, 2024
1 parent 1e85866 commit 198e038
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 7 deletions.
22 changes: 18 additions & 4 deletions src/sentry/api/endpoints/organization_tags.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import datetime

import sentry_sdk
from rest_framework.exceptions import ParseError
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import tagstore
from sentry import features, options, tagstore
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.bases import NoProjects
from sentry.api.bases.organization import OrganizationEndpoint
from sentry.api.serializers import serialize
from sentry.api.utils import handle_query_errors
from sentry.api.utils import clamp_date_range, handle_query_errors
from sentry.snuba.dataset import Dataset
from sentry.utils.numbers import format_grouped_length
from sentry.utils.sdk import set_measurement
Expand Down Expand Up @@ -39,11 +41,23 @@ def get(self, request: Request, organization) -> Response:

with sentry_sdk.start_span(op="tagstore", name="get_tag_keys_for_projects"):
with handle_query_errors():
start = filter_params["start"]
end = filter_params["end"]

if features.has("organizations:tag-key-sample-n", organization) and start and end:
# Tag queries longer than 14 days tend to time out for large customers. For getting a list of tags, clamping to 14 days is a reasonable compromise of speed vs. completeness
(start, end) = clamp_date_range(
(start, end),
datetime.timedelta(
days=options.get("visibility.tag-key-max-date-range.days")
),
)

results = tagstore.backend.get_tag_keys_for_projects(
filter_params["project_id"],
filter_params.get("environment"),
filter_params["start"],
filter_params["end"],
start,
end,
use_cache=request.GET.get("use_cache", "0") == "1",
dataset=dataset,
tenant_ids={"organization_id": organization.id},
Expand Down
29 changes: 29 additions & 0 deletions src/sentry/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,35 @@ def get_date_range_from_stats_period(
return start, end


def clamp_date_range(
range: tuple[datetime.datetime, datetime.datetime], max_timedelta: datetime.timedelta
) -> tuple[datetime.datetime, datetime.datetime]:
"""
Accepts a date range and a maximum time delta. If the date range is shorter
than the max delta, returns the range as-is. If the date range is longer than the max delta, clamps the range range, anchoring to the end.
If any of the inputs are invalid (e.g., a negative range) returns the range
without modifying it.
:param range: A tuple of two `datetime.datetime` objects
:param max_timedelta: Maximum allowed range delta
:return: A tuple of two `datetime.datetime` objects
"""

[start, end] = range
delta = end - start

# Ignore negative max time deltas
if max_timedelta < datetime.timedelta(0):
return (start, end)

# Ignore if delta is within acceptable range
if delta < max_timedelta:
return (start, end)

return (end - max_timedelta, end)


# The wide typing allows us to move towards RpcUserOrganizationContext in the future to save RPC calls.
# If you can use the wider more correct type, please do.
def is_member_disabled_from_limit(
Expand Down
7 changes: 7 additions & 0 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -2897,6 +2897,13 @@
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

# option for clamping project tag key date range
register(
"visibility.tag-key-max-date-range.days",
default=14,
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

# option used to enable/disable applying
# stack trace rules in profiles
register(
Expand Down
7 changes: 4 additions & 3 deletions src/sentry/tagstore/snuba/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,13 +451,14 @@ def get_tag_keys_for_projects(
# We want to disable FINAL in the snuba query to reduce load.
optimize_kwargs = {"turbo": True}

# Add static sample amount to the query. Turbo will sample at 10% by
# default, but organizations with many events still get timeouts. A
# static sample creates more consistent performance.
organization_id = get_organization_id_from_project_ids(projects)
organization = Organization.objects.get_from_cache(id=organization_id)
if features.has("organizations:tag-key-sample-n", organization):
# Add static sample amount to the query. Turbo will sample at 10% by
# default, but organizations with many events still get timeouts. A
# static sample creates more consistent performance.
optimize_kwargs["sample"] = options.get("visibility.tag-key-sample-size")

# If we are fetching less than max_unsampled_projects, then disable
# the sampling that turbo enables so that we get more accurate results.
# We only want sampling when we have a large number of projects, so
Expand Down
34 changes: 34 additions & 0 deletions tests/sentry/api/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from sentry.api.utils import (
MAX_STATS_PERIOD,
clamp_date_range,
get_date_range_from_params,
handle_query_errors,
print_and_capture_handler_exception,
Expand Down Expand Up @@ -196,3 +197,36 @@ def test_handle_query_errors(self, mock_parse_error):
raise ex
except Exception as e:
assert isinstance(e, (FooBarError, APIException))


class ClampDateRangeTest(unittest.TestCase):
def test_no_clamp_if_range_under_max(self):
start = datetime.datetime(2024, 1, 1)
end = datetime.datetime(2024, 1, 2)
max_timedelta = datetime.timedelta(days=7)

assert clamp_date_range((start, end), max_timedelta) == (start, end)

def test_no_clamp_for_negative_range(self):
start = datetime.datetime(2024, 1, 1)
end = datetime.datetime(2023, 1, 2)
max_timedelta = datetime.timedelta(hours=1)

assert clamp_date_range((start, end), max_timedelta) == (start, end)

def test_clamps_even_to_zero(self):
start = datetime.datetime(2024, 1, 1)
end = datetime.datetime(2024, 1, 2)
max_timedelta = datetime.timedelta(0)

assert clamp_date_range((start, end), max_timedelta) == (end, end)

def test_clamps_to_end(self):
start = datetime.datetime(2024, 1, 1)
end = datetime.datetime(2024, 1, 14)
max_timedelta = datetime.timedelta(days=1)

assert clamp_date_range((start, end), max_timedelta) == (
datetime.datetime(2024, 1, 13),
end,
)

0 comments on commit 198e038

Please sign in to comment.