Skip to content

feat(attributes): add sampling to autocomplete #7267

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.

Sign up for GitHub

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

Jump to bottom
Merged
volokluev merged 6 commits into from
Jul 8, 2025
Merged

feat(attributes): add sampling to autocomplete #7267

volokluev merged 6 commits into from
Jul 8, 2025

Conversation

@volokluev
Copy link
Member

@volokluev volokluev commented Jul 2, 2025

  1. We have a lot of timeouts on the autocomplete endpoint which uses the main tables to suggest attribute values. We can definitely sample these queries
  2. The TraceItemStats endpoint often times out (especially for large projects), so put in a special case to downgrade the storage tier for them automatically

@volokluev volokluev requested review from a team as code owners July 2, 2025 21:01
@codecov
Copy link

codecov bot commented Jul 2, 2025 •
edited
Loading

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2788 1 2787 12
View the top 1 failed test(s) by shortest run time 
tests.web.rpc.v1.test_endpoint_trace_item_stats_logs.TestTraceItemStatsForLogs::test_with_logs_data
Stack Traces | 0.494s run time 
Traceback (most recent call last):
  File ".../local/lib/python3.11........./site-packages/_pytest/runner.py", line 341, in from_call
    result: TResult | None = func()
                             ^^^^^^
  File ".../local/lib/python3.11........./site-packages/_pytest/runner.py", line 242, in <lambda>
    lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11....../site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11....../site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11.../site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File ".../local/lib/python3.11....../site-packages/_pytest/threadexception.py", line 92, in pytest_runtest_call
    yield from thread_exception_runtest_hook()
  File ".../local/lib/python3.11....../site-packages/_pytest/threadexception.py", line 68, in thread_exception_runtest_hook
    yield
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File ".../local/lib/python3.11....../site-packages/_pytest/unraisableexception.py", line 95, in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
  File ".../local/lib/python3.11....../site-packages/_pytest/unraisableexception.py", line 70, in unraisable_exception_runtest_hook
    yield
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File ".../local/lib/python3.11....../site-packages/_pytest/logging.py", line 846, in pytest_runtest_call
    yield from self._runtest_for(item, "call")
  File ".../local/lib/python3.11....../site-packages/_pytest/logging.py", line 829, in _runtest_for
    yield
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File ".../local/lib/python3.11.../site-packages/_pytest/capture.py", line 880, in pytest_runtest_call
    return (yield)
            ^^^^^
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File ".../local/lib/python3.11.../site-packages/_pytest/skipping.py", line 257, in pytest_runtest_call
    return (yield)
            ^^^^^
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11........./site-packages/_pytest/runner.py", line 174, in pytest_runtest_call
    item.runtest()
  File ".../local/lib/python3.11....../site-packages/_pytest/python.py", line 1627, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File ".../local/lib/python3.11....../site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11....../site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File ".../local/lib/python3.11.........................../site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../local/lib/python3.11....../site-packages/_pytest/python.py", line 159, in pytest_pyfunc_call
    result = testfunction(**testargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../rpc/v1/test_endpoint_trace_item_stats_logs.py", line 79, in test_with_logs_data
    assert actual[0] == AttributeDistribution(
AssertionError: assert attribute_name: "sentry.severity_text"\nbuckets {\n  label: "info"\n  value: 11\n}\n == attribute_name: "sentry.severity_text"\nbuckets {\n  label: "info"\n  value: 60\n}\n
 +  where attribute_name: "sentry.severity_text"\nbuckets {\n  label: "info"\n  value: 60\n}\n = AttributeDistribution(attribute_name='sentry.severity_text', buckets=[label: "info"\nvalue: 60\n])

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

onkar
onkar approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@onkar onkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, added a minor nit.

snuba/web/rpc/v1/endpoint_trace_item_stats.py
Comment on lines +17 to +24
def downgrade_tier(tier: Tier) -> Tier:
if tier == Tier.TIER_1:
return Tier.TIER_8
elif tier == Tier.TIER_8:
return Tier.TIER_64
elif tier == Tier.TIER_64:
return Tier.TIER_512
return tier
Copy link
Contributor

@onkar onkar Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add a simple unit test to make sure this logic is guarded from future changes?

def test_downgrade_tier_behavior():
    assert downgrade_tier(Tier.TIER_1) == Tier.TIER_8
    assert downgrade_tier(Tier.TIER_8) == Tier.TIER_64
    assert downgrade_tier(Tier.TIER_64) == Tier.TIER_512
    assert downgrade_tier(Tier.TIER_512) == Tier.TIER_512

@volokluev volokluev mentioned this pull request Jul 7, 2025
Merged
volokluev added a commit to getsentry/sentry that referenced this pull request Jul 7, 2025
@volokluev
chore(eap): make seer tests not sample (#95010)
6493e48 
In order to merge getsentry/snuba#7267 we want
to make sure that these tests don't break. These tests insert two items
and get their data back. Not indicative of the usual workflow of things.
Make them pass the `HIGHEST_ACCURACY` downsampling mode into the RPC in
order to assure no sampling happens
@volokluev volokluev mentioned this pull request Jul 8, 2025
Merged
volokluev added a commit to getsentry/sentry that referenced this pull request Jul 8, 2025
@volokluev
Use HIGHEST_ACCURACY sampling in org span fields stats endpoint tests (…
dbdd535 
…#95065)

Necessary for getsentry/snuba#7267
@volokluev volokluev merged commit 02d168a into master Jul 8, 2025
35 checks passed
@volokluev volokluev deleted the volo/sample_autocomplete branch July 8, 2025 22:48
andrewshie-sentry pushed a commit to getsentry/sentry that referenced this pull request Jul 14, 2025
@volokluev @andrewshie-sentry
chore(eap): make seer tests not sample (#95010)
11192d9 
In order to merge getsentry/snuba#7267 we want
to make sure that these tests don't break. These tests insert two items
and get their data back. Not indicative of the usual workflow of things.
Make them pass the `HIGHEST_ACCURACY` downsampling mode into the RPC in
order to assure no sampling happens
andrewshie-sentry pushed a commit to getsentry/sentry that referenced this pull request Jul 14, 2025
@volokluev @andrewshie-sentry
Use HIGHEST_ACCURACY sampling in org span fields stats endpoint tests (…
320725c 
…#95065)

Necessary for getsentry/snuba#7267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aayush-se aayush-se aayush-se approved these changes

+1 more reviewer

@onkar onkar onkar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@volokluev @onkar @aayush-se