fix(metrics): prevent utilization metric loss on configuration reload #24080
Conversation
On configuration reload, Vector starts spamming debug logs: ``` vector::utilization: Couldn't send utilization start wait message. ``` This is caused by building the config diff, which just creates a new utilization emitter, registers all the changed components to that new emitter and then just drops it at the end, because one already exists in the running topology. This changes it so that the existing emitter is reused on reloads, ensuring that reloaded components still send their utilization messages. Related: vectordotdev#24060 (comment)
github-actions
bot
added
the
domain: topology
esensar
changed the title
| impl Clone for UtilizationEmitter { | ||
| /// Cloning for UtilizationEmitter skipps the timer received. Only one instance is expected to | ||
| /// hold it and that instance should be running the `run_utilization` task. | ||
| fn clone(&self) -> Self { | ||
| Self { | ||
| timers: self.timers.clone(), | ||
| timer_rx: None, | ||
| timer_tx: self.timer_tx.clone(), | ||
| } |
There was a problem hiding this comment.
I have opted for Clone implementation which skips timer_rx for the cloned version.
I have also considered an approach with 2 separate components - the emitter, which would run the task and "registry" which would just be used to add components. While the second approach sounds better to me, it would require more significant changes to the codebase, but without any further benefit (I would still need to have a lock on timers map, since that part would be shared).
pront
added
the
domain: reload
|
Hi @esensar, I will prioritize reviewing this. FYI, I will push a small PR to convert the builder to actually use a builder pattern and will add a few related unit tests (you are welcome to add more). Just trying to clean up and future proof this part of the codebase. |
2 participants
Summary
On configuration reload, Vector starts spamming debug logs:
This is caused by building the config diff, which just creates a new utilization emitter, registers all the changed components to that new emitter and then just drops it at the end, because one already exists in the running topology. This changes it so that the existing emitter is reused on reloads, ensuring that reloaded components still send their utilization messages.
How did you test this PR?
Ran vector with any configuration, made changes to the configuration and forced a reload (using SIGHUP, but config watching would work too). Before this fix the debug spam was visible for changed components, indicating that they didn't have receiver to send utilization messages to. After this fix, the spam was gone.
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details here.