Skip to content

Conversation

@phacops
Copy link
Contributor

@phacops phacops commented Jan 30, 2026

Summary

Adds two features to support complex formula calculations in EAP queries:

1. NULL-safe division

When a BinaryFormula with OP_DIVIDE has a default_value specified, the division now safely handles NULL and zero divisors:

-- Before: returns NULL when divisor is NULL or zero
count(*) / ((now - min(timestamp)) / 3600)

-- After: returns default value (e.g., 0.0) when divisor is NULL or zero  
if(isNull(divisor) OR divisor = 0, default_value, numerator / divisor)

This enables calculations like count / hours_since_first_seen where hours_since_first_seen derives from min(timestamp).

2. ConditionalFormula support (waiting for sentry-protos PR #173)

Adds support for Column.ConditionalFormula to express conditional expressions:

if(min(timestamp) < one_week_ago, count/WEEK_IN_HOURS, count/hours_since_first_seen)

The code is implemented and ready but dormant until the proto is released. Uses _has_proto_field() helper to safely check for proto fields that don't exist yet.

Changes

  • resolver_trace_item_table.py:

    • Added _has_proto_field() helper for safe proto field checking
    • Added COMPARISON_OP_TO_EXPR mapping for conditional formula operators
    • Modified _formula_to_expression() for NULL-safe division
    • Added _conditional_formula_to_expression() using existing if_cond() DSL
    • Updated _column_to_expression() to handle conditional_formula
    • Updated _get_reliability_context_columns() for conditional formulas
  • proto_visitor.py:

    • Updated ColumnWrapper.accept() to traverse conditional formula children

Test plan

  • test_hourly_rate_with_dynamic_divisor_using_aggregate passes (NULL-safe division)
  • test_conditional_rate_based_on_aggregate_first_seen skips until proto available
  • All existing tests pass (91 passed, 1 skipped, 2 xfailed)
  • Linting and mypy pass

🤖 Generated with Claude Code

phacops and others added 4 commits January 30, 2026 10:07
Add test coverage for querying EAP with OCCURRENCE item type to calculate
hourly event rates. The tests validate:

- Counting occurrences grouped by group_id (issue)
- Calculating hourly rates using division formulas (count / WEEK_IN_HOURS)
- Taking P95 of hourly_event_rate across issues
- Conditional aggregations based on issue age (old vs new issues)

The hourly event rate formula tested:
- Old issues (>1 week): rate = event_count / WEEK_IN_HOURS
- New issues (<1 week): rate = event_count / hours_since_first_seen

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ed query patterns

Updated tests to properly categorize:

SUPPORTED:
- Count and min(timestamp) per group
- Division by static literal (count / WEEK_IN_HOURS)
- P95 of pre-computed attribute values

UNSUPPORTED (xfail):
- Division by nested formula containing aggregate (returns null)
- Conditional expression with aggregate in condition (no 'if' operator)
- P95 of calculated aggregate formula across groups (nested aggregation)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds test_countif_with_aggregate_in_condition to demonstrate the core
limitation: we cannot use an aggregate result (e.g., min(timestamp) as
first_seen) in a conditional aggregation filter.

What WORKS:
- countIf(timestamp > X) - filtering individual rows

What DOESN'T WORK:
- countIf(min(timestamp) > X) - condition uses an aggregate

This is marked xfail as AttributeConditionalAggregation.filter can only
compare row-level attributes to literals, not aggregate results.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…formulas

Phase 1 - NULL-safe division:
- Modified _formula_to_expression() to use if(isNull(right) OR right = 0, default, left / right)
  when default_value is specified, protecting against NULL/zero divisors
- This fixes hourly rate calculations where divisor uses aggregates like min(timestamp)

Phase 2 - ConditionalFormula support (code ready, waiting for sentry-protos PR #173):
- Added _has_proto_field() helper to safely check for proto fields that may not exist yet
- Added COMPARISON_OP_TO_EXPR mapping for conditional formula comparison operators
- Added _conditional_formula_to_expression() using existing if_cond() DSL
- Updated _column_to_expression() to handle conditional_formula field
- Updated _get_reliability_context_columns() for conditional formulas
- Updated ColumnWrapper.accept() in proto_visitor.py to traverse conditional formula children

Tests:
- test_hourly_rate_with_dynamic_divisor_using_aggregate now passes
- test_conditional_rate_based_on_aggregate_first_seen updated to use ConditionalFormula
  proto structure (skips until proto is available)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@phacops phacops marked this pull request as ready for review January 30, 2026 23:15
@phacops phacops requested review from a team as code owners January 30, 2026 23:15
@phacops phacops changed the title feat(eap): Add tests for OCCURRENCE item type hourly event rate queries feat(eap): Add NULL-safe division and ConditionalFormula support for formulas Jan 30, 2026
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.

2 participants