feat(aci): Add serialized data source to issue occurrence evidence data

[malwilley]

When viewing the issue details for a metric detector open period, we want to display the query which failed. This adds the serialized data source on the issue occurrence's evidence_data for this purpose. I've done this in a generalized way that should work across all detector types.

[malwilley]

Is it okay to prefetch the data sources here as well? Or would it be better to wait and query for this when the occurrence is being generated?

[saponifi3d]

it's probably better to do the select when we're generating the issue occurrence.

we only create an occurrence / status change on like 0.5% of the evaluations so it's a bit more efficient to make a second select statement.

we also can't guarantee that other product areas will use this method. for example, uptime fetches their detectors separately, then directly calls process_detectors.

[malwilley]

Makes sense! Pushed that change, we now query for the data source when building the evidence data.

[codecov]

Codecov Report

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

Files with missing lines	Patch %	Lines
...ntry/workflow_engine/handlers/detector/stateful.py	76.92%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #103549      +/-   ##
===========================================
+ Coverage   80.09%    80.58%   +0.48%     
===========================================
  Files        9256      9259       +3     
  Lines      395241    395378     +137     
  Branches    25207     25207              
===========================================
+ Hits       316581    318609    +2028     
+ Misses      78231     76340    -1891     
  Partials      429       429              

[malwilley]

Also wanted to call out that I'm adding a property to this dataclass and I know python will raise an exception when instantiating if it doesn't exist. Is it safe to do this? Want to make sure this won't start failing for old event data when this is merged

[saponifi3d]

🤔 it should be okay, but might want to set a default value to None to be safe.

afaik, we only apply this dataclass to the write on an issue occurrence, and when we're fetching for the API we just pass it through the serializer.

[malwilley]

Okay added a default value, but I agree I think it would probably be okay without one

[saponifi3d]

🤔 it should be okay, but might want to set a default value to None to be safe.

afaik, we only apply this dataclass to the write on an issue occurrence, and when we're fetching for the API we just pass it through the serializer.

[saponifi3d]

we should probably allow this to have multiple data sources rather than limit to the first one. could we just serialize the list of data sources even though it'll generally only be 1? it might also help with debugging to see if there are multiple sources connected or not.

[malwilley]

Okay makes sense - I thought that because this sent a source_id that meant that only one datasource should match, but if that's not the case the list makes sense. Updated

[saponifi3d]

woohoo! thanks for all those changes.