Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: change some layers to require recorders that are Sync #538

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

tobz
Copy link
Member

@tobz tobz commented Oct 30, 2024

Context

Mentioned in #533, our recent change to require the global recorder to be Sync had a downstream effect for users taking advantage of certain layers like Fanout and Router. This is due to the fact that those layers only require taking something that is Recorder, which then is held as a Box<dyn Recorder>.

Solution

This PR updates Fanout and Router (and by extension, their builders, FanoutBuilder and RouterBuilder) to take recorders that are Sync, and then holds them as Box<dyn Recorder + Sync>. This facilitates being able to once again use these layers as part of the global recorder.

The original intent, before the change to require Sync in the global recorder, was always to have layers be usable for the global recorder, so this change is simply aligning these utilities to match.

While there are areas where a Sync recorder is not required, such as metrics::with_local_recorder or metrics::set_default_local_recorder... they're much more rare and generally scoped to testing, where something like DebuggingRecorder is being used and there would be no reason to use a layer. If somehow it turns out that there's a need for non-Sync layers for these use cases, we can address this by creating Unsync variants in the future.

@tobz tobz added C-util Component: utility classes and helpers. E-simple Effort: simple. T-ergonomics Type: ergonomics. labels Oct 30, 2024
@tobz tobz merged commit d97f801 into main Oct 30, 2024
13 checks passed
@tobz tobz added the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label Nov 2, 2024
@tobz
Copy link
Member Author

tobz commented Jan 6, 2025

Released as metrics-util@v0.19.0.

@tobz tobz deleted the tobz/sync-contraint-boxed-recorders branch January 6, 2025 15:17
@tobz tobz removed the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-util Component: utility classes and helpers. E-simple Effort: simple. T-ergonomics Type: ergonomics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant