Skip to content

fix(eap): support integer dynamic columns within VCC#7858

Open
xurui-c wants to merge 6 commits intomasterfrom
rachel/vcc
Open

fix(eap): support integer dynamic columns within VCC#7858
xurui-c wants to merge 6 commits intomasterfrom
rachel/vcc

Conversation

@xurui-c
Copy link
Copy Markdown
Member

@xurui-c xurui-c commented Apr 2, 2026

https://linear.app/getsentry/issue/EAP-462/support-typed-vcc-values-for-integer-attributes

VCC currently only supports normalized columns OR string dynamic. So when you pass in an integer dynamic columns like group_id, it'll try to look for that as a string which is empty.

solution: allow user to specify integer or string for dynamic columns. matching on floats make no sense because floating point imprecision and conceptually makes no sense

VCC history:

@xurui-c xurui-c changed the title c fix(eap): support non-normalized columns within VCC Apr 6, 2026
@xurui-c xurui-c changed the title fix(eap): support non-normalized columns within VCC fix(eap): support non-normalized attributes within VCC Apr 6, 2026
@xurui-c xurui-c marked this pull request as ready for review April 6, 2026 16:56
@xurui-c xurui-c requested review from a team as code owners April 6, 2026 16:56
Copilot AI review requested due to automatic review settings April 6, 2026 16:56
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for virtual column context (VCC) mappings where the source attribute is not normalized and may be typed (notably integer attributes), matching the request shape used by api.organization-events for OCCURRENCE items.

Changes:

  • Bump sentry-protos to >=0.8.9 (and update uv.lock) to use the newer VCC schema.
  • Update the EAP trace item table resolver to honor VirtualColumnContext.from_column_type when building the source expression for VCC mappings.
  • Add an integration test reproducing OCCURRENCE virtual column remapping (e.g. group_id -> issue, sentry.project_id -> project / project.name).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py Uses from_column_type to resolve the correct typed source attribute for VCC transforms.
tests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.py Adds coverage for typed/non-normalized VCC mapping in OCCURRENCE requests.
pyproject.toml Updates sentry-protos minimum version to include required proto fields.
uv.lock Locks the updated sentry-protos version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +161 to +166
f.CAST(
f.ifNull(
attribute_expression,
literal("") if source_type == AttributeKey.TYPE_STRING else literal(0),
),
"String",
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

The null-coalescing fallback for non-string source types uses 0, which can change semantics vs the previous empty-string fallback: a missing value becomes "0" after the cast and could incorrectly match a value_map entry for "0". It also doesn’t handle TYPE_BOOLEAN cleanly (falls into the 0 branch). Consider casting attribute_expression to String first and then applying ifNull(..., "") (or otherwise choosing a fallback literal that matches source_type, e.g. False for booleans) so missing values reliably map to the default_value/"unknown" instead of potentially mapping to a real key.

Suggested change
f.CAST(
f.ifNull(
attribute_expression,
literal("") if source_type == AttributeKey.TYPE_STRING else literal(0),
),
"String",
f.ifNull(
f.CAST(
attribute_expression,
"String",
),
literal(""),

Copilot uses AI. Check for mistakes.
@xurui-c xurui-c changed the title fix(eap): support non-normalized attributes within VCC fix(eap): support integer dynamic columns within VCC Apr 6, 2026
Copy link
Copy Markdown

@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 prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Assert used for validation instead of proper exception
    • Replaced the assert-based source_type check with an explicit BadSnubaRPCRequestException guard so invalid types always produce a proper request error.

Create PR

Or push these changes by commenting:

@cursor push c6a30c42b5
Preview (c6a30c42b5)
diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py
--- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py
+++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py
@@ -151,10 +151,13 @@
                     context.from_column_name, [AttributeKey.TYPE_STRING]
                 )[0]
             )
-            assert source_type in (
+            if source_type not in (
                 AttributeKey.Type.TYPE_STRING,
                 AttributeKey.Type.TYPE_INT,
-            ), "VCC can only map string or int attributes"
+            ):
+                raise BadSnubaRPCRequestException(
+                    "VCC can only map string or int attributes"
+                )
             attribute_expression = attribute_key_to_expression(
                 AttributeKey(
                     name=context.from_column_name,

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7a1209b. Configure here.

AttributeKey.Type.TYPE_STRING,
AttributeKey.Type.TYPE_INT,
):
raise MalformedAttributeException("VCC can only map string or int attributes")
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.

spell out VirtualColumnContext

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants