subscriber: strip quotes of EnvFilter field values

[rustonaut]

Motivation

The documentation states that [span_b{name=\"bob\"}] should match spans where the value of name is bob but it does match spans where the value of name is "bob".

Additionally there is no good way to match strings which happen to be true, false or a number. [{value="true"}] matches the rust string r#""true""# and [{value=true}] only matches a bool value true but not a string true.

Solution

• for values wrapped into " quotes we stip them when parsing ValueMatach / MatchDebug
• if encountering unbalanced outer quotes (e.g. "bob, bob" or " we treat it as a syntax error)
• the behavior of quotes inside of a value (e.g. bo"b) is not changed (i.e. bo"b matches bo"b)

Partially Fixes: #1181 Fixes: #2809

Stability Considerations

• some malformed/ambiguous directives did pass parsing, it might make sense to change the behavior for unbalanced outer quotes ("bob, bob") to not create an error but parse include the " in the regex like previously

• if anyone did accidentally log a string as debug (e.g. foo = %"my string") then the previous filter happened to work, and this happening isn't even that unlikely (due to instrument) . At the same time not stripping the quotes is a huge problem for anything of which the debug/display output is true/false or a number

  • in context of this maybe it makes sense to provide an alternative Directive parsing algorithm and some form to opt into it. We could start by defining a grammar for the syntax. Avoiding ambiguity and allowing escaping of various characters (e.g. ,[{}]) on the level of the directive string representation. Which would fix multiple issues caused from the current formats ambiguities.
    • this could be a feature flag, as this aspect tends to be handled by the final upstream crate and not libraries
    • this could be available through methods like EnvFilter::builder2() and similar
  • or deprecating the EnvFilter after providing a alternative more robust implementation could also work (e.g. a FilterDirectives given that the EnvFilter is IMHO by now more focused on flexible filter directives then on).
  • EDIT: or provide an interface to programmatically build Directives in which case external crates can provide their own parsing solutions (or e.g. from config deserialization) without having to re-implement the filter application logic. I really like this idea. Through that would make the EnvFilter have even less to do with the env.

[rustonaut]

Some additional comments:

• I hadn't been aware that the compatibility problems might be bigger then expected until after nearly finishing writing this PR, I still want to present it but I'm no longer sure if it's the best way to fix the problem. But at least we should update the documentation + add some (? but how ?) way to handle cases like %true / ?10 etc. and test for this cases, too.

• maybe a [{field=true}] should match all of true , %true and ?true ?

• The reason the integration tests are a bit more complicated is because there is the issue that [{field}] filters apply to events but [{field=value}] filters only apply to spans but not events.

[rustonaut]

Closed as fixing the issues also introduce breaking changes, instead following will be done:

• allow programmatically creating filters #3018 (is a workaround for the issue this PR was for and a long requested feature)

• TODO PR to add integration tests (based on the ones from this PR) to test that other PRs don't accidentally break parsing/filtering related to wrapping " quotes

• TODO PR to update documentation to be more clear about limitations