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

subscriber: prepare to release 0.2.1 #586

Merged
merged 1 commit into from
Feb 14, 2020
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Feb 14, 2020

Changed

Fixed

A big thank-you to @samschlegel for lots of help with EnvFilter
performance tuning in this release!

Signed-off-by: Eliza Weisman eliza@buoyant.io

@hawkw hawkw added crate/subscriber Related to the `tracing-subscriber` crate kind/maintenance labels Feb 14, 2020
@hawkw hawkw requested a review from a team February 14, 2020 01:03
@hawkw hawkw self-assigned this Feb 14, 2020
@hawkw
Copy link
Member Author

hawkw commented Feb 14, 2020

the repeated CI failures all seem to be due to network errors downloading crates:

 warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/rayon/1.3.0/download`, got 503
##[warning]warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/futures-util/0.3.4/download`, got 503
warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/rayon/1.3.0/download`, got 503
##[warning]  Downloaded itertools v0.8.2
warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/futures-util/0.3.4/download`, got 503
##[warning]  Downloaded num-traits v0.2.11
  Downloaded byteorder v1.3.4
error: failed to download from `https://crates.io/api/v1/crates/rayon/1.3.0/download`
##[error]failed to download from `https://crates.io/api/v1/crates/rayon/1.3.0/download`
Caused by:
  failed to get 200 response from `https://crates.io/api/v1/crates/rayon/1.3.0/download`, got 503

and similar (it's not always the same crate). I wonder if crates.io is experiencing service degradation?

Changed

- **filter**: `EnvFilter` directive selection now behaves correctly
  (i.e. like `env_logger`) (#583)

Fixed

- **filter**: Fixed `EnvFilter` incorrectly allowing less-specific
  filter directives to enable events that are disabled by more-specific
  filters (#583)
- **filter**: Multiple significant `EnvFilter`  performance
  improvements, especially when filtering events generated by `log`
  records (#578, #583)
- **filter**: Replaced `BTreeMap` with `Vec` in `DirectiveSet`,
  improving iteration performance significantly with typical numbers of
  filter directives (#580)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw merged commit 4dad420 into master Feb 14, 2020
hawkw added a commit to linkerd/linkerd2-proxy that referenced this pull request Feb 14, 2020
This release makes a number of significant performance improvements when
filtering events emitted through the `log` compatibility layer (such as
from `hyper` and `h2`). See tokio-rs/tracing#586 for details.

Since these performance improvements apply to cases where a filter
_disables_ an event, as well as to enabling events, this should improve
the proxy's performance with the default log configuration, especially
given that `h2` and `hyper` emit a _lot_ of `trace`-level logs in hot
paths. Ignoring those logs should now have a much lower overhead.

I'm going to do some benchmarking to quantify the performance
improvement from this change, so I'll add benchmark results when my
tests complete.

In addition, 0.2.1 fixes a bug where `EnvFilter`'s filter selection
differed from the `env_logger` behavior it was intended to emulate. If
the most specific filter directive that applies to a given event did not
enable that  event's level, filter selection would continue to try
decreasingly-specific filters, and would only disable the event if
**no** filters enabled it (see tokio-rs/tracing#512).

In practice, this means that if you set a filter like

```
LINKERD2_PROXY_LOG=warn,linkerd=debug,linkerd2_metrics=info
```

in an attempt to disable the `debug`-level events in the
`linkerd2_metrics` crate, they would still be enabled by the
`linkerd=debug` directive.

This is now fixed.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
olix0r pushed a commit to linkerd/linkerd2-proxy that referenced this pull request Feb 17, 2020
This release makes a number of significant performance improvements when
filtering events emitted through the `log` compatibility layer (such as
from `hyper` and `h2`). See tokio-rs/tracing#586 for details.

Since these performance improvements apply to cases where a filter
_disables_ an event, as well as to enabling events, this should improve
the proxy's performance with the default log configuration, especially
given that `h2` and `hyper` emit a _lot_ of `trace`-level logs in hot
paths. Ignoring those logs should now have a much lower overhead.

I'm going to do some benchmarking to quantify the performance
improvement from this change, so I'll add benchmark results when my
tests complete.

In addition, 0.2.1 fixes a bug where `EnvFilter`'s filter selection
differed from the `env_logger` behavior it was intended to emulate. If
the most specific filter directive that applies to a given event did not
enable that  event's level, filter selection would continue to try
decreasingly-specific filters, and would only disable the event if
**no** filters enabled it (see tokio-rs/tracing#512).

In practice, this means that if you set a filter like

```
LINKERD2_PROXY_LOG=warn,linkerd=debug,linkerd2_metrics=info
```

in an attempt to disable the `debug`-level events in the
`linkerd2_metrics` crate, they would still be enabled by the
`linkerd=debug` directive.

This is now fixed.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants