[CORRUPTED] Synthetic Benchmark PR #103102 - feat(tracemetrics): Parse metric from aggregations

[tomerqodo]

User description

Benchmark PR getsentry#103102

Type: Corrupted (contains bugs)

Original PR Title: feat(tracemetrics): Parse metric from aggregations
Original PR Description: This parses the metric from aggregations instead of relying on the top level metric name/type. The top level ones still take precedence for better compatibility. It has the limitation that it only allows a single metric across all the aggregations.
Original PR URL: getsentry#103102

---

PR Type

Enhancement

---

Description

• Parse metric metadata from aggregation functions instead of relying solely on top-level metric parameters

• Extract metric name, type, and unit from selected columns and equations to automatically filter trace metrics

• Refactor metric filtering logic into reusable utility functions and configuration methods

• Add validation to prevent querying multiple different metrics in a single request

---

Diagram Walkthrough

flowchart LR
  A["Aggregation Functions"] -->|extract metric metadata| B["ResolvedMetricAggregate"]
  C["Selected Columns/Equations"] -->|resolve and parse| B
  B -->|validate single metric| D["Metric Filter"]
  E["Top-level Config"] -->|fallback to| D
  D -->|apply conditions| F["Query Execution"]

Loading

File Walkthrough

	Relevant files
Enhancement	columns.py Add metric metadata extraction to aggregates                          src/sentry/search/eap/columns.py Add ResolvedMetricAggregate dataclass to store metric metadata (name, type, unit) extracted from aggregations Import cast utility and MetricType from types module Modify TraceMetricAggregateDefinition.resolve() to extract and return metric information from aggregation arguments Update return type from ResolvedAggregate to ResolvedMetricAggregate with populated metric fields +24/-6    resolver.py Refactor query resolution with dataset conditions                src/sentry/search/eap/resolver.py Extract environment query merging logic into and_trace_item_filters() utility function Add new resolve_query_with_columns() method that calls resolve_dataset_conditions() to inject dataset-specific filters Implement resolve_dataset_conditions() to delegate to config's extra_conditions() method with selected columns and equations Simplify resolve_query() to use the new utility function for filter merging +35/-22  rpc_utils.py Add trace item filter utility functions                                    src/sentry/search/eap/rpc_utils.py Create new utility module with and_trace_item_filters() function Implement filter merging logic that handles None values and combines multiple filters with AND logic Support combining 1 or more filters into a single TraceItemFilter with AndFilter +14/-0    config.py Extract metrics from aggregations with fallback                    src/sentry/search/eap/trace_metrics/config.py Move MetricType definition from local to types.py module Create Metric dataclass to represent metric with name, type, and unit Refactor extra_conditions() to accept selected_columns and equations parameters Add _extra_conditions_from_metric() to handle top-level metric config filtering Add _extra_conditions_from_columns() to extract metrics from aggregation functions and equations Extract get_metric_filter() as standalone function to build filter conditions for a metric Add validation to raise InvalidSearchQuery when multiple different metrics are detected +134/-37 types.py Update config interface and centralize types                          src/sentry/search/eap/types.py Update extra_conditions() signature to accept selected_columns and equations parameters Move MetricType type alias definition from trace_metrics config to this module Define MetricType as Literal["counter", "gauge", "distribution"] +9/-1      rpc_dataset_common.py Use centralized filter utilities and new query resolution src/sentry/snuba/rpc_dataset_common.py Import and_trace_item_filters from new rpc_utils module Update get_table_rpc_request() to call resolve_query_with_columns() instead of resolve_query() Remove duplicate and_trace_item_filters() function definition (moved to rpc_utils) Update get_timeseries_query() to call resolve_query_with_columns() with selected axes and equations Remove manual extra_conditions calls from run_top_events_timeseries_query() as they are now handled internally +20/-30  trace_metrics.py Remove redundant extra conditions handling                              src/sentry/snuba/trace_metrics.py Remove manual extra_conditions calls from run_table_query() and run_timeseries_query() Simplify method signatures by removing extra_conditions parameter from TableQuery calls Rely on internal query resolution to handle metric condition injection +0/-6
Tests	test_organization_events_trace_metrics.py Update tests for metric aggregation changes                            tests/snuba/api/endpoints/test_organization_events_trace_metrics.py Update test assertions to use - instead of none for metric unit parameter in aggregation functions Add three new test cases for embedded metric aggregations Test single metric extraction from aggregation functions Test multiple aggregations with same metric name Test error handling when multiple different metrics are specified +74/-11

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing Auditing: The new metric-parsing and condition-injection paths add/alter critical query-building behavior without adding any audit logging of who performed the action, when, and what was applied. Referred Code def resolve_query_with_columns( self, querystring: str | None, selected_columns: list[str] | None, equations: list[str] | None, ) -> tuple[ TraceItemFilter | None, AggregationFilter | None, list[VirtualColumnDefinition | None], ]: where, having, contexts = self.resolve_query(querystring) # Some datasets like trace metrics require we inject additional # conditions in the top level. dataset_conditions = self.resolve_dataset_conditions(selected_columns, equations) where = and_trace_item_filters(where, dataset_conditions) return where, having, contexts Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Broad Exception: In metric extraction from equations an overly broad exception is caught and ignored, which may hide parsing/validation failures and degrade input validation guarantees. Referred Code try: resolved_equation, _ = search_resolver.resolve_equation(equation) if isinstance(resolved_equation, ResolvedMetricAggregate): if resolved_equation.metric_name and resolved_equation.metric_type: metric = Metric( metric_name=resolved_equation.metric_name, metric_type=resolved_equation.metric_type, metric_unit=resolved_equation.metric_unit, ) selected_metrics.add(metric) except Exception: pass Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid catching and silencing all exceptions Replace the broad except Exception: pass with except InvalidSearchQuery: pass to avoid silencing unexpected errors during equation resolution. src/sentry/search/eap/trace_metrics/config.py [79-93] if equations: for equation in equations: # Attempt to resolve the equation to extract metrics try: resolved_equation, _ = search_resolver.resolve_equation(equation) if isinstance(resolved_equation, ResolvedMetricAggregate): if resolved_equation.metric_name and resolved_equation.metric_type: metric = Metric( metric_name=resolved_equation.metric_name, metric_type=resolved_equation.metric_type, metric_unit=resolved_equation.metric_unit, ) selected_metrics.add(metric) - except Exception: + except InvalidSearchQuery: + # This can happen if the equation is not a valid metric aggregation, + # which is fine. We only care about valid ones. pass Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that catching a broad Exception and silencing it is a bad practice that can hide bugs. Replacing it with a more specific InvalidSearchQuery improves code robustness and maintainability by allowing unexpected errors to surface while still correctly handling expected parsing failures.	Medium
	Validate metric type against allowed values Add a runtime validation check to ensure the metric_type argument is one of the allowed values ("counter", "gauge", "distribution") before casting. src/sentry/search/eap/columns.py [403-410] if all( isinstance(resolved_argument, str) and resolved_argument != "" for resolved_argument in resolved_arguments[1:] ): # a metric was passed metric_name = cast(str, resolved_arguments[1]) - metric_type = cast(MetricType, resolved_arguments[2]) + metric_type_val = resolved_arguments[2] + if metric_type_val not in MetricType.__args__: + raise InvalidSearchQuery(f"Invalid metric type '{metric_type_val}'. Must be one of {MetricType.__args__}") + metric_type = cast(MetricType, metric_type_val) metric_unit = cast(str, resolved_arguments[3]) Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a missing runtime validation for metric_type. Adding this check improves the robustness of the code by ensuring only valid metric types are processed, preventing potential downstream errors and providing clearer feedback to the user on invalid input.	Medium
More