Skip to content

fix: Surface total SignalFx alert formatting failure - #6684

Open
mvanhorn wants to merge 1 commit into
keephq:mainfrom
mvanhorn:fix/6674-signalfx-total-format-failure
Open

fix: Surface total SignalFx alert formatting failure#6684
mvanhorn wants to merge 1 commit into
keephq:mainfrom
mvanhorn:fix/6674-signalfx-total-format-failure

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Closes #

📑 Description

Keep the existing per-incident isolation so one malformed incident does not discard valid alerts from the same response. Track whether formatting failures occurred and, after processing, raise a descriptive exception only when the upstream response was non-empty and no incident could be converted; preserve the original formatting failure as the cause where practical. Continue returning [] for a genuinely empty response and returning the successful subset for mixed valid/invalid responses.

SignalfxProvider._get_alerts catches and logs each incident-formatting exception, then returns the successfully formatted alerts. This preserves useful partial results, but when a non-empty API response contains only malformed incidents it returns [], making total data loss indistinguishable from a genuinely empty SignalFx response. Transport errors already propagate through raise_for_status, so the defect is limited to the per-incident formatting boundary. The base provider's get_alerts method already delegates to this method and will propagate its exception, so no caller or registry change is required.

Closes #6674

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • All the tests have passed

ℹ Additional Information

Nothing beyond what is described above.

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. Bug Something isn't working Provider Providers related issues labels Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.46%. Comparing base (ce302f6) to head (9af3294).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6684       +/-   ##
===========================================
- Coverage   46.42%   30.46%   -15.96%     
===========================================
  Files         178      101       -77     
  Lines       18694    11773     -6921     
===========================================
- Hits         8679     3587     -5092     
+ Misses      10015     8186     -1829     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Bug Something isn't working Provider Providers related issues size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SignalFx _get_alerts returns [] when every incident fails to format — VictoriaMetrics raises

1 participant