Skip to content

refactor outbound sender to allow for multiple modes of dispatch#614

Draft
piccione99 wants to merge 9 commits into
mainfrom
stream
Draft

refactor outbound sender to allow for multiple modes of dispatch#614
piccione99 wants to merge 9 commits into
mainfrom
stream

Conversation

@piccione99

@piccione99 piccione99 commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

also added kinesis and batch packages, but they aren't used yet, please ignore them for reviewing.

OutboundSender contains the common functionality for WebhookOutboundSender and eventually StreamOutboundSender. t

However, OutboundSender has the Update function that takes ancla.Webhook so it doesn't really fit that well with StreamOutboundSender. So, that is a code smell that probably needs to be addressed. That said, we might want to actually update stream senders from Argus so we don't have to reboot caduceus. Initially this will be a predefined set of streams and we need to manually add them to headwaters, so it can't be entirely automated. But would be nice to not have to redeploy caduceus.

@codecov

codecov Bot commented Sep 5, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.01180% with 244 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.73%. Comparing base (d496c94) to head (47cbc87).
⚠️ Report is 108 commits behind head on main.

Files with missing lines Patch % Lines
internal/kinesis/kinesis.go 41.74% 114 Missing and 6 partials ⚠️
webhook_dispatcher.go 72.62% 40 Missing and 9 partials ⚠️
webhook_outbound_sender.go 72.27% 22 Missing and 6 partials ⚠️
main.go 0.00% 21 Missing ⚠️
stream_dispatcher.go 80.30% 10 Missing and 3 partials ⚠️
internal/stream/sender.go 67.85% 6 Missing and 3 partials ⚠️
stream_outbound_sender.go 89.28% 2 Missing and 1 partial ⚠️
outboundSender.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #614      +/-   ##
==========================================
+ Coverage   53.89%   55.73%   +1.83%     
==========================================
  Files           9       16       +7     
  Lines        1334     1830     +496     
==========================================
+ Hits          719     1020     +301     
- Misses        574      749     +175     
- Partials       41       61      +20     
Flag Coverage Δ
unittests 55.73% <64.01%> (+1.83%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@piccione99
piccione99 requested a review from schmidtw September 5, 2025 13:42
@piccione99
piccione99 marked this pull request as draft September 5, 2025 13:43
Comment thread dispatcher.go Outdated
Send(urls *ring.Ring, secret, acceptType string, msg *wrp.Message)
}

func DispatcherFactory(webhook bool, obs *CaduceusOutboundSender) Dispatcher {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is removed

Comment thread outboundSender_test.go Outdated
}

obs.(*CaduceusOutboundSender).queueOverflow()
obs.(*WebhookOutboundSender).obs.dispatcher.QueueOverflow()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this will eventually also return StreamOutboundSender

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants