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.

Sign up for GitHub

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

Jump to bottom

EnvFilter cannot parse filters with multiple fields #1584

Open
CAD97 opened this issue Sep 21, 2021 · 0 comments
Open

EnvFilter cannot parse filters with multiple fields #1584

CAD97 opened this issue Sep 21, 2021 · 0 comments

Comments

@CAD97
Copy link
Contributor

CAD97 commented Sep 21, 2021

... and as a result, the filter::env::tests::callsite_enabled_includes_span_directive_multiple_fields test is just broken.

Bug Report

Crates

tracing-subscriber

Description

The first thing EnvFilter::parse does is split the input string on commas. Because field directives are also comma-delimited, this means that the whole directive will now just fail to parse.

Potential fix

#1542 - rewrite the parser to properly handle commas.
@CAD97 CAD97 mentioned this issue Sep 21, 2021
Closed
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
@rustonaut
subscriber: added builder for filter Directive
c575e47 
\## Motivation

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

\## Solution

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#3016, tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
@rustonaut
subscriber: added builder for filter Directive
b2347b2 
Motivation
----------

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#3016, tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
@rustonaut
subscriber: added builder for filter Directive
7864c36 
Motivation
----------

1. There are some filters which can not be created with the current
   Directive string repr syntax, like e.g. the strings `bool`, `1`
   or `2.3` or values containing characters including but not limited
   to `}],` (for some of this there are partial fixes in other PRS).
   Furthermore updating the syntax the accomadate this is always a
   potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` progrematically,
   e.g. from a different kind of logging config or to set more
   complicated defaults.

3. We might want to add other matching capabilities in the future
   which might not be representable in the string syntax (or at
   least entail a braking change). Like e.g. matching fields regex,
   matching based on `dyn Value`, a field matching the string `true`
   and boolean `true` at the same time,  etc.

By allowing programatic creation of `Directive`s we allow downstream
users to work around existing issues, experiement with solution to
such issues in external crates and in addition set the fundation for
adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
@rustonaut
subscriber: added builder for filter Directive
f401122 
Motivation
----------

1. There are some filters which can not be created with the current Directive string repr syntax, like e.g. the strings `bool`, `1` or `2.3` or values containing characters including but not limited to `}],` (for some of this there are partial fixes in other PRS). Furthermore updating the syntax the accommodate this is always a potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` programmatically, e.g. from a different kind of logging config or to set more complicated defaults.

3. We might want to add other matching capabilities in the future which might not be re presentable in the string syntax (or at least entail a braking change). Like e.g. matching fields regex, matching based on `dyn Value`, a field matching the string `true` and boolean `true` at the same time,  etc.

By allowing programmatic creation of `Directive`s we allow downstream users to work around existing issues, experiment with solution to such issues in external crates and in addition set the foundation for adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
@rustonaut rustonaut mentioned this issue Jun 24, 2024
Open
rustonaut added a commit to rustonaut/tracing that referenced this issue Jun 24, 2024
@rustonaut
subscriber: added builder for filter Directive
0c5d313 
Motivation
----------

1. There are some filters which can not be created with the current Directive string repr syntax, like e.g. the strings `bool`, `1` or `2.3` or values containing characters including but not limited to `}],` (for some of this there are partial fixes in other PRS). Furthermore updating the syntax the accommodate this is always a potential braking change, one which also can be subtle to detect.

2. Sometimes there is need to create `Directives` programmatically, e.g. from a different kind of logging config or to set more complicated defaults.

3. We might want to add other matching capabilities in the future which might not be re presentable in the string syntax (or at least entail a braking change). Like e.g. matching fields regex, matching based on `dyn Value`, a field matching the string `true` and boolean `true` at the same time,  etc.

By allowing programmatic creation of `Directive`s we allow downstream users to work around existing issues, experiment with solution to such issues in external crates and in addition set the foundation for adding future matching capabilities in the future.

Solution
--------

- adds `Directive::builder()` / `DirectiveBuilder`
- `ValueMatch` is now public, but wrapped to that we can freely extend and change it without braking changes

Workaround For: tokio-rs#1584, tokio-rs#1181
Fixes: tokio-rs#2507, tokio-rs#404
Refs: tokio-rs#1584, tokio-rs#1181
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

implement stricter env filter parsing CAD97/tracing
1 participant
@CAD97