402 Commits

Author SHA1 Message Date
Eliza Weisman
a1d1282086 subscriber: fix missing doc(cfg(...)) attributes for EnvFilter (#1544)
## Motivation

Currently, some types in the `filter` module that require the
"env-filter" feature flag don't properly show in the docs that they
require this feature. This is because the `doc(cfg(feature = "env-filter"))`
attribute is only placed on the _re-export_ from the
`env` module (and on the `EnvFilter` type), not on the individual
types that are re-exported. However, placing a `doc(cfg(...))` attribute
on a re-export doesn't actually do anything.

## Solution

This commit fixes that.
2021-09-11 12:15:29 -07:00
Jeremy Fitzhardinge
6ac52155d5
subscriber: make Layer impls for Box<dyn Layer<..>> Send+Sync (#1547)
And for Arc

This is a fixup for https://github.com/tokio-rs/tracing/pull/1536.
2021-09-10 13:28:49 -07:00
Eliza Weisman
3bfddc6fbd
subscriber: impl Layer for FilterFn and DynFilterFn (#1546)
## Motivation

Currently, the `FilterFn` and `DynFilterFn` filters are only usable as
`Filter`s for per-layer filtering. However, there's no real reason they
can't _also_ have `Layer` implementations, making them usable as global
filters as well.

## Solution

This branch adds `Layer` implementations to `FilterFn` and
`DynFilterFn`, and moves them into their own file. It also changes the
feature flagging so that the "registry" feature is only required for the
`Filter` implementations --- the types themselves don't involve any
feature-flagged code, only the `Filter` trait.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-09-10 09:09:03 -07:00
Eliza Weisman
084d0971bb
subscriber: implement per-layer filtering (#1523)
## Motivation

Currently, filtering with `Layer`s is always _global_. If a `Layer`
performs filtering, it will disable a span or event for _all_ layers
that compose the current subscriber. In some cases, however, it is often
desirable for individual layers to see different spans or events than
the rest of the `Layer` stack.

Issues in other projects, such as tokio-rs/console#64 and
tokio-rs/console#76, linkerd/linkerd2-proxy#601,
influxdata/influxdb_iox#1012 and influxdata/influxdb_iox#1681,
jackwaudby/spaghetti#86, etc; as well as `tracing` feature requests like
#302, #597, and #1348, all indicate that there is significant demand for
the ability to add filters to individual `Layer`s.

Unfortunately, doing this nicely is somewhat complicated. Although a
naive implementation that simply skips events/spans in `Layer::on_event`
and `Layer::new_span` based on some filter is relatively simple, this
wouldn't really be an ideal solution, for a number of reasons. A proper
per-layer filtering implementation would satisfy the following
_desiderata_:

* If a per-layer filter disables a span, it shouldn't be present _for
  the layer that filter is attached to_ when iterating over span
  contexts (such as `Context::event_scope`, `SpanRef::scope`, etc), or
  when looking up a span's parents.
* When _all_ per-layer filters disable a span or event, it should be
  completely disabled, rather than simply skipped by those particular
  layers. This means that per-layer filters should participate in
  `enabled`, as well as being able to skip spans and events in
  `new_span` and `on_event`.
* Per-layer filters shouldn't interfere with non-filtered `Layer`s.
  If a subscriber contains layers without any filters, as well as
  layers with per-layer filters, the non-filtered `Layer`s should
  behave exactly as they would without any per-layer filters present.
* Similarly, per-layer filters shouldn't interfere with global
  filtering. If a `Layer` in a stack is performing global filtering
  (e.g. the current filtering behavior), per-layer filters should also
  be effected by the global filter.
* Per-layer filters _should_ be able to participate in `Interest`
  caching, _but only when doing so doesn't interfere with
  non-per-layer-filtered layers_.
* Per-layer filters should be _tree-shaped_. If a `Subscriber` consists
  of multiple layers that have been `Layered` together to form new
  `Layer`s, and some of the `Layered` layers have per-layer filters,
  those per-layer filters should effect all layers in that subtree.
  Similarly, if `Layer`s in a per-layer filtered subtree have their
  _own_ per-layer filters, those layers should be effected by the union
  of their own filters and any per-layer filters that wrap them at
  higher levels in the tree.

Meeting all these requirements means that implementing per-layer
filtering correctly is somewhat more complex than simply skipping
events and spans in a `Layer`'s `on_event` and `new_span` callbacks.

## Solution

This branch has a basic working implementation of per-layer filtering
for `tracing-subscriber` v0.2. It should be possible to add this in a
point release without a breaking change --- in particular, the new
per-layer filtering feature _doesn't_ interfere with the global
filtering behavior that layers currently have when not using the new
per-layer filtering APIs, so existing configurations should all behave
exactly as they do now.

### Implementation

The new per-layer filtering API consists of a `Filter` trait that
defines a filtering strategy and a `Filtered` type that combines a
`Filter` with a `Layer`. When `enabled` is called on a `Filtered` layer,
it checks the metadata against the `Filter` and stores whether that
filter enabled the span/event in a thread-local cell. If every per-layer
filter disables a span/event, and there are no non-per-layer-filtered
layers in use, the `Registry`'s `enabled` will return `false`.
Otherwise, when the span/event is recorded, each `Filtered` layer's
`on_event` and `new_span` method checks with the `Registry` to see
whether _it_ enabled or disabled that span/event, and skips passing it
to the wrapped layer if it was disabled. When a span is recorded, the
`Registry` which filters disabled it, so that they can skip it when
looking up spans in other callbacks.

 A new `on_layer` method was added to the `Layer` trait, which allows
 running a callback when adding a `Layer` to a `Subscriber`. This allows
 mutating both the `Layer` and the `Subscriber`, since they are both
 passed by value when layering a `Layer` onto a `Subscriber`, and a new
 `register_filter` method was added to `LookupSpan`, which returns a
 `FilterId` type. When a `Filtered` layer is added to a subscriber that
 implements `LookupSpan`, it calls `register_filter` in its `on_layer`
 hook to generate a unique ID for its filter. `FilterId` can be used to
 look up whether a span or event was enabled by the `Filtered` layer.

Internally, the filter results are stored in a simple bitset to reduce
the performance overhead of adding per-layer filtering as much as
possible. The `FilterId` type is actually a bitmask that's used to set
the bit corresponding to a `Filtered` layer in that bitmap when it
disables a span or event, and check whether the bit is set when querying
if a span or event was disabled. Setting a bit in the bitset and testing
whether its set is extremely efficient --- it's just a couple of bitwise
ops. Additionally, the "filter map" that tracks which filters disabled a
span or event is just a single `u64` --- we don't need to allocate a
costly `HashSet` or similar every time we want to filter an event. This
*does* mean that we are limited to 64 unique filters per
`Registry`...however, I would be _very_ surprised if anyone _ever_
builds a subscriber with 64 layers, let alone 64 separate per-layer
filters.

Additionally, the `Context` and `SpanRef` types now also potentially
carry `FilterId`s, so that they can filter span lookups and span
traversals for `Filtered` layers. When `Filtered` layers are nested
using the `Layered`, the `Context` _combines_ the filter ID of the inner
and outer layers so that spans can be skipped if they were disabled by
either.

Finally, an implementation of the `Filter` trait was added for
`LevelFilter`, and new `FilterFn` and `DynFilterFn` structs were added
to produce a `Filter` implementation from a function pointer or closure.

### Notes

Some other miscellaneous factors to consider when reviewing this change
include:

* I did a bit of unrelated refactoring in the `layer` module. Since we
  were adding more code to the `Layer` module (the `Filter` trait), the
  `layer.rs` file got significantly longer and was becoming somewhat
  hard to navigate. Therefore, I split out the `Context` and `Layered`
  types into their own files. Most of the code in those files is the
  same as it was before, although some of it was changed in order to
  support per-layer filtering. Apologies in advance to reviewers for
  this...
* In order to test this change, I added some new test-support code in
  `tracing-subscriber`. The `tracing` test support code provides a
  `Subscriber` implementation that can be configured to expect a
  particular set of spans and events, and assert that they occurred as
  expected. In order to test per-layer filtering, I added a new `Layer`
  implementation that does the same thing, but implements `Layer`
  rather than `Subscriber`. This allows testing per-layer filter
  behavior in the same way we test global filter behavior. Reviewers
  should feel free to just skim the test the test support changes, or
  skip reviewing them entirely.

## Future Work

There is a bunch of additional stuff we can do once this change lands.
In order to limit the size of this PR, I didn't make these changes yet,
but once this merges, we will want to consider some important follow
up changes:

* [ ] Currently, _only_ `tracing-subscriber`'s `Registry` is capable of
      serving as a root subscriber for per-layer filters. This is in
      order to avoid stabilizing a lot of the per-layer filtering
      design as public API without taking the time to ensure there are
      no serious issues. In the future, we will want to add new APIs to
      allow users to implement their own root `Subscriber`s that can
      host layers with per-layer filtering.
* [ ] The `EnvFilter` type doesn't implement the `Filter` trait and thus
      cannot currently be used as a per-layer filter. We should add an
      implementation of `Filter` for `EnvFilter`.
* [ ] The new `FilterFn` and `DynFilterFn` types could conceivably _also_
      be used as global filters. We could consider adding `Layer`
      implementations to allow them to be used for global filtering.
* [ ] We could consider adding new `Filter` implementations for performing
      common filtering predicates. For example, "only enable spans/events
      with a particular target or a set of targets", "only enable spans",
      "only enable events", etc. Then, we could add a combinator API to
      combine these predicates to build new filters.
* [ ] It would be nice if the `Interest` system could be used to cache
      the result of multiple individual per-layer filter evaluations, so
      that we don't need to always use `enabled` when there are several
      per-layer filters that disagree on whether or not they want a
      given span/event. There are a lot of unused bits in `Interest`
      that could potentially be used for this purpose. However, this
      would require new API changes in `tracing-core`, and is therefore
      out of scope for this PR.

Closes #302
Closes #597
Closes #1348

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-09-09 14:06:38 -07:00
Eliza Weisman
ac4a8dd27c chore: fix inconsistent terminology
I noticed a handful of places where `v0.1.x` refers to `Subscriber`s as
 "collectors". This probably happened because we backported some commits
 from master and forgot to change every instance of "collector" back to
 "subscriber".

 This commit fixes that. Whoops.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-09-04 11:57:42 -07:00
Max Burke
840f9b7d73 core: add support for visiting floating point values (#1507)
## Motivation

Tracing is a really useful framework but a lack of floating point value
support for the core visitors means these get coerced unnecessarily to
strings.

## Solution

This change adds support for floating point (`f64`) visitors.
2021-09-04 11:57:42 -07:00
Jeremy Fitzhardinge
f21028f84f
subscriber: add Layer implementations for various containers (#1536)
Add `Layer<S>` impls for:
- `Box<L>`
- `Box<dyn Layer<S>>`
- `Arc<L>`
- `Arc<dyn Layer<S>>`

where `L: Layer<S>`
2021-09-03 20:47:09 -07:00
Eliza Weisman
ca19cd2af1
subscriber: add feature flags to tests (#1532)
## Motivation

Currently we can't run

```bash
cargo test -p tracing-subscriber --no-default-features
```

successfully, because there are a bunch of tests for feature flagged
APIs that are always enabled.

## Solution

This commit adds feature flags to the modules and/or individual test
cases that test feature-flagged APIs.

There are still a few doctests that use feature-flagged APIs, and will
fail with `--no-default-features`. These are primarily the examples for
various `Layer::context` methods that rely on `LookupSpan`, and use the
`Registry` type, as it's the only subscriber that *implements*
`LookupSpan`. We could consider changing these examples, either by
removing the actual use of the layers in them, or by changing them to
use a mocked-out version of the registry. However, I think it's nicer to
show how the API would be used in real life. Perhaps we should just run

```bash
cargo test -p tracing-subscriber --no-default-features--tests --lib
```

to ignore doctests when testing without default features.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-09-02 10:43:34 -07:00
Eliza Weisman
06ecec499b
subscriber: nicer fmt::Debug for Layered (#1528)
Currently, `Layered` only implements `fmt::Debug` if the outer layer,
the inner layer, *and* the subscriber type parameters all implement
`fmt::Debug`. However, in the case of a `Layered` with two `Layer`s, the
subscriber itself is not present --- it's only a `PhantomData`. So the
`fmt::Debug` implementation should be possible when only the actual
`layer` and `inner` values are `Debug`. For the `PhantomData`, we can
just print the type name.

This means that `Layered` consisting of two `Layer`s and no `Subscriber`
will now implement `Debug` if the `Layer`s are `Debug`, regardless of
the subscriber type. When the `Layered` is a `Layer` and a `Subscriber`,
the behavior will be the same, because the separate `PhantomData`
subscriber type param is always the same as the type of the inner
subscriber.

Because printing the whole type name of the subscriber is potentially
verbose (e.g. when the subscriber itself is a big pile of layers), we
only include it in alt-mode.
2021-08-30 10:33:20 -07:00
Eliza Weisman
326d74f280
subscriber: fix Context's LookupSpan method feature flagging (#1525)
The `layer::Context` type has additional methods that require the inner
subscriber to implement `LookupSpan`. These currently are only present
when the `registry` feature flag is enabled. However, this isn't
actually necessary --- the `LookupSpan` trait always exists, and doesn't
require the `registry` feature flag; that feature flag only controls
whether the `Registry` _type_, a concrete implementation of
`LookupSpan`, is enabled.

This branch removes the `cfg` attributes from these methods.
2021-08-27 11:38:32 -07:00
Eliza Weisman
270de1cde0
docs: fix a bunch of RustDoc warnings/errors (#1524)
This branch fixes some minor RustDoc issues. In particular:

- The `broken_intra_doc_links` lint was renamed to
  `rustdoc::broken_intra_doc_links`. This generates a warning, since the
  old name was deprecated.
- `ignore` code blocks are specifically for _Rust_ code that should not
  be compiled, not for other text blocks. We were using `ignore` on JSON
  blocks, which generates a warning.
- A bunch of links in `tracing-subscriber`'s RustDocs were broken. This
  fixes that.

I also changed the Netlify configuration to run with `-D warnings`, so that
we can surface RustDoc warnings in CI builds.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-08-27 11:23:46 -07:00
Eliza Weisman
26506d5eaa
subscriber: prepare to release v0.2.20 (#1514)
# 0.2.20 (August 17, 2021)

### Fixed

- **fmt**: Fixed `fmt` printing only the first `source` for errors with
  a chain of sources ([#1460])
- **fmt**: Fixed missing space between level and event in the `Pretty`
  formatter ([#1498])
- **json**: Fixed `Json` formatter not honoring `without_time` and
  `with_level` configurations ([#1463])

### Added

- **registry**: Improved panic message when cloning a span whose ID
  doesn't exist, to aid in debugging issues with multiple subscribers
  ([#1483])
- **registry**: Improved documentation on span ID generation ([#1453])

Thanks to new contributors @joshtriplett and @lerouxrgd, and returning
contributor @teozkr, for contributing to this release!

[#1460]: https://github.com/tokio-rs/tracing/pull/1460
[#1483]: https://github.com/tokio-rs/tracing/pull/1483
[#1463]: https://github.com/tokio-rs/tracing/pull/1463
[#1453]: https://github.com/tokio-rs/tracing/pull/1453
[#1498]: https://github.com/tokio-rs/tracing/pull/1498
2021-08-17 16:10:41 -07:00
Eliza Weisman
4556cb3dc2 subscriber: add docs on registry span ID generation (#1453)
## Motivation

Currently, `tracing-subscriber`'s `Registry` type doesn't document how
span IDs are generated, or the uniqueness guarantees for span IDs. This
can cause confusion for users trying to implement `Subscribe`rs and
other code that interacts with the `Registry`'s generated span IDs.

## Solution

This branch adds a new documentation section to the `Registry` docs
describing how IDs are generated and when they are guaranteed to be
unique. In particular, the new section explicitly states that the
registry's IDs will not uniquely identify a span historically, because
IDs may be reused when a span has closed, and that the registry's
`tracing` span IDs should not be used as span IDs in a distributed
tracing system.

While I was working on these docs, I also fixed a link on adding
subscribers to a registry that went to a specific method on
`FmtSubscriber`, rather than to the more general docs on how
subscribers are composed with collectors.

Thanks to @Folyd for suggesting this docs improvement!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: David Barsky <me@davidbarsky.com>
2021-08-16 17:38:30 -07:00
Eliza Weisman
2b5bd7f402 chore: fix a giant pile of annoying clippy lints (#1486)
Clippy added a new lint detecting unnecessary borrows in expressions
where the borrowed thing is already a reference...which is nice, and
all, but the downside is that now we have to fix this.

This PR fixes it.
2021-08-16 17:38:30 -07:00
Romain Leroux
86ff5c09d3 subscriber: use display_timestamp and display_level in Json::format_event (#1463)
## Motivation

It should be possible to remove `timestamp` and `level` fields when
using json formatting.

As of now this has no effect:

```rs
let subscriber = tracing_subscriber::fmt()
    .with_level(false)
    .without_time()
    .json()
    .finish();
```

## Solution

Use the existing `display_timestamp` and `display_level` fields to
conditionally serialize `timestamp` and `level`.
2021-08-16 17:38:30 -07:00
Eliza Weisman
478156ae39
subscriber: add space after level with pretty formatter (#1498)
## Motivation

On v0.1.x,  `tracing_subscriber::fmt`'s `Pretty` formatter is missing
a space after the event's level, when printing levels is enabled.

## Solution

This branch fixes that.

Fixes #1488

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-08-07 13:55:10 -07:00
Teo Klestrup Röijezon
aeae4fab32
subscriber::fmt: print all error sources (#1460)
## Motivation

Fixes #1347  

## Solution

Change the format from `err=message, err: source1` to `err=message
err.sources=[source1, source2, source3, ...]`, as suggested in
https://github.com/tokio-rs/tracing/issues/1347#issuecomment-813674313
(still leaving out the sources if there are none). 

## Caveats

Haven't changed the JSON formatter, since I'm not really sure about how
to do that. The current format is `{.., "fields": {.., "err":
"message"}}`, disregarding the sources entirely. 

We could change that to `{.., "fields": {.., "err": "message",
"err.sources": ["source1", "source2", "source3", ..]}}`, which would
keep backwards compatibility but looks pretty ugly.

Another option would be `{.., "fields": {.., "err": {"message":
"message", "sources": ["source1", "source2", "source3", ..]}}}` which
leaves room for future expansion.

Then again, that begs the question about why the first error is special,
so maybe it ought to be `{.., "fields": {.., "err": {"message":
"message", "source": {"message": "source1", "source": ..}}}}`.

But that style of linked list is pretty annoying to parse, so maybe it
ought to be flattened to `{.., "fields": {.., "err": [{"message":
"message"}, {"message": "source1"}, ..]}}`?

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-08-06 10:06:03 -07:00
Teo Klestrup Röijezon
5f0f5cc97e
chore: fix warnings introduced by Rustc 1.54 (#1495)
Split from #1460
2021-08-06 09:36:27 -07:00
Josh Triplett
0af3ea6465
subscriber: expand the "tried to clone" panic with a hint about borrowing parents (#1483)
This may help give the user a hint to start debugging this.

See https://github.com/tokio-rs/tracing/issues/688 for further details
on this issue.
2021-08-01 07:22:06 -07:00
Eliza Weisman
807bc8fae7
subscriber: prepare to release 0.2.19 (#1448)
# 0.2.19 (June 25, 2021)

### Deprecated

- **registry**: `SpanRef::parents`, `SpanRef::from_root`, and
  `Context::scope` iterators, which are replaced by new `SpanRef::scope`
  and `Scope::from_root` iterators (#1413)

### Added

- **registry**: `SpanRef::scope` method, which returns a leaf-to-root
  `Iterator` including the leaf span (#1413)
- **registry**: `Scope::from_root` method, which reverses the `scope`
  iterator to iterate root-to-leaf (#1413)
- **registry**: `Context::event_span` method, which looks up the parent
  span of an event (#1434)
- **registry**: `Context::event_scope` method, returning a `Scope`
  iterator over the span scope of an event (#1434)
- **fmt**: `MakeWriter::make_writer_for` method, which allows returning
  a different writer based on a span or event's metadata (#1141)
- **fmt**: `MakeWriterExt` trait, with `with_max_level`,
  `with_min_level`, `with_filter`, `and`, and `or_else` combinators
  (#1274)
- **fmt**: `MakeWriter` implementation for `Arc<W> where &W: io::Write`
  (#1274)

Thanks to @teozkr and @Folyd for contributing to this release!
2021-06-25 17:31:10 -07:00
Eliza Weisman
4bf3d0000c subscriber: add MakeWriter combinators (#1274)
This backports #1274 from `master`.

Depends on #1141.

This branch adds a `MakeWriterExt` trait which adds a number of
combinators for composing multiple types implementing `MakeWriter`.
`MakeWriter`s can now be teed together, filtered with minimum and
maximum levels, filtered with a `Metadata` predicate, and combined with
a fallback for when a writer is _not_ made for a particular `Metadata`.

I've also added a `MakeWriter` impl for `Arc<W>` when `&W` implements
`Write`. This enables `File`s to be used as `MakeWriter`s similarly to
how we will be able to in 0.3, with the additional overhead of having to
do ref-counting because we can't return a reference from `MakeWriter`
until the next breaking change.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-06-25 17:01:47 -07:00
Folyd
ab6207621d subscriber: explain why we always call inner.register_callsite() before if statement (#1433)
This backports #1433 from `master`.

* subscriber: explain why we always call `inner.register_callsite()` before if statement

* Apply suggestions from code review

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-06-25 17:01:47 -07:00
Eliza Weisman
de76e552cb chore: fix new clippy warnings (#1444)
This fixes a handful of new clippy lints. Should fix CI.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-06-25 17:01:47 -07:00
Eliza Weisman
805eb51bc9 subscriber: add MakeWriter::make_writer_for (#1141)
This backports PR #1141 from `master`.

subscriber: add `MakeWriter::make_writer_for`

## Motivation

In some cases, it might be desirable to configure the writer used for
writing out trace data based on the metadata of the span or event being
written. For example, we might want to send different levels to
different outputs, write logs from different targets to separate files,
or wrap formatted output in ANSI color codes based on levels. Currently,
it's not possible for the `MakeWriter` trait to model this kind of
behavior --- it has one method, `make_writer`, which is completely
unaware of *where* the data being written came from.

In particular, this came up in PR #1137, when discussing a proposal that
writing to syslog could be implemented as a `MakeWriter` implementation
rather than as a `Subscribe` implementation, so that all the formatting
logic from `tracing_subscriber::fmt` could be reused. See [here][1] for
details.

## Solution

This branch adds a new `make_writer_for` method to `MakeWriter`, taking
a `Metadata`. Implementations can opt in to metadata-specific behavior
by implementing this method. The method has a default implementation
that just calls `self.make_writer()` and ignores the metadata, so it's
only necessary to implement this when per-metadata behavior is required.
This isn't a breaking change to existing implementations.

There are a couple downsides to this approach: it's possible for callers
to skip the metadata-specific behavior by calling `make_writer` rather
than `make_writer_for`, and the impls for closures can't easily provide
metadata-specific behavior.

Since the upcoming release is going to be a breaking change anyway, we
may want to just make the breaking change of having
`MakeWriter::make_writer` _always_ take a `Metadata`, which solves these
problems. However, that can't be backported to v0.1.x as easily. Additionally,
that would mean that functions like `io::stdout` no longer implement
`MakeWriter`; they would have to be wrapped in a wrapper type or closure
that ignores metadata.

[1]: https://github.com/tokio-rs/tracing/pull/1137#discussion_r542728604

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-06-25 17:01:47 -07:00
Teo Klestrup Röijezon
067d214a73
subscriber: add Context method for resolving an Event's SpanRef (#1434)
## Motivation

Fixes #1428

## Solution

Adds a new `Context::event_span` method as proposed in the issue.

No existing formatters were changed to use it yet, that seems like a
secondary issue (especially if they already work correctly).
2021-06-23 11:26:35 -07:00
Teo Klestrup Röijezon
7a012603ea
subscriber: unify span traversal (#1431)
## Motivation

Fixes #1429 

## Solution

Implemented as described in #1429, and migrated all internal uses of the
deprecated methods. All tests passed both before and after the migration
(9ec8130).

- Add a new method `SpanRef::scope(&self)` that returns a leaf-to-root
  `Iterator`, including the specified leaf
- Add a new method `Scope::from_root(self)` (where `Scope` is the type
  returned by `SpanRef::scope` defined earlier) that returns a
  root-to-leaf `Iterator` that ends at the current leaf (in other
  words: it's essentially the same as
  `Scope::collect::<Vec<_>>().into_iter().rev()`)
- Deprecate all existing iterators, since they can be replaced by the
  new unified mechanism:
  - `Span::parents` is equivalent to `Span::scope().skip(1)` (although
    the `skip` is typically a bug)
  - `Span::from_root` is equivalent to `Span::scope().skip(1).from_root()`
    (although the `skip` is typically a bug)
  - `Context::scope` is equivalent to
    `Context::lookup_current().scope().from_root()` (although the
    `lookup_current` is sometimes a bug, see also #1428)
2021-06-22 10:33:42 -07:00
David Barsky
f910a2a9b5
docs: Fix rendering issue with misaligned tooltips. (#1435) 2021-06-13 11:21:01 -04:00
Eliza Weisman
614a327adb chore: fix clippy's awful new "inconsistent struct constructor" lint (#1401)
This branch fixes some annoying new clippy lints no one cares about.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
# Conflicts:
#	tracing-subscriber/src/layer.rs
2021-05-17 13:35:53 -07:00
Eliza Weisman
dd49b99016
subscriber: prepare to release 0.2.18 (#1384)
# 0.2.18 (April 30, 2021)

### Deprecated

- Deprecated the `CurrentSpan` type, which is inefficient and largely
  superseded by the `registry` API ([#1321])

### Fixed

- **json**: Invalid JSON emitted for events in spans with no fields
  ([#1333])
- **json**: Missing span data for synthesized new span, exit, and close
  events ([#1334])
- **fmt**: Extra space before log lines when timestamps are disabled
  ([#1355])

### Added

- **env-filter**: Support for filters on spans whose names contain any
  characters other than `{` and `]` ([#1368])

Thanks to @Folyd, and new contributors @akinnane and @aym-v for
contributing to  this release!

[#1321]: https://github.com/tokio-rs/tracing/pull/1321
[#1333]: https://github.com/tokio-rs/tracing/pull/1333
[#1334]: https://github.com/tokio-rs/tracing/pull/1334
[#1355]: https://github.com/tokio-rs/tracing/pull/1355
[#1368]: https://github.com/tokio-rs/tracing/pull/1368
2021-05-01 09:25:15 -07:00
Aymerick Valette
1e8446eaa0 subscriber: support special chars in span names (#1368)
Filtering log events using the `target[span{field=value}]=level` syntax
doesn't work when the span name contains a special char.

This PR closes #1367 by adding support for span names with any
characters other than `{` and `]` to `EnvFilter`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-04-30 11:27:10 -07:00
Folyd
396d9f42a5 chore: fix cargo docs warnings (#1362)
This PR simply fixes some cargo docs warnings.
2021-04-30 11:27:10 -07:00
Folyd
5e435b1220
subscriber: deprecate CurrentSpan (#1321)
The `tracing-subscriber::CurrentSpan` should be deprecated in `v0.2.x`.

Related PR #1320. 

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-04-30 10:25:44 -07:00
Eliza Weisman
eea2bb8308 subscriber: remove space when timestamps are disabled (#1355)
## Motivation

Currently, the default `Compact` and `Full` formatters in
`tracing-subscriber` will prefix log lines with a single space when
timestamps are disabled. The space is emitted in order to pad the
timestamp from the rest of the log line, but it shouldn't be emitted
when timestamps are turned off. This should be fixed.

## Solution

This branch fixes the issue by skipping `time::write` entirely when
timestamps are disabled. This is done by tracking an additional boolean
flag for disabling timestamps.

Incidentally, this now means that span lifecycle timing can be enabled
even if event timestamps are disabled, like this:
```rust
use tracing_subscriber::fmt;
let subscriber = fmt::subscriber()
    .without_time()
    .with_timer(SystemTime::now)
    .with_span_events(fmt::FmtSpan::FULL);
```
or similar.

I also added a new test reproducing the issue, and did a little
refactoring to try and clean up the timestamp formatting code a bit.

Closes #1354
2021-04-17 14:48:07 -07:00
akinnane
07af5f8fd8 subscriber: fix span data for new, exit, and close events (#1334)
* subscriber: fix span data for new, exit, and close events

New, exit and close span events are generated while the current
context is set to either `None` or the parent span of the span the
event relates to. This causes spans data to be absent from the JSON
output in the case of the `None`, or causes the span data to reference
the parent's span data. Changing the way the current span is
determined allows the correct span to be identified for these
events. Trying to access the events `.parent()` allows access of the
correct span for the `on_event` actions, while using `.current_span()`
works for normal events.

Ref: #1032

* Fix style

* subscriber: improve test for #1333

Based on feedback by @hawkw, I've improved the test for #1333 to parse
the json output. This is more specifc for the bug and allows easier
testing of the different span `on_events`.

Ref: https://github.com/tokio-rs/tracing/pull/1333#pullrequestreview-623737828

* subscriber: improve #1334 tests covering all span states

Use the `on_records` test method check all events have the correct
context as described in the PR.

* Apply suggestions from code review

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-04-17 14:48:07 -07:00
akinnane
f2c182543b subscriber: fix on_event serialization when no fields set on span (#1333)
Serializing a spans `on_ACTION` events, when no fields are set on the
span, results in invalid JSON. This is because `serializier_map` was
getting a size hint for `self.0.metadata().fields().len()` then
serializing `self.0.fields.field_set()` instead. This resulted in the
fields key being set to an empty object, then Serde serializes the k/v
pairs from `field_set()`. This was causing an erroneous closing brace
`}` to be added after the serialized fields.

This change aligns the size hint with the actual serialized data.

Refs: https://github.com/tokio-rs/tracing/issues/829#issuecomment-661984255

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-03-31 15:17:39 -07:00
Eliza Weisman
18bd9f3ae3
chore: ignore flaky field filter tests (#1336)
This branch changes the `field_filter_events` and `field_filter_spans`
tests in `tracing-subscriber` to be ignored by default, as they often
fail spuriously on CI.

The tests can still be run using
```shell
$ RUSTFLAGS="--cfg flaky_tests" cargo test
```
or similar.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-03-31 12:49:44 -07:00
Eliza Weisman
84ff3a4f1c
chore: reverse the polarity of conversions, fix clippy (#1335)
This backports PR #1335 from `master` to `v0.1.x`.

Clippy now warns about implementing `Into` rather than `From`, since
`From` automatically provides `Into` but `Into` does not provide `From`.

This commit fixes the direction of those conversions, placating Clippy.

Additionally, it fixes a redundant use of slice syntax that Clippy also
complained about.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-03-29 16:35:50 -07:00
Eliza Weisman
88685417f6
subscriber: prepare to release 0.2.17 (#1302)
# 0.2.17 (March 12, 2020)

### Fixed

- **fmt**: `Pretty` formatter now honors `with_ansi(false)` to disable
  ANSI terminal formatting ([#1240])
- **fmt**: Fixed extra padding when using `Pretty` formatter ([#1275])
- **chrono**: Removed extra trailing space with `ChronoLocal` time
  formatter ([#1103])

### Added

- **fmt**: Added `FmtContext::current_span()` method, returning the
  current span
  ([#1290])
- **fmt**: `FmtSpan` variants may now be combined using the `|` operator
  for more granular control over what span events are generated
  ([#1277])

Thanks to new contributors @cratelyn, @dignati, and @zicklag, as well as
@Folyd, @matklad, and @najamelan, for contributing to this release!

[#1240]: https://github.com/tokio-rs/tracing/pull/1240
[#1275]: https://github.com/tokio-rs/tracing/pull/1275
[#1103]: https://github.com/tokio-rs/tracing/pull/1103
[#1290]: https://github.com/tokio-rs/tracing/pull/1290
[#1277]: https://github.com/tokio-rs/tracing/pull/1277
2021-03-12 14:16:41 -08:00
katelyn martin
ec7313466d subscriber: update pretty formatter for no ansi (#1240)
This backports #1240 from `master`.

* subscriber: update pretty formatter for no ansi

 ## Background

    Currently, when the `Pretty` event formatter is being used, it does
    not change its output when the `with_ansi` flag is set to false by
    the `CollectorBuilder`.

 ## Overview

    While this formatter is generally used in situations such as local
    development, where ANSI escape codes are more often acceptable,
    there are some situations in which this can lead to mangled output.

    This commit makes some minor changes to account for this `ansi` flag
    when formatting events using `Pretty`.

    Becuase ANSI codes were previously used to imply the event level
    using colors, this commit additionally modifies `Pretty` so that
    it respects `display_level` when formatting an event.

 ## Changes

    * Changes to `<Format<Pretty, T> as FormatEvent<C, N>>::format_event`

    * Add a `PrettyVisitor::ansi` boolean flag, used in its `Visit`
      implementation.

        * Add a new `PrettyVisitor::with_ansi` builder pattern method to
          facilitate this.

 ## Out of Scope

    One detail worth nothing is that this does not solve the problem of
    *fields* being formatted without ANSI codes. Configuring a
    subscriber using this snippet would still lead to bolded fields in
    parent spans.

```rust
tracing_subscriber::fmt()
    .pretty()
    .with_ansi(false)
    .with_level(false)
    .with_max_level(tracing::Level::TRACE)
    .init();
```

    This can be worked around by using a different field formatter, via
    `.fmt_fields(tracing_subscriber::fmt::format::DefaultFields::new())`
    in the short-term.

    In the long-term, #658 is worth investigating further.

Refs: #658
2021-03-12 11:21:58 -08:00
Zicklag
a947c5a63f subscriber: change FmtSpan to a combinable bitflag (#1277)
This backports #1277 from `master`.

Fixes #1136.

Allows arbitrarily combining different FmtSpan events to listen to.

Motivation
----------

The idea is to allow any combination of `FmtSpan` options, such as the
currently unachievable combination of `FmtSpan::NEW | FmtSpan::CLOSE`.

Solution
--------

Make `FmtSpan` behave like a bitflag that can be combined using the
bitwise or operator ( `|` ) while maintaining backward compatibility by
keeping the same names for all existing presets and keeping the
implementation details hidden.
2021-03-12 11:21:58 -08:00
Ole Schönburg
f3eb5ccf2d subscriber: fix extra padding in pretty format (#1275)
## Motivation

This fixes #1212, where extra padding was written when logging with the
pretty formatter.

## Solution 

With this change we only write padding when we actually decide to write
a value but not when skipping a metadata value such as `log.file` or
`log.line`
2021-03-12 11:21:58 -08:00
Folyd
544da6b5d8 subscriber: Add a public current_span() method for FmtContext (#1290)
This backports PR #1290 from `master`.
2021-03-12 11:21:58 -08:00
Aleksey Kladov
14d11ebdd6 subscriber: remove unnecessary transparent attribute (#1282)
As far as I can tell, we are not relying on transparency anywhere, so
using this rather refined feature here is confusing.
2021-03-12 11:21:58 -08:00
Folyd
51812267b8 subscriber: use struct update syntax when constructing from self (#1289)
This backports #1289 from `master`.

This PR aims to remove a lot of initializer boilerplate code by adopting
the`struct update syntax.  If the [RFC 2528]  gets merged and
implemented, we can remove more. 😸

[RFC 2528]: https://github.com/rust-lang/rfcs/pull/2528
2021-03-12 11:21:58 -08:00
Naja Melan
6f9b537c2e subscriber: Remove trailing space from ChronoLocal time formatter. (#1103)
I reckon this is a bug, and @samschlegel fixed this for ChronoUtc,
but not for ChronoLocal.
2021-03-12 10:52:23 -08:00
Eliza Weisman
4538d74d22
subscriber: prepare to release v0.2.16 (#1256)
### Fixed

- **env-filter**: Fixed directives where the level is in mixed case
  (such as `Info`) failing to parse ([#1126])
- **fmt**: Fixed `fmt::Subscriber` not providing a max-level hint
  ([#1251])
- `tracing-subscriber` no longer enables `tracing` and `tracing-core`'s
  default features ([#1144])

### Changed

- **chrono**: Updated `chrono` dependency to 0.4.16 ([#1189])
- **log**: Updated `tracing-log` dependency to 0.1.2

Thanks to @salewski, @taiki-e, @davidpdrsn and @markdingram for
contributing to this release!

[#1126]: https://github.com/tokio-rs/tracing/pull/1126
[#1251]: https://github.com/tokio-rs/tracing/pull/1251
[#1144]: https://github.com/tokio-rs/tracing/pull/1144
[#1189]: https://github.com/tokio-rs/tracing/pull/1189

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-02-19 16:24:42 -08:00
Eliza Weisman
0cdd5e88ac log: forward LogTracer::enabled to the subscriber (#1254)
PRs #1247, #1248, and #1251 improve `tracing`'s behavior for the `log`
crate's `log_enabled!` macro when a max-level hint is set. However, this
*doesn't* get us correct behavior when a particular target is not
enabled but the level is permitted by the max-level filter. In this
case, `log_enabled!` will still return `true` when using `LogTracer`,
because it doesn't currently call `Subscriber::enabled` on the current
subscriber in its `Log::enabled` method. Instead, `Subscriber::enabled`
is only checked when actually recording an event.

This means that when a target is disabled by a target-specific filter
but it's below the max level, `log::log_enabled!` will erroneously
return `true`. This also means that skipping disabled `log` records in
similar cases will construct the `log::Record` struct even when it isn't
necessary to do so.

This PR improves this behavior by adding a call to `Subscriber::enabled`
in `LogTracer`'s `Log::enabled` method. I've also added to the existing
tests for filtering `log` records to ensure that we also handle the
`log_enabled!` macro correctly.

While I was here, I noticed that the `Log::log` method for `LogTracer`
is somewhat inefficient --- it gets the current dispatcher *three*
times, and checks `enabled` twice. Currently, we check if the event
would be enabled, and then call the`format_trace` function, which *also*
checks if the event is enabled, and then dispatches it if it is. This is
not great. :/ I fixed this by moving the check-and-dispatch inside of a
single `dispatcher::get_default` block, and removing the duplicate
check.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-02-19 14:18:39 -08:00
Eliza Weisman
8d83326a5f subscriber: fix FmtCollector not forwarding max level (#1251)
The `FmtCollector` type is missing a `Collect::max_level_hint
method that forwards the inner stack's max level hint.

This fixes that, and adds tests.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-02-19 14:18:39 -08:00
Eliza Weisman
31aa6afecc subscriber: set the max log LevelFilter in init (#1248)
Depends on #1247.

Since `tracing-subscriber`'s `init` and `try_init` functions set the
global default subscriber, we can use the subscriber's max-level hint as
the max level for the log crate, as well. This should significantly
improve performance for skipping `log` records that fall below the
collector's max level, as they will not have to call the
`LogTracer::enabled` method.

This will prevent issues like bytecodealliance/wasmtime#2662
from occurring in the future. See also #1249.

In order to implement this, I also changed the `FmtSubscriber`'s
`try_init` to just use `util::SubscriberInitExt`'s `try_init` function,
so that the same code isn't duplicated in multiple places. I also
added `AsLog` and `AsTrace` conversions for `LevelFilter`s in
the `tracing-log` crate.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
2021-02-18 16:37:09 -08:00
Mark Ingram
220f8738ba chore(deps): update chrono dependency to 0.4.16 (#1173)
- this allows dropping the old time 0.1 dependency (see https://github.com/chronotope/chrono/blob/main/CHANGELOG.md#0416)

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
2021-01-29 09:42:44 -08:00