Skip to content

fix: Validate chart metric column for non-count aggregations#1391

Draft
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/chart-metric-column-validation
Draft

fix: Validate chart metric column for non-count aggregations#1391
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/chart-metric-column-validation

Conversation

@sentry

@sentry sentry Bot commented May 28, 2026

Copy link
Copy Markdown

This PR addresses the ValueError: Column is required for avg aggregation issue (DALGO-BACKEND-2GW) by implementing early input validation for chart metrics.

Previously, the API allowed requests with non-count aggregations (e.g., avg) to be submitted with an empty string for the column field. This would pass initial Pydantic validation but lead to a ValueError at runtime in ddpui/core/charts/charts_service.py. This runtime error was then caught and logged as an error in Sentry, generating unnecessary alerts for what is fundamentally a client-side input validation problem.

Changes include:

  • Added a @model_validator(mode='after') to the ChartMetric schema in ddpui/schemas/chart_schema.py. This validator now:
    • Normalizes any empty string column values to None.
    • Raises a ValueError if a non-count aggregation is specified without a valid column (i.e., column is None).
  • Downgraded the logger.error to logger.warning in the ValueError catch blocks within ddpui/api/charts_api.py for chart data generation endpoints. This ensures that validation-related errors are logged appropriately without triggering high-severity Sentry alerts.

This ensures that invalid chart metric configurations are rejected with a clear 422 HTTP response at the API boundary, improving API robustness and reducing Sentry noise. Existing runtime checks in charts_service.py remain as a defense-in-depth.

Fixes DALGO-BACKEND-2GW

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.

0 participants