Skip to content

Conversation

@hawkw
Copy link
Member

@hawkw hawkw commented Jun 25, 2021

This backports the following changes:

hawkw and others added 8 commits June 25, 2021 15:36
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]: #1137 (comment)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation

The RAII pattern documentation has been moved.

## Solution

Update the link to the document.
The opentelemetry specification calls for a number of attributes to correlate
traces to their location in the code. They are documented here [1]. This commit
adds support for the following fields based on the tracing span metadata (all
relative to span creation):

- `code.namespace`: Crate & module path (`my_crate::my_mod`)
- `code.filepath`: Relative path to the source code file (`src/my_mod.rs`)
- `code.lineno`: Line number in the file indicated by `code.filepath` (`72`)

As written this will annotate all spans with these attributes. If we want to be
a bit more conservative, I could add an instance variable to the Subscriber that
allows users to opt-in or opt-out of this functionality.

[1]: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#source-code-attributes

Co-authored-by: Lily Mara <lilymara@onesignal.com>
This backports PR #1441 from `master`

## Motivation

Newest versions of opentelemetry and opentelemetry-jaeger don't work
with the tracing-opentelemtry due to the latter still depending on a
0.14 version.

## Solution

Adjust dependencies and use new TraceFlags struct instead of constants
This fixes a handful of new clippy lints. Should fix CI.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
…fore 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>
#1327)

The `SpanBuilder` uses `Vec` to store span's fields. However, the
current solution can be slightly improved by preparing the capacity of
`Vec` in advance, this can reduce a few memory reallocations. 

Since the max number of tracing's span fields is 32, we can replace
`Vec` with `SmallVec` to further improve performance. Maybe, we should
add a new feature (such as `smallvec`?) to the `opentelemetry` crate.
Well, this should be discussed in the `opentelemetry` repo.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
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>
@hawkw hawkw requested review from a team and jtescher as code owners June 25, 2021 23:23
This adds a section to the `Level` docs explaining the comparison rules
for `Level` and `LevelFilter`. It turns out this wasn't really
explicitly documented anywhere.

I may have gone a bit overboard on this, but I think the new docs should
be helpful...

See #1274 (comment)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: David Barsky <me@davidbarsky.com>
@hawkw hawkw force-pushed the eliza/a-lot-of-backports branch from 82d576f to 22f53bf Compare June 25, 2021 23:35
@hawkw hawkw merged commit 204077f into v0.1.x Jun 26, 2021
@hawkw hawkw deleted the eliza/a-lot-of-backports branch June 26, 2021 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants