While we're making breaking changes to `tokio-trace`, it would be good
to get rid of the `AsId` trait. The goal of span functions that are
generic over `Span`/`Id` can be achieved without the unnecessary
complexity of defining a new trait. This would also make the API added
to `tokio_trace_core::Event` in #1109 more consistent with the
`tokio-trace::Span` API.
This branch removes `AsId` from `tokio-trace` and replaces its uses with
`impl Into<Option<Id>>` and `impl Into<Option<&'a Id>>`. While `AsRef`
might be more semantically correct for the borrowed-`Id` conversion, its
signature doesn't permit conversion into an `Option`. Implementations of
`Into<Option<Id>>` and `Into<Option<&'a Id>>` have been added for
`tokio_trace::Span`.
This is _technically_ a breaking API change, as it changes function
signatures. However, the existing macro syntax still works as-is, and
the tests which pass `&Id`, `&Span`, and `&Option<Id>` to the span
macros all still compile after this change.
Closes#1143
Signed-off-by: Eliza Weisman <[email protected]>
The runtime is inherently multi-threaded, so it's going to have to deal
with synchronization when submitting new tasks anyway. This allows a
runtime to be shared by multiple threads more easily when e.g. building
a blocking facade over a tokio-based API.
## Motivation
A common pattern in `tokio-trace` is to use the value of a local
variable as a field on a span or event. Currently, this requires code
like:
```rust
info!(foo = foo);
```
which is not particularly ergonomic given how commonly this occurs.
Struct initializers support a shorthand syntax for fields where the name
of the field is the same as a local variable, and `tokio-trace` should
as well.
## Solution
This branch adds support for syntax like
```rust
let foo = ...;
info!(foo);
```
and
```rust
let foo = Foo {
bar: ...,
...
};
info!(foo.bar)
```
to the `tokio-trace` span and event macros. This syntax also works with
the `Debug` and `Display` field shorthand.
The span macros previously used a field name with no value to indicate
an uninitialized field. A new issue, #1138, has been opened for finding a
replacement syntax for uninitialized fields. Until then, the `tokio-trace`
macros will no longer provide a way to create fields without values,
although the `-core` API will continue to support this.
Closes#1062
Signed-off-by: Eliza Weisman <[email protected]>
## Motivation
Currently, the primary way to use a span is to use `.enter` and pass a
closure to be executed under the span. While that is convenient in many
settings, it also comes with two decently inconvenient drawbacks:
- It breaks control flow statements like `return`, `?`, `break`, and
`continue`
- It require re-indenting a potentially large chunk of code if you wish
it to appear under a span
## Solution
This branch changes the `Span::enter` function to return a scope guard
that exits the span when dropped, as in:
```rust
let guard = span.enter();
// code here is within the span
drop(guard);
// code here is no longer within the span
```
The method previously called `enter`, which takes a closure and
executes it in the span's context, is now called `Span::in_scope`, and
was reimplemented on top of the new `enter` method.
This is a breaking change to `tokio-trace` that will be part of the
upcoming 0.2 release.
Closes#1075
Signed-off-by: Eliza Weisman <[email protected]>
This updates tests to track a fix applied in Mio. Previously, Mio
incorrectly fired HUP events. This was due to Mio mapping `RDHUP` to
HUP. The test is updated to correctly generate a HUP event.
Additionally, HUP events will be removed from all platforms except for
Linux. This is caused by the inability to reliably map kqueue events to
the epoll HUP behavior.
## Motivation
In `tokio-trace`, field values may be recorded as either a subset of
Rust primitive types or as `fmt::Display` and `fmt::Debug`
implementations. Currently, `tokio-trace` provides the `field::display`
and `field::debug` functions which wrap a type with a type that
implements `Value` using the wrapped type's `fmt::Display` or
`fmt::Debug` implementation. However, importing and using these
functions adds unnecessary boilerplate.
In #1081, @jonhoo suggested adding shorthand syntax to the macros,
similar to that used by the `slog` crate, as a solution for the
wordiness of the current API.
## Solution
This branch adds `?` and `%` sigils to field values in the span and
event macros, which expand to the `field::debug` and `field::display`
wrappers, respectively. The shorthand sigils may be used in any position
where the macros take a field value.
For example:
```rust
trace_span!("foo", my_field = ?something, ...); // shorthand for `debug`
info!(foo = %value, bar = false, ...) // shorthand for `display`
```
Adding this shorthand required a fairly large change to how field
key-value pairs are handled by the macros --- since `%foo` and `%foo`
are not valid Rust expressions, we can no longer match repeated
`$ident = $expr` patterns, and must now match field lists as repeated
token trees. The inner helper macros for constructing `FieldSet`s and
`ValueSet`s have to parse the token trees recursively. This added a
decent chunk of complexity, but fortunately we have a large number of
compile tests for the macros and I'm quite confident that all existing
invocations will still work.
Closes#1081
Signed-off-by: Eliza Weisman <[email protected]>
Callers may not always have `futures` available at the root of the
crate. Re-exporting dependencies makes them available to the macro at a
deterministic location.
The signal-hook library got split into lower-level and higher-level
parts. The tokio-signal uses only API from the lower-level one, so it
can depend on it directly.
The only effect of this change is smaller amount of compiled (and
unused) code during compilation. There's no change in the code actually
used.
This PR introduces `Lock`: A concurrency primitive built on top of `Semaphore` that provides a `Mutex`-like primitive that interacts nicely with futures. Specifically, `LockGuard` (in contrast to `MutexGuard`) does _not_ borrow the `Lock`, and can thus be passed into a future where it will later be unlocked.
This replaces #958, which attempted to introduce a less generic version. The primitive proposed there will instead live in [`async-lease`](https://github.com/jonhoo/async-lease).
This branch changes `dispatcher::get_default` to unset the thread's
current dispatcher while the reference to it is held by the closure.
This prevents infinite loops if the subscriber calls code paths which
emit events or construct spans.
Note that this also means that nested calls to `get_default` inside of a
`get_default` closure will receive a `None` dispatcher rather than the
"actual" dispatcher. However, it was necessary to unset the default in
`get_default` rather than in dispatch methods such as `Dispatch::enter`,
as when those functions are called, the current state has already been
borrowed.
Before:
```
test enter_span ... bench: 3 ns/iter (+/- 0)
test span_no_fields ... bench: 51 ns/iter (+/- 12)
test span_repeatedly ... bench: 5,073 ns/iter (+/- 1,528)
test span_with_fields ... bench: 56 ns/iter (+/- 49)
test span_with_fields_record ... bench: 363 ns/iter (+/- 61)
```
After:
```
test enter_span ... bench: 3 ns/iter (+/- 0)
test span_no_fields ... bench: 35 ns/iter (+/- 12)
test span_repeatedly ... bench: 4,165 ns/iter (+/- 298)
test span_with_fields ... bench: 48 ns/iter (+/- 12)
test span_with_fields_record ... bench: 363 ns/iter (+/- 91)
```
Closes#1032
Signed-off-by: Eliza Weisman <[email protected]>
* Fix crate path in `Cargo.toml` of examples
* Add `edition2018` to examples in the documentation to make it compiled
on Rust 2018
* Fix an example in the documentation
## Motivation
To ease the implementation of `Subscriber::register_callsite`, a field
should be added to `Metadata` to indicate if this callsite is an event or
a span.
## Solution
A new struct, `Kind`, is added to the `metadata` module in
`tokio-trace-core`, and a `Kind` field is added to the `Metadata`
struct. Macros which construct `metadata` now require a `Kind`.
`Kind` is represented as a struct with a private inner enum to allow new
`Kind`s to be added without breaking changes. However, the _addition_ of
the kind field _is_ a breaking change. While this could be done in a
backward-compatible way, it would permit the construction of metadata
with unknown kinds, and since the next `tokio-trace-core` release will
be a breaking change, I opted to make the breaking change instead.
New API tests for the `callsite!` and `metadata!` macros have been added
to guard against future API breakage.
Fixes: #986Closes: #1008
Co-Authored-By: csmoe <[email protected]>
## Motivation
Currently, `tokio-trace-core` permits `Subscriber`s to indicate that
they are "always", "sometimes", or "never" interested in a particular
callsite. When "always" or "never" is returned, then the interest is
cached and the subscriber will not be asked again about that callsite.
This is much more efficient than requiring the filter to be re-evaluated
every time the callsite is hit.
However, if a subscriber wishes to change its filter configuration
dynamically at runtime, it cannot benefit from this caching. Instead, it
must always return `Interest::sometimes`. Even when filters change very
infrequently, they must still always be re-evaluated every time.
In order to support a use-case where subscribers may change their filter
configuration at runtime (e.g. tokio-rs/tokio-trace-nursery#42),
but do so infrequently, we should introducing a new function to
invalidate the cached interest.
## Solution
This branch adds a new function in the `callsite` module, called
`rebuild_interest_cache`, that will invalidate and rebuild all cached
interest.
## Breaking Change
In order to fix a race condition that could occur when rebuilding
interest caches using `clear_interest` and `add_interest`, these methods
have been replaced by a new `set_interest` method. `set_interest` should
have the semantics of atomically replacing the previous cached interest,
so that the callsite does not enter a temporary state where it has no
interest.
Closes#1038
Co-Authored-By: yaahallo <[email protected]>
## Motivation
The `Span::enter` function previously required an `&mut` reference to
enter a span. This is a relic of an earlier design where span closure
logic was determined by dropping an inner span component, and is no
longer strictly necessary.
Requiring `&mut self` to enter a span leads to awkward patterns in cases
when a user wishes to enter a span and then call methods on the span
(such as recording field values). For example, we cannot say
```rust
let mut span = span!("foo", bar);
span.enter(|| {
span.record("bar" &false);
});
```
since the span is mutably borrowed by `enter`. Instead, we must clone
the span, like so:
```rust
let mut span = span!("foo", bar);
span.clone().enter(|| {
span.record("bar" &false);
});
```
Having to clone the span is somewhat less ergonomic, and it has
performance disadvantages as well: cloning a `Span` will clone the
span's `Dispatch` handle, requiring an `Arc` bump, as well as calling
the `Subscriber`'s `clone_span` and `drop_span` functions. If we can
enter spans without a mutable borrow, we don't have to update any of
these ref counts.
The other reason we may wish to require mutable borrows to enter a span
is if we want to disallow entering a span multiple times before exiting
it. However, it is trivially possible to re-enter a span on the same
thread regardless, by cloning the span and entering it twice. Besides,
there may be a valuable semantic meaning in entering a span from inside
itself, such as when a function is called recursively, so disallowing
this is not a goal.
## Solution
This branch rewrites the `Span::enter`, `Span::record`, and
`Span::record_all` functions to no longer require mutable borrows.
In the case of `record` and `record_all`, this was trivial, as borrowing
mutably was not actually *necessary* for those functions. For `enter`,
the `Entered` guard type was reworked to consist of an `&'a Inner`
rather than an `Inner`, so it is no longer necessary to `take` the
span's `Inner`.
## Notes
In addition to allowing spans to be entered without mutable borrows,
`Entered` was changed to exit the span automatically when the guard is
dropped, so we may now observe correct span exits even when unwinding.
Furthermore, this allows us to simplify the `enter` function a bit,
leading to a minor performance improvement when entering spans.
Before:
```
test enter_span ... bench: 13 ns/iter (+/- 1)
```
...and after:
```
test enter_span ... bench: 3 ns/iter (+/- 1)
```
Note that this branch also contains a change to make the
`subscriber::enter_span` benchmark more accurate. Previously, this
benchmark constructed a new span inside of `b.iter(|| {...})`. This
means that the benchmark was measuring not only the time taken to enter
a span, but the time taken to construct a `Span` handle as well.
However, we already have benchmarks for span construction, and the
intention of this particular benchmark was to measure the overhead of
constructing a span.
I've updated the benchmark by moving the span construction out of the
`iter` closure. Now, the span is constructed a single time and entered
on every iteration. This allows us to measure only the overhead of
actually entering a span. The "before" benchmark numbers above were
recorded after backporting this change to master, so they are "fair" to
the previous implementation. Prior to this change the benchmark took
approximately 53 ns.
Signed-off-by: Eliza Weisman <[email protected]>
## Motivation
In order to support conventions that add namespacing to `tokio-trace`
field names, it's necessary to accept at least one type of separator
character. Currently, the `tokio-trace` macros only accept valid Rust
identifiers, so there is no clear separator character for namespaced
conventions. See also #1018.
## Solution
This branch changes the single `ident` fragment matcher for field names
to match *one or more* `ident` fragments separated by `.` characters.
## Notes
The resulting key is still exposed to `tokio-trace-core` as a string
constant created by stringifying the dotted expression. However, if
`tokio-trace-core` were later to adopt a first class notion of
hierarchical field keys, we would be able to track that change in
`tokio-trace` as an implementation detail.
Closes#1018.
Closes#1022.
Signed-off-by: Eliza Weisman <[email protected]>
This branch fixes the `tokio-trace` Subscriber benchmarks panicking due
to constructing spans with ID 0. They will now use an arbitrary constant
instead.
Signed-off-by: Eliza Weisman <[email protected]>
Igor Gnatenko