fix: deduplicate STACKS_EVENT_OBSERVER endpoint already registered in config file - #7204
Conversation
|
|
federico-stacks
left a comment
There was a problem hiding this comment.
@vwinee21 Thanks for your contribution, and apologies for the delayed review.
A couple of general items first:
- Please rebase the PR onto the
mainbranch. - Please add a changelog entry following the instructions in
changelog.d/README.md. You can also find examples from other PRs in that directory.
The fix is close, but I suggest one behavioral change: instead of discarding the environment-variable observer when its endpoint already exists, merge AnyEvent into the existing observer’s events_keys if it is not already present. This preserves the behavior requested through STACKS_EVENT_OBSERVER while still ensuring that the endpoint is registered only once and does not receive duplicate notifications. The existing TOML timeout and retry settings should be preserved.
Could you also add unit tests covering at least these cases?
- The same endpoint with narrower TOML subscriptions gains AnyEvent and remains a single observer.
- An endpoint already containing AnyEvent remains unchanged and is not duplicated.
- Different endpoints remain registered separately.
|
I merged latest into the PR, but @vwinee21 you still need to sign the CLA and respond to @federico-stacks's feedback. Thanks. |
Problem
Fixes #2810
When
STACKS_EVENT_OBSERVERenv var points to the same endpoint already registered in theevents_observerTOML block, stacks-node inserts twoEventObserverConfigentries into theHashSet— one with the events_keys from TOML, and one withAnyEventfrom the env var.Because
EventObserverConfigderivesHash + Eqacross all fields (endpoint, events_keys, timeout_ms, disable_retries), two configs with the same endpoint but differentevents_keysare considered distinct — both inserted, both receive broadcasts. The API server receives every event twice, causing it to fail over time.Fix
Before inserting the env var observer, check if any existing observer already has the same endpoint. If found, emit a
warn!log and skip the insert:Notes
warn!log is emitted so operators are aware of the duplicate configuration.