Skip to content

fix(api): Hide internal attributes in events and cross-trace queries#116094

Closed
nsdeschenes wants to merge 8 commits into
masterfrom
nd/feat-attributes-visibility-events
Closed

fix(api): Hide internal attributes in events and cross-trace queries#116094
nsdeschenes wants to merge 8 commits into
masterfrom
nd/feat-attributes-visibility-events

Conversation

@nsdeschenes
Copy link
Copy Markdown
Contributor

Summary

  • Apply visibility checks to organization_events and organization_events_timeseries endpoints — queries on hidden internal attributes return empty results for non-staff users
  • Refactor get_cross_trace_queries to accept the full SearchResolver instead of config + params, propagating hidden attribute markers from cross-trace sub-resolvers back to the primary resolver
  • Add has_hidden_api_attributes() short-circuit in both run_table_query and run_bulk_table_query to avoid dispatching RPCs
  • Derive the visibility item type from the resolver's column definitions rather than the primary request type, so cross-trace sub-resolvers use the correct item type
  • Return properly structured empty timeseries responses (with field metadata) when attributes are hidden

Depends on #116091

Test plan

  • New tests in test_organization_events_span_indexed.py, test_organization_events_timeseries_spans.py, test_organization_events_cross_trace.py, and test_rpc_dataset_common.py
  • Coverage for log resolvers receiving a spans visibility config in test_ourlogs.py

Closes TODO

nsdeschenes and others added 8 commits May 22, 2026 10:45
Add a shared helper for hiding internal Sentry convention attributes from API surfaces and let SearchResolver track attributes hidden by API visibility configuration.

This keeps default resolver behavior unchanged unless an API caller opts into visibility enforcement.
Apply API attribute visibility checks when the events endpoint queries RPC datasets. This prevents non-staff users from selecting internal attributes with field params and receiving their values.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Apply the same API attribute visibility checks to the events-timeseries RPC path. Return an empty timeseries response instead of querying hidden internal attributes for non-staff users.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Return empty timeseries metadata when API visibility hides requested attributes, and type resolver visibility options explicitly so backend typing accepts the config calls.

Co-Authored-By: Codex <noreply@openai.com>
Rename the hidden-attribute metadata variable so mypy does not see two typed definitions for final_meta in the same function scope.

Co-Authored-By: Codex <codex@openai.com>
Propagate hidden API attribute markers from cross-trace resolvers back to the primary resolver so table and bulk table queries return empty results instead of dispatching RPCs. Resolve cross-trace query visibility using the target trace item type for span, log, metric, and occurrence filters.

Add regression coverage for non-staff cross-trace filters on internal span and log attributes.

Co-Authored-By: Codex <noreply@openai.com>
Add regression coverage that cross-trace occurrence filters resolve API attribute visibility with the occurrence item type instead of the primary spans item type.

Co-Authored-By: Codex <noreply@openai.com>
Use the trace item type from the resolver's column definitions for API attribute visibility checks. This keeps dataset-specific resolvers from applying a mismatched primary request item type.

Add coverage for log resolvers receiving a spans visibility config.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 22, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e480b18. Configure here.

request_context_pairs.append(
(query.name, table_request, cls.build_rpc_table_row_context(query))
)
responses = snuba_rpc.table_rpc(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Table RPC skips hidden short-circuit

High Severity

This change adds has_hidden_api_attributes() guards before Snuba in bulk table and plain timeseries paths, but _run_table_query still always calls snuba_rpc.table_rpc after resolution. Organization events and top-events timeseries table steps use that path, so non-staff queries touching hidden internal fields or cross-trace filters can still return real data instead of the empty results the new tests expect.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e480b18. Configure here.

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 22, 2026

EXP-966

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Backend Test Failures

Failures on f7e99dd in this run:

tests/snuba/api/endpoints/test_organization_events_cross_trace.py::OrganizationEventsCrossTraceEndpointTest::test_cross_trace_query_with_internal_log_attribute_is_staff_onlylog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/snuba/api/endpoints/test_organization_events_cross_trace.py:195: in test_cross_trace_query_with_internal_log_attribute_is_staff_only
    assert response.data["data"] == []
E   AssertionError: assert [{'count()': ...oo]': 'five'}] == []
E     
E     Left contains one more item: �[0m{�[33m'�[39;49;00m�[33mcount()�[39;49;00m�[33m'�[39;49;00m: �[94m1.0�[39;49;00m, �[33m'�[39;49;00m�[33mtags[foo]�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33mfive�[39;49;00m�[33m'�[39;49;00m}�[90m�[39;49;00m
E     
E     Full diff:
E     �[0m�[91m- []�[39;49;00m�[90m�[39;49;00m
E     �[92m+ [�[39;49;00m�[90m�[39;49;00m
E     �[92m+     {�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'count()': 1.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'tags[foo]': 'five',�[39;49;00m�[90m�[39;49;00m
E     �[92m+     },�[39;49;00m�[90m�[39;49;00m
E     �[92m+ ]�[39;49;00m�[90m�[39;49;00m
tests/snuba/api/endpoints/test_organization_events_span_indexed.py::OrganizationEventsSpansEndpointTest::test_sentry_internal_field_values_are_staff_onlylog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/snuba/api/endpoints/test_organization_events_span_indexed.py:4579: in test_sentry_internal_field_values_are_staff_only
    assert response.data["data"] == []
E   AssertionError: assert [{'__sentry_i...ount()': 1.0}] == []
E     
E     Left contains one more item: �[0m{�[33m'�[39;49;00m�[33m__sentry_internal_test�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33minternal_value�[39;49;00m�[33m'�[39;49;00m, �[33m'�[39;49;00m�[33mcount()�[39;49;00m�[33m'�[39;49;00m: �[94m1.0�[39;49;00m}�[90m�[39;49;00m
E     
E     Full diff:
E     �[0m�[91m- []�[39;49;00m�[90m�[39;49;00m
E     �[92m+ [�[39;49;00m�[90m�[39;49;00m
E     �[92m+     {�[39;49;00m�[90m�[39;49;00m
E     �[92m+         '__sentry_internal_test': 'internal_value',�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'count()': 1.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+     },�[39;49;00m�[90m�[39;49;00m
E     �[92m+ ]�[39;49;00m�[90m�[39;49;00m
tests/snuba/api/endpoints/test_organization_events_cross_trace.py::OrganizationEventsCrossTraceEndpointTest::test_cross_trace_query_with_internal_span_attribute_is_staff_onlylog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/snuba/api/endpoints/test_organization_events_cross_trace.py:145: in test_cross_trace_query_with_internal_span_attribute_is_staff_only
    assert response.data["data"] == []
E   AssertionError: assert [{'count()': ...oo]': 'five'}] == []
E     
E     Left contains one more item: �[0m{�[33m'�[39;49;00m�[33mcount()�[39;49;00m�[33m'�[39;49;00m: �[94m1.0�[39;49;00m, �[33m'�[39;49;00m�[33mtags[foo]�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33mfive�[39;49;00m�[33m'�[39;49;00m}�[90m�[39;49;00m
E     
E     Full diff:
E     �[0m�[91m- []�[39;49;00m�[90m�[39;49;00m
E     �[92m+ [�[39;49;00m�[90m�[39;49;00m
E     �[92m+     {�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'count()': 1.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'tags[foo]': 'five',�[39;49;00m�[90m�[39;49;00m
E     �[92m+     },�[39;49;00m�[90m�[39;49;00m
E     �[92m+ ]�[39;49;00m�[90m�[39;49;00m
tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py::OrganizationEventsStatsSpansMetricsEndpointTest::test_sentry_internal_groupby_values_are_staff_onlylog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/snuba/api/endpoints/test_organization_events_timeseries_spans.py:168: in test_sentry_internal_groupby_values_are_staff_only
    assert response.data["timeSeries"] == []
E   AssertionError: assert [{'groupBy': ...': 'count()'}] == []
E     
E     Left contains one more item: �[0m{�[33m'�[39;49;00m�[33mgroupBy�[39;49;00m�[33m'�[39;49;00m: [{�[33m'�[39;49;00m�[33mkey�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33m__sentry_internal_test�[39;49;00m�[33m'�[39;49;00m, �[33m'�[39;49;00m�[33mvalue�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33minternal_value�[39;49;00m�[33m'�[39;49;00m}], �[33m'�[39;49;00m�[33mmeta�[39;49;00m�[33m'�[39;49;00m: {�[33m'�[39;49;00m�[33mdataScanned�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33mfull�[39;49;00m�[33m'�[39;49;00m, �[33m'�[39;49;00m�[33minterval�[39;49;00m�[33m'�[39;49;00m... �[94mNone�[39;49;00m, ...}, {�[33m'�[39;49;00m�[33mconfidence�[39;49;00m�[33m'�[39;49;00m: �[94mNone�[39;49;00m, �[33m'�[39;49;00m�[33mincomplete�[39;49;00m�[33m'�[39;49;00m: �[94mFalse�[39;49;00m, �[33m'�[39;49;00m�[33msampleCount�[39;49;00m�[33m'�[39;49;00m: �[94m0�[39;49;00m, �[33m'�[39;49;00m�[33msampleRate�[39;49;00m�[33m'�[39;49;00m: �[94mNone�[39;49;00m, ...}], �[33m'�[39;49;00m�[33myAxis�[39;49;00m�[33m'�[39;49;00m: �[33m'�[39;49;00m�[33mcount()�[39;49;00m�[33m'�[39;49;00m}�[90m�[39;49;00m
E     
E     Full diff:
E     �[0m�[91m- []�[39;49;00m�[90m�[39;49;00m
E     �[92m+ [�[39;49;00m�[90m�[39;49;00m
E     �[92m+     {�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'groupBy': [�[39;49;00m�[90m�[39;49;00m
E     �[92m+             {�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'key': '__sentry_internal_test',�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'value': 'internal_value',�[39;49;00m�[90m�[39;49;00m
E     �[92m+             },�[39;49;00m�[90m�[39;49;00m
E     �[92m+         ],�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'meta': {�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'dataScanned': 'full',�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'interval': 3600000,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'isOther': False,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'order': 0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'valueType': 'integer',�[39;49;00m�[90m�[39;49;00m
E     �[92m+             'valueUnit': None,�[39;49;00m�[90m�[39;49;00m
E     �[92m+         },�[39;49;00m�[90m�[39;49;00m
E     �[92m+         'values': [�[39;49;00m�[90m�[39;49;00m
E     �[92m+             {�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'confidence': 'high',�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'incomplete': False,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleCount': 1,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleRate': 1.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'timestamp': 1779357600000,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'value': 1.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             },�[39;49;00m�[90m�[39;49;00m
E     �[92m+             {�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'confidence': None,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'incomplete': False,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleCount': 0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleRate': None,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'timestamp': 1779361200000,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'value': 0.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             },�[39;49;00m�[90m�[39;49;00m
E     �[92m+             {�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'confidence': None,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'incomplete': False,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleCount': 0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'sampleRate': None,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'timestamp': 1779364800000,�[39;49;00m�[90m�[39;49;00m
E     �[92m+                 'value': 0.0,�[39;49;00m�[90m�[39;49;00m
E     �[92m+             },�[39;49;00m�[90m�[39;49;00m
... (28 more lines)

Base automatically changed from nd/feat-attributes-visibility-core to master May 27, 2026 13:24
@nsdeschenes
Copy link
Copy Markdown
Contributor Author

Closing out in favour of a clean slate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant