✨ feat(aci): Update EvidenceData to match Stateful Detector Evidence Data
#93676
Conversation
github-actions
bot
added
the
Scope: Backend
| @@ -32,7 +32,7 @@ | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class MetricIssueEvidenceData(EvidenceData): | |||
| class MetricIssueEvidenceData(EvidenceData[float]): | |||
There was a problem hiding this comment.
just want to confirm that it's a float for all metric issues?
There was a problem hiding this comment.
yep, i have dug through a lot of code around this we use float and int interchangeably for typing in this area, but based on queries i have ran and some issues i have seen in the past, the correct type is float
| for condition in evidence_data.conditions: | ||
| # The condition.condition_result is of type DetectorPriorityLevel | ||
| if ( | ||
| condition["condition_result"] | ||
| == PRIORITY_LEVEL_TO_DETECTOR_PRIORITY_LEVEL[PriorityLevel(priority_level)] | ||
| ): | ||
| threshold_type = fetch_threshold_type(Condition(condition["type"])) | ||
| resolve_threshold = fetch_resolve_threshold(condition["comparison"], group_status) | ||
| alert_threshold = fetch_alert_threshold(condition["comparison"], group_status) | ||
| sensitivity = fetch_sensitivity(condition) | ||
| break | ||
| else: | ||
| raise ValueError("No threshold type found for metric issues") |
There was a problem hiding this comment.
🤔 i wonder if we could split this up a little differently, first finding the condition that we care about, then getting the information about it.
target_priority = PRIORITY_LEVEL_TO_DETECTOR_PRIORITY_LEVEL[PriorityLevel(priority_level)]
try:
condition = next(
cond for cond in evidence_data.conditions
if cond["condition_result"] == target_priority
)
threshold_type = fetch_threshold_type(Condition(condition["type"]))
resolve_threshold = fetch_resolve_threshold(condition["comparison"], group_status)
alert_threshold = fetch_alert_threshold(condition["comparison"], group_status)
sensitivity = fetch_sensitivity(condition)
except StopIteration:
# thrown by `next` if no value is found
raise ValueError("No threshold type found for metric issues")This works more or less the same way as you have it, but it will find the condition we care about, then we can find values. (more info about next here)
| @@ -486,6 +487,7 @@ def _create_decorated_issue_occurrence( | |||
| **detector_occurrence.evidence_data, | |||
| "detector_id": self.detector.id, | |||
| "value": new_priority, | |||
| "data_source_id": data_packet.source_id, | |||
There was a problem hiding this comment.
🤔 we should probably come up with a different name for this, data_packet_source_id? i fear data_source_id will be confused with DataSource.id -- if we include packet it will also be clear that this is coming from the evaluation of a single data packet.
There was a problem hiding this comment.
good point, i think data_packet_source_id is good
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #93676 +/- ##
==========================================
+ Coverage 88.02% 88.03% +0.01%
==========================================
Files 10328 10327 -1
Lines 596000 595765 -235
Branches 23154 23108 -46
==========================================
- Hits 524638 524505 -133
+ Misses 70869 70767 -102
Partials 493 493 |
Updates the
EvidenceDatadataclass to match what theStatefuleDetectoradds to theIssueOccurrencefor evidence data.Also updates how the NOA handles and parses this data to fetch the appropriate information