feat(upsampling) - Support upsampled error count with performance optimizations

[zaibkhan]

This PR introduces upsampled error count support in the events-stats API, improving error metrics accuracy under race conditions. It also includes performance optimizations with caching to reduce response latency.

Key changes:

• Add upsampled error count path to events-stats
• Mitigate race conditions around sampling computations
• Add caching/perf improvements for hot endpoints
• Update tests to cover the new path

Benchmark reference:
Replicated from ai-code-review-evaluation/sentry-greptile PR #3

[codoki-pr-intelligence]

Codoki PR Review

Summary: Register upsampled_count, fix cache key stability, apply safe transforms
What’s good: Clear separation of concerns: eligibility check, dataset guard, and late column transformation minimize unintended scope. Integration tests exercise the new path across errors/transactions and allowlist mixes.
Review Status: ❌ Requires changes
Overall Priority: High

Issues (Critical & High only)

Severity	Issue	Why it matters	File
High	Correctness — upsampled_count not registered in production registry	upsampled_count is only registered in the test factory but not in the production function registry (e.g., sentry.snuba.functions). In production, this will cause query parsing to fail when the transform is applied, breaking the endpoint for allowlisted orgs.	…/helpers/error_upsampling.py

Key Feedback (click to expand)

• Needs improvement: Cache key construction uses Python's hash(), which is non-deterministic across processes and will break shared caching/invalidation; switch to a deterministic key. The new upsampling function is only registered in test factories, so live queries will error where the transform is applied.
• Testing: Integration tests pass by registering upsampled_count in test factories, masking the fact that production function registry lacks this function. Add a test that runs through the real function registry (e.g., via sentry.snuba.functions) to ensure 'upsampled_count' is defined in production code.
• Documentation: Consider documenting in error_upsampling.py where/when invalidate_upsampling_cache must be triggered (e.g., option change hook), and that upsampled_count must be present in the production function registry.
• Performance: Using Python's hash() in cache keys prevents cross-process cache reuse and breaks invalidation; use a deterministic join or stable digest of project IDs to enable shared caching and reliable eviction.
• Open questions: What mechanism will call invalidate_upsampling_cache when the allowlist option changes? If none exists, should we reduce TTL or wire a signal to ensure consistency?

Confidence: 3/5 — Needs work before merge (1 high · status: Requires changes)

Sequence Diagram

sequenceDiagram
    participant Client
    participant Endpoint
    participant Helper
    Client->>Endpoint: GET /organization-events-stats
    Endpoint->>Helper: is_errors_query_for_error_upsampled_projects(snuba_params, org, dataset, request)
    alt cache hit
        Helper-->>Endpoint: cached_result && _should_apply_sample_weight_transform()
    else cache miss
        Helper->>Helper: _are_all_projects_error_upsampled()
        Helper->>Helper: cache.set(key, is_eligible, 60)
        Helper-->>Endpoint: is_eligible && _should_apply_sample_weight_transform()
    end
    alt should_upsample
        Endpoint->>Endpoint: transform_query_columns_for_error_upsampling()
    end
    Endpoint-->>Client: timeseries response

Loading

React with 👍 or 👎 if you found this review useful.

[codoki-pr-intelligence]

🔷 Medium: Using Python's hash() in a cache key is non-deterministic across processes (hash randomization) and will break shared cache reuse and invalidation, leading to inconsistent behavior and missed performance benefits.

Suggested change

	cache_key = f"error_upsampling_eligible:{organization.id}:{hash(tuple(sorted(snuba_params.project_ids)))}"
	cache_key = f"error_upsampling_eligible:{organization.id}:{','.join(map(str, sorted(snuba_params.project_ids)))}"

[codoki-pr-intelligence]

🔷 Medium: Same issue as above: invalidation key must match the setting key across processes; using hash() prevents reliable eviction.

Suggested change

	cache_key = f"error_upsampling_eligible:{organization_id}:{hash(tuple(sorted(project_ids)))}"
	cache_key = f"error_upsampling_eligible:{organization_id}:{','.join(map(str, sorted(project_ids)))}"

[codoki-pr-intelligence]

⚠️ High: upsampled_count is only registered in the test factory but not in the production function registry (e.g., sentry.snuba.functions). In production, this will cause query parsing to fail when the transform is applied, breaking the endpoint for allowlisted orgs.