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

tracing-subscriber: fix parsing of filters with multiple span fields #2959

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mladedav
Copy link
Contributor

@mladedav mladedav commented May 1, 2024

Motivation

Closes #2935
Builds on #2936

Parsing of filters checking for field values is broken when used with more than one field. There may be multiple directives split by commas (e.g. lib=warn,bin=info) but the directives themselves can use commas as separators for span fields (e.g. [span-name{field1,field2}].

Currently, a string containing all directives is first split by commas and then each substring is parsed as its own directive. This does not work when commas are present also because of filters on multiple fields.

Solution

Make the split aware of { and } and do not split on commas that are between those two characters. This fix makes the code work as documented and multiple fields can be matched as long as they do not contain } in its value (e.g. [name{json_field="{}"}]).

Another potential issue is when a user provides a malformed directive such as [name{] then all subsequent directives will be considered as part of the malformed directive so they will not be applied.

Alternatives

Since the feature seems to never have worked, we could also decide to use a different separator for fields, e.g. ;. Then we could simply have [span1{field1=0;field2=2}]=info,[span2{field3;field4}] and the split between directives would not need to be changed. This would require changes to documentation. As env_logger does not have any feature like this, having a similar format should not be a concern here.

@mladedav mladedav requested review from hawkw, davidbarsky and a team as code owners May 1, 2024 08:32
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.

parsing multiple field values in EnvFilter is broken
1 participant