This PR renames:
- `ServiceExt::ready_and` to `ServiceExt::ready`
- the `ReadyAnd` future to `Ready`
- the associated documentation to refer to `ServiceExt::ready`
and `ReadyAnd`.
This PR deprecates:
- the `ServiceExt::ready_and` method
- the `ReadyAnd` future
These can be removed in Tower 0.5.
My recollection of the original conversation surrounding the
introduction of the `ServiceExt::ready_and` combinator in
https://github.com/tower-rs/tower/pull/427 was that it was meant to be a
temporary workaround for the unchainable `ServiceExt::ready` combinator
until the next breaking release of the Tower crate. The unchainable
`ServiceExt::ready` combinator was removed, but `ServiceExt::ready_and`
was not renamed. I believe, but am not 100% sure, that this was an
oversight.
* builder: Add `ServiceBuilder::map_future`
I forgot at add this one in #542. Think its nice to have for
consistency.
* Update tower/src/builder/mod.rs
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Currently, when using `SpawnReady`, the current `tracing` span is not
propagated to the spawned task when the inner service is not ready. This
means that any traces emitted by the inner service's `poll_ready` occur
in their own root span, rather than the span the `SpawnReady` service
was polled in.
This branch fixes this by propagating the current trace span to the
spawned task.
This means that "spawn-ready" now enables the "tracing" feature. In the
future, we may want to consider feature-flagging `tracing` separately
from the middleware implementation that contain tracing instrumentation,
but doing so would break traces if the feature flag isn't enabled. This
doesn't break API compatibility, but it *does* break functionality, so
we may not want to do that until the next breaking change release.
I also added tests for span propagation. I realized the same test could
easily be applied to `Buffer`, which also propagates `tracing` spans, to
guard against regressions, so I also added a test for buffer.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
* make: Add `Shared`
Fixes https://github.com/tower-rs/tower/issues/262
`Shared` is a `MakeService` that produces services by cloning an inner
service.
* Fix build with different set of features
* Formatting
* Make `Shared` generic over any target
* Fix tests
* Move `Shared` into its own file
* Add example
* Add `util::option_layer` and `ServiceBuilder::option_layer`
Closes#553
* Apply suggestions to improve the docs
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
The [`Semaphore` implementation in `tokio::sync`][1] doesn't expose a
`poll_acquire` method to acquire owned semaphore permits in a
non-async-await context. Currently, the `limit::concurrency` and
`buffer` middleware use our own pollable wrapper for
`tokio::sync::Semaphore`. This works by creating a new
`Semaphore::acquire_owned` future and boxing it every time the semaphore
starts acquiring a new permit.
Recently, the `tokio_util` crate introduced its own [`PollSemaphore`
wrapper type][2]. This provides the same functionality of a pollable
version of `tokio::sync`'s `Semaphore`, just like our semaphore wrapper.
However, `tokio_util`'s version is significantly more efficient: rather
than allocating a new `Box` for _each_ `acquire_owned` future, it uses
the [`ReusableBoxFuture` type][3] to reuse a *single* allocation every
time a new `acquire_owned` future is needed. This means that rather than
allocating *every* time a `Buffer` or `ConcurrencyLimit` service starts
acquiring a new permit, there's a single allocation for each clone of
the service. Unless services are cloned per-request, this means that the
allocation is moved out of the request path in most cases.
I had originally considered an approach similar to this, but I didn't
think the reduced allocations were worth adding a bunch of unsafe code
in `tower` (which presently contains no unsafe code). However, this type
fits in perfectly in `tokio-util`, and now that there's an upstream
implementation, we should use it.
This introduces a dependency on `tokio-util` when the "limit" or "buffer"
features are enabled.
Additionally, I've added a new test for `Buffer` asserting that once an
individual `Buffer` service has been driven to readiness but not called,
additional `poll_ready` calls won't acquire additional buffer capacity.
This reproduces a bug that existed in earlier versions of
`tower::buffer`, which could result in starvation of buffer waiters.
This bug doesn't exist in 0.4, but I wanted to ensure that changing the
buffer internals here didn't introduce any new regressions.
[1]: https://docs.rs/tokio/1.2.0/tokio/sync/struct.Semaphore.html
[2]: https://docs.rs/tokio-util/0.6.3/tokio_util/sync/struct.PollSemaphore.html
[3]: https://docs.rs/tokio-util/0.6.3/src/tokio_util/sync/poll_semaphore.rs.html#13-16
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
* util: Add `ServiceExt::map_future`
I ran into a thing today where I wanted to write a middleware that wraps
all futures produced by a service in a `tracing::Span`. So something
like `self.inner.call(req).instrument(span)`.
Afaik all the combinators we have today receive the value produced by
the future and not the future itself. At that point its too late to call
`.instrument`. So I thought this made sense to add a combinator for.
* Add additional trait bounds to improve UX
* Add docs
* Update changelog
* Clean up debug impl
* Better debug impl for `MapFutureLayer`
This branch adds to the "Usage" section in the `tower` crate's README
and lib.rs docs. In particular, I've added a discussion of the various
use-cases for Tower, and a list of crates that support Tower.
Let me know if I'm missing anything!
* Add "full" feature that turns on all other features
Fixes https://github.com/tower-rs/tower/issues/530
* Don't include "log" feature in "full"
Makes it easier to disable "log" since its already included in the
default feature set.
* Fix changelog
This change modifies the `SpawnReady` service to track a
`tokio::task::JoinHandle` instead of creating a `tokio::sync::oneshot`
on each pending poll. This avoid unnecessary allocation.
The `spawn_ready` test is also fixed to avoid using `thread::sleep`,
instead using `tokio::time::pause`.
* util: add `FutureService::new`, with relaxed bounds
There are a couple issues with the current implementation of
`FutureService`.
The first, and less important, is a minor usability
issue: there's no `FutureService::new`, just a free function that
returns `FutureService`. While the free function is nice in some cases,
it means that if a user *is* naming the type someplace, they need to
import `tower::util::future_service` *and* `tower::util::FutureService`,
which is slightly annoying. Also, it just kind of violates the common
assumption that most publicly constructable types have a `new`,
requiring a look at the docs.
The second, more significant issue is that the `future_service` function
places a `Service` bound on the future's output type. While this is of
course necessary for the *`Service` impl* on `FutureService`, it's not
required to construct a `FutureService`. Of course, you generally don't
want to construct a `FutureService` that *won't* implement service.
However, the bound also means that additional generic parameters are now
required at the site where the `FutureService` is constructed. In
particular, the caller must now either know the request type, or be
generic over one.
In practice, when other middleware returns or constructs a
`FutureService`, this essentially means that it's necessary to add a
`PhantomData` for the request type parameter. This complicates code, and
perhaps more importantly, increases compile times, especially with
deeply-ensted middleware stacks.
As an example of the downside of aggressive bounds at the constructor,
it's worth noting that the implementation of `FutureService` currently
in `tower` is based directly on a similar implementation in
`linkerd2-proxy`. Except for the difference of whether or not the
constructor has a `Service` bound on the future's output, the two
implementations are very similar, almost identical. This gist shows some
of the change necessary to replace our otherwise identical
implementation with the `tower` version that bounds the `Service` type
at construction-time:
https://gist.github.com/hawkw/a6b07f9f4a8bce0c4b61036ed94114db
This PR solves these issues by adding a `FutureService::new` constructor
that does not introduce the `Service` bound. I didn't change the
`future_service` function: I don't *think* removing bounds is a breaking
change, but it is a modification to a publicly exposed function's type
signature, so I'm a little leery about it. Also, I thought that the more
aggressive bounding at construction-time might still be useful in
simpler use-cases where the `FutureService` is not part of a more
complex middleware stack, and that the free fn might be more likely to
be used in those cases anyway.
cc @davidpdrsn
* relax bounds on free fn
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
* Revert "relax bounds on free fn"
This reverts commit 5ee4fd36c3d1849acede223218a1d457306b9247. This
actually *is* breaking --- it would mean removing the `R` type parameter
for the request type on the function. This changes the function
definition, which might break uses of it.
This branch updates the READMEs for all Tower crates.
I've added the lib.rs docs to the `tower` crate's README, and added
crates.io, docs.rs, and updated CI badges to all the crates READMEs.
Since we no longer use Azure Pipelines for CI or Gitter for chat, I've
removed those badges and replaced them with GitHub Actions and Discord
badges, respectively.
I also fixed a typo in the `tower` lib.rs docs that was breaking some of the
RustDoc links, since I noticed it after copying those docs into the README.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
The `Layer` types for `AndThen`, `MapRequest`, and `MapErr` middleware
were missing `Clone` impls, which the `MapResponse`, `MapResult`, and
`Then` middleware's layer types *do* have. These layers should all be
`Clone` when the function is `Clone`.
I've added `derive(Clone)` to all these layer types.
This branch adds `AndThen::layer`, `Then::layer`, `MapErr::layer`,
`MapRequest::layer`, `MapResponse::layer`, and `MapResult::layer`
associated functions that simply return each middleware's associated
layer type. This can be more convenient in some cases, since it avoids
having to import both the layer type and the middleware type.
Similar functions already exist for other middleware, such as
`BoxService`.
Tower's docs builds are currently failing on `master` because of broken
docs links. These links are broken because they reference modules that
are feature flagged and disabled by default.
In order to build docs successfully, we should build with
`--all-features`. This also means we'll actually build docs for feature
flagged modules --- because we weren't doing that previously, we
actually weren't building most of the docs on CI.
Additionally, I've changed the docs build workflow to build on nightly and
to set `RUSTDOCFLAGS="--cfg docsrs"` so that we can use `doc(cfg)`
attributes when building the Git API docs.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch adds methods to access the inner service of a `Filter` or an
`AsyncFilter`. These are identical to the similarly-named method
provided by many other middleware services.
Along with the changes in #521, this is also necessary to implement
filtered versions of foreign traits in downstream code. I probably
should've added this in that PR, but I wasn't thinking it through...
## Motivation
In Linkerd 2, we have our own `RequestFilter` middleware. Now that Tower
0.4 has been released, including a new synchronous request filtering
middleware, we'd like to migrate to Tower's filter middleware. However,
there's one hitch: we implement additional traits for our filtering
middleware --- in particular, we use a `NewService` trait, which is
essentially a synchronous version of `MakeService`, for services that
can be constructed immediately. We'd like to be able to implement that
trait for `tower::filter::Filter` services as well.
## Solution
This branch adds new `Filter::check` and `AsyncFilter::check` methods
which expose the inner `Predicate::check` and `AsyncPredicate::check`
methods on the filter service's predicate. This can be used to call into
the predicate directly, allowing external traits to be implemented for
`Filter`/`AsyncFilter` in downstream code. Of course, the additional
traits must be defined in the crate providing implementaitons for
`tower::filter`, since `Filter` and `AsyncFilter` would be foreign
types, but in our use case, at least, this isn't an issue.
* tower: prepare to release 0.4.1
This branch updates the tower-layer dependency to 0.3.1 and prepares a
new release of `tower`. This should fix the broken re-exports of
`layer_fn` and get us a successful docs.rs build.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
