chore: Tag extrapolation mode #103540
chore: Tag extrapolation mode #103540
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| extrapolation_mode_str = None | ||
| if dataset == Dataset.PerformanceMetrics.value: | ||
| extrapolation_mode = ExtrapolationMode.EXTRAPOLATION_MODE_SERVER_ONLY | ||
| extrapolation_mode_str = "serverOnly" | ||
| if dataset == Dataset.Transactions.value: | ||
| extrapolation_mode = ExtrapolationMode.EXTRAPOLATION_MODE_NONE | ||
| extrapolation_mode_str = "none" |
There was a problem hiding this comment.
Bug: The extrapolation_mode_str variable remains None for unhandled datasets, leading to missing Sentry tags and extrapolationMode=None in API calls.
Severity: CRITICAL | Confidence: 0.95
🔍 Detailed Analysis
The extrapolation_mode_str variable is initialized to None and only assigned string values for Dataset.PerformanceMetrics.value and Dataset.Transactions.value. When make_rpc_request() is called with other dataset values, extrapolation_mode_str remains None. This causes sentry_sdk.set_tag("extrapolation_mode", extrapolation_mode_str) to not set a tag, and the API URL will include extrapolationMode=None, which downstream APIs may misinterpret or not expect, leading to silent failures for alert rules using unsupported datasets.
💡 Suggested Fix
Add an else clause or elif to provide a default extrapolation_mode_str for unhandled datasets, or implement input validation to restrict dataset to supported types.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/discover/compare_timeseries.py#L93-L99
Potential issue: The `extrapolation_mode_str` variable is initialized to `None` and only
assigned string values for `Dataset.PerformanceMetrics.value` and
`Dataset.Transactions.value`. When `make_rpc_request()` is called with other `dataset`
values, `extrapolation_mode_str` remains `None`. This causes
`sentry_sdk.set_tag("extrapolation_mode", extrapolation_mode_str)` to not set a tag, and
the API URL will include `extrapolationMode=None`, which downstream APIs may
misinterpret or not expect, leading to silent failures for alert rules using unsupported
datasets.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2780202
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103540 +/- ##
===========================================
- Coverage 80.58% 80.58% -0.01%
===========================================
Files 9261 9261
Lines 395411 395415 +4
Branches 25212 25212
===========================================
+ Hits 318641 318642 +1
- Misses 76340 76343 +3
Partials 430 430 |
| sampling_mode="NORMAL", | ||
| ) | ||
|
|
||
| sentry_sdk.set_tag("extrapolation_mode", extrapolation_mode_str) |
There was a problem hiding this comment.
@Zylphrex extrapolation mode isn't the same as sampling mode! extrapolation mode refers to what sampling factors we want to consider for extrapolation (new stuff added so we can migrate alerts)
There was a problem hiding this comment.
ahh my bad, but also oh boy, another mode?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| start=snuba_params.start.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), | ||
| end=snuba_params.end.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), | ||
| sampling="NORMAL", | ||
| extrapolationMode=extrapolation_mode_str, |
There was a problem hiding this comment.
Bug: Extrapolation mode string passed as None to URL
When dataset is neither PerformanceMetrics nor Transactions, extrapolation_mode_str remains None and gets passed to format_api_call. Python's urlencode converts None to the string "None", resulting in URLs like ...?extrapolationMode=None. This creates misleading debug URLs in Sentry extras. The parameter should either be omitted when None or use a meaningful default value.
No description provided.