Skip to content

Conversation

@kcons
Copy link
Member

@kcons kcons commented Nov 8, 2025

DataCondition failures are currently reported to Sentry/logs individually, then treated as a non-triggering.
Here we return a special error type, so that the caller knows the result isn't necessarily reliable, and in DataConditionGroup logic we aggregate these results in such a way that we know at the end if an untrustworthy result influenced the outcome.

These "tainted" results are reported for each Detector evaluation, so we can track how often our results were influenced by errors, giving us a better high level picture of user impact than we'd have purely by looking at individual reported error counts.

This pattern will be extended to cover over DataCondition evaluation cases.

@kcons kcons requested review from a team as code owners November 8, 2025 00:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 8, 2025
@codecov
Copy link

codecov bot commented Nov 8, 2025

Codecov Report

❌ Patch coverage is 91.37931% with 10 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...c/sentry/workflow_engine/handlers/detector/base.py 81.25% 3 Missing ⚠️
...rc/sentry/workflow_engine/models/data_condition.py 81.81% 2 Missing ⚠️
...workflow_engine/processors/data_condition_group.py 97.01% 2 Missing ⚠️
src/sentry/grouping/grouptype.py 80.00% 1 Missing ⚠️
...ntry/workflow_engine/handlers/detector/stateful.py 85.71% 1 Missing ⚠️
src/sentry/workflow_engine/models/workflow.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103003      +/-   ##
===========================================
+ Coverage   80.58%    80.82%   +0.23%     
===========================================
  Files        8937      8948      +11     
  Lines      391228    391894     +666     
  Branches    24887     24887              
===========================================
+ Hits       315277    316746    +1469     
+ Misses      75581     74778     -803     
  Partials      370       370              

@kcons
Copy link
Member Author

kcons commented Nov 8, 2025

TODO: document how DataConditions should handle failure.

@kcons kcons marked this pull request as ready for review November 11, 2025 00:42
if condition_results is not None and condition_results.logic_result.is_tainted():
tainted = True

if condition_results is None or condition_results.logic_result.triggered is False:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Group Counter Resets Misdirected

enqueue_counter_reset() is called without passing group_key, causing it to reset counters for the default None group key instead of the current group being processed. This results in counter resets being applied to the wrong group when multiple groups are being evaluated.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants