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

Tracking issue for composing multiple subscribers #135

Closed
hawkw opened this issue Jul 2, 2019 · 3 comments · Fixed by #420
Closed

Tracking issue for composing multiple subscribers #135

hawkw opened this issue Jul 2, 2019 · 3 comments · Fixed by #420
Assignees
@davidbarsky @hawkw
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/feature New feature or request

Comments

@hawkw
Copy link
Member

hawkw commented Jul 2, 2019 •
edited
Loading

Feature Request

Crates

tracing-subscriber

Motivation

In many cases, it is valuable to compose multiple consumers for trace events. However, certain aspects of tracing's design --- in particular, that span IDs are assigned by the subscriber, and that a single subscriber collects events from multiple threads --- make it difficult to compose the Subscriber trait directly.

Proposal

I think it should be possible to implement subscriber composition in the tracing-subscriber crate without making changes to tracing-core. Here's how I think we should do this:

  • Provide a Layer/Middleware-like approach for composing multiple consumers (which don't get to assign span IDs) around a single core subscriber that does assign IDs. I have a work in progress implementation of this, and will open a draft PR for discussion.
  • Provide a similar interface for composing filters, based on logical combinators, that slots into the Layer interface.
  • Provide an optimized concurrent structure for subscriber implementations to use for storing span data. I have some thoughts on this, which I described in subscriber: High performance span store #157. This will give subscriber layers an alternative to Mutex<HashMap<Id, MyPerSpanData>> and similar bad ideas.
  • Update existing subscriber implementations to be compatible with this API.
  • Investigate combinators like tees as well.

Alternatives

There are other potential ways to implement subscriber fan-out.

In the past, I've considered an approach where there is a central registry that manages per-span data and provides it to multiple consumers. I think this introduces a lot of complexity and is hard to come up with a design that satisfies the needs of various subscribers --- this is a big part of why tools for subscriber composition haven't landed sooner.

@davidbarsky proposed modifying the Dispatch type in tracing-core to take a slice of subscribers and iterate over them. I don't think this is the right approach for the following reasons:

  • It's a change in tracing-core, which I really don't think is necessary to implement this. IMO, if we can do it in a crate on top of -core, we should.
  • This approach does not solve the problem that all spans must have a single ID. If the dispatcher iterates over multiple implementations of the Subscriber trait, we would need a method for determining whose ID is the authoritative one. This could lead to very surprising behaviour when implementations expect their IDs to be used.
  • Similarly, it is difficult to reason about whether or not a span is enabled in such a world.

Finally, we could do nothing. Many implementations are already writing ad-hoc versions of the layered/stacked subscriber I described above (@LucioFranco's experiments w/ this was part of the inspiration behind the design I proposed). However there are several pitfalls/gotchas that ad-hoc implementations need to avoid, while if we provide a principled implementation in a library, it is only necessary to solve these problems once.
@hawkw hawkw added kind/feature New feature or request crate/subscriber Related to the `tracing-subscriber` crate labels Jul 2, 2019
@hawkw hawkw added this to the tracing-subscriber 0.1 milestone Jul 2, 2019
@LucioFranco
Copy link
Member

@hawkw this sounds really good. I think the layer approach will work well for decorating subscribers. I'm curious to see how composing filters might work. I think the most valuable thing we can provide right now is the data structure for tracking state. This is something ive rewritten three times. So I think by far the first step here is to produce that. Then continue to learn how wrapper subs work.

@hawkw hawkw mentioned this issue Jul 2, 2019
Merged
@hawkw
Copy link
Member Author

hawkw commented Jul 2, 2019

#136 is a draft proposal for a layer trait for subscriber composition.

@hawkw
Copy link
Member Author

hawkw commented Jul 9, 2019 •
edited
Loading

currently in progress:

hawkw added a commit that referenced this issue Jul 17, 2019
@hawkw
subscriber: Add a Layer type for composing subscribers (#136)
8ad3e8e 
* add `Layer` trait for composing subscribers

### Motivation

In many cases, it is valuable to compose multiple consumers for trace
events. However, certain aspects of `tracing`'s design --- in
particular, that span IDs are assigned by the subscriber, and that a
single subscriber collects events from multiple threads --- make it
difficult to compose the `Subscriber` trait directly. 

## Solution

This branch introduces a new trait, `Layer`, in `tracing-subscriber`.
`Layer` represents the composable subset of the `Subscriber`
functionality --- it recieves notifications of spans and events, and can
filter callsites, but it does not assign IDs or manage reference counts,
instead being notified on span closure by the underlying subscriber. A
`Layer` can thus be wrapped around another subscriber to add additional
functionality. In addition, this branch adds functionality for composing
layers, and a `filter` module that provides `Layer` implementations for
simple filters.

## Future Work

To have a complete story for multi-subscriber fanout, we will also want
to implement a performant concurrent span store, as I described in #157.
To better support the use of subscriber layers, may wish to consider
having a "registry" subscriber that implements _only_ span storage, ID
generation, and lifecycle management, for composing layers on top of.
Finally, we should consider adding `Layer` implementations to other
crates that currently only expose `Subscriber` impls.

Refs: #135

Signed-off-by: Eliza Weisman <[email protected]>

* subscriber: remove ref-counting from Layer (#149)

## Motivation

As I mentioned in #136 (comment), the
`Layer` trait proposed in #136 should probably _not_ be responsible for
managing ref-counts and determining span closures. Instead, the root
subscriber should be responsible for this, and `Layer`s should be able
to opt into a callback that's notified _when_ a span is closed. 

## Solution

Adding a callback that's called on closure to `Layer` required modifying
the `Subscriber` trait to allow indicating when a span closes. This
branch attempts to do this without a breaking change, by adding a new
`try_close` trait method to `Subscriber`. `try_close` is identical to
`drop_span` except that it returns a boolean value, set to `true` if the
span has closed. This function was added in #153. 

This branch updates `tracing-subscriber::Layer` to use `try_close`. 
Rather than having `clone_span`/`drop_span` callbacks on `Layer`, we
now have a `close` function, which is called when the inner subscriber's
`try_close` returns `true`.

Signed-off-by: Eliza Weisman <[email protected]>

* ensure a Context always refers to a Subscriber

This reworks layer composition a bit, and fixes an issue where
`Context`s were not actually guaranteed to wrap a `Subscriber`, and thus
implemented no methods. Layers now guarantee that `S` implements
`Subscriber`.

Signed-off-by: Eliza Weisman <[email protected]>
@jarrodldavis jarrodldavis mentioned this issue Aug 27, 2019
Closed
@hawkw hawkw removed this from the tracing-subscriber 0.1 milestone Sep 4, 2019
@hawkw hawkw mentioned this issue Nov 12, 2019
Merged
davidbarsky added a commit that referenced this issue Nov 14, 2019
@davidbarsky
subscriber: Introduce Registry; Refactor fmt::Subscriber in terms of …
919a628 
…Registry + Layers (#420)

This branch introduces: 

- A registry build atop of https://github.com/hawkw/sharded-slab. Layers
  are expected to consume this registry through the traits `SpanData`,
  `LookupSpan`, and `LookupMetadata`. Layer-specific data, such as
  formatted spans and events, are stored in the `Extensions` typemap. Data
  is writable via the `ExtensionsMut` view struct of the typemap. This
  enables layers to read and write data that they are specifically
  interested in.

- The `tracing_subscriber::fmt::Subscriber` has been re-implemented in
  terms of `tracing_subscriber::registry::Registry` and
  `tracing_subscriber::fmt::Layer`.

- The event/field formatters have been modified (in a non-backwards
  compatible way) to accept a `tracing_subscriber::fmt::FmtContext`. A
  similar structure existed in `tracing_subscriber::fmt::Subscriber`, but
  it was not publicly exposed.

Resolves #135 Resolves #157 Resolves #391

Signed-off-by: David Barsky <[email protected]>
Coauthored-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davidbarsky davidbarsky

@hawkw hawkw

Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/feature New feature or request
Projects
None yet
Milestone
No milestone
3 participants
@davidbarsky @hawkw @LucioFranco