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.

Sign up for GitHub

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

Jump to bottom

subscriber: fix reload ergonomics #1035

Merged
merged 2 commits into from
Oct 13, 2020
Merged

subscriber: fix reload ergonomics #1035

merged 2 commits into from
Oct 13, 2020

Commits on Oct 13, 2020

  1. subscriber: fix reload ergonomics 

    Currently, the `reload` layer is generic over both the type of the layer
being reloaded, *and* the type of the subscriber that the layer is
layered onto. This means that the `reload::Handle` type that's used to
reload the value of the layer *also* is parameterized over the
subscriber's type.

The subscriber type parameter makes the `reload` API significantly
harder to use. Any time a `reload::Handle` is returned by a function,
taken as an argument, or stored in a struct, the full type of the
subscriber under the layer must be written out --- and often, it is
quite long. What makes this worse is that sometimes the type of the
subscriber may vary at runtime based on some configuration, while the
type of the layer that's reloaded remains the same. For example, in
Linkerd, we've had to do [this][1], which is really not ideal.

This branch removes the `Subscriber` type parameter from `reload::Layer`
and `reload::Handle`. Now, the `Handle` type is only generic over the
type of the inner layer that's being reloaded. It turns out that the
`Subscriber` type parameter was only necessary to add a `L: Layer<S>`
bound to `reload::Layer`'s constructor, which isn't really necessary ---
if the layer does not implement `Layer<S>`, the type error will occur
when `Subscriber::with` is actually used to layer it, which is fine.

I also changed the `with_filter_reloading` option on the `FmtSubscriber`
builder to also work with `LevelFilter`s, since there's no reason for it
not to, and added an example.

Since this breaks existing code, this change has to be made as part of
0.3.

[1]: https://github.com/linkerd/linkerd2-proxy/blob/6c484f6dcdeebda18b68c800b4494263bf98fcdc/linkerd/app/core/src/trace.rs#L19-L36
    @hawkw
    hawkw committed Oct 13, 2020
    Configuration menu
    Copy the full SHA
    8a87ea5 View commit details
    Browse the repository at this point in the history

  2. fix rustdoc links 

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    @hawkw
    hawkw committed Oct 13, 2020
    Configuration menu
    Copy the full SHA
    a8d564c View commit details
    Browse the repository at this point in the history