seize
seize copied to clipboard
impl Clone for Collector might be a footgun
I recently tracked down a bug in some code caused by my incorrect assumption about Collector::clone: I assumed it would behave like an Arc in maintaining a reference to the underlying data structure, which (I thought) is a common pattern in Rust crates dealing with concurrency. This, of course, was incorrect and led to objects being mixed between separate collectors.
I see now that the docs for Collector::clone mention this behavior, but rustdoc doesn't do a great job of showing docs for trait implementations. I wondered if it would be less likely to cause confusion if the cloning behavior was in a differently named method, e.g. clone_config.
This is just a suggestion. Maybe I'm the only one who would make this assumption about clone, not sure.
Yes, I completely agree. I was actually meaning to change this a while ago but it fell off my radar. I think we should probably just remove the Clone implementation, and add with_config(self, config: Config) and config(&self) -> Config methods that can be used to easily recreate collector configuration (where Config holds the current epoch_frequency and batch_size settings).
Removed in https://github.com/ibraheemdev/seize/commit/97ef5794916a1e5afdefdebaac5fac68e1244466.
Reopening this until the fix releases, it's not part of the latest release as it is a breaking change.