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.

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

backport recent changes from master #1447

Merged
merged 9 commits into from
Jun 26, 2021
Merged

backport recent changes from master #1447

merged 9 commits into from
Jun 26, 2021

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 jtescher and a team 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 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.

None yet

6 participants