Skip to content

implement a way to filter fields by name on log json formatter#2353

Draft
bnjjj wants to merge 1 commit intotokio-rs:masterfrom
bnjjj:filter_fields_name
Draft

implement a way to filter fields by name on log json formatter#2353
bnjjj wants to merge 1 commit intotokio-rs:masterfrom
bnjjj:filter_fields_name

Conversation

@bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Oct 24, 2022

This is a WIP PR to have your feedback

Motivation

I recently needed to filter fields from json logs, especially because sometimes for some logics in our own exporters we put some specific fields on spans and logs. But these fields are only useful for logic stuffs, they doesn't add any useful information for the users. So I looked over your documentation but was only able to find a way to filer the entire event based on fields and not filter fields from an event (log).

Solution

Implement the method filter_fields_by_name method for json formatter.

Example:

let subscriber = subscriber()
            .flatten_event(false)
            .with_current_span(true)
            .with_span_list(true)
            .filter_fields_by_name(|field_name| field_name != "answer" && field_name != "test");

@davidbarsky
Copy link
Member

davidbarsky commented Nov 2, 2022

hi! i'm sorry for the delay in reviewing this. here's some general feedback:

  1. in general, please point PRs at the master branch instead of v0.1.0! We'll handle backports.
  2. I don't really want to add behavior to just the JSON formatter (in this this, filter_fields_by_name) that doesn't apply to other formatters. We already have a lot of divergence between formatters, and I don't want to add something as big as this.
  3. Instead of this approach, I wrapping Json and JsonFields in another FormatFields or Visitor that is capable of filtering out fields you're not interested in. This has the nice side-effect of being generalizable to other formatters (if you're interested, we'd accept this change!)

@hawkw
Copy link
Member

hawkw commented Nov 4, 2022

You may want to look at issue #73, which proposes a more general solution like what @davidbarsky's describing in #2353 (comment).

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 9, 2022

Thanks ! I will take a look and update the code :)

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj bnjjj force-pushed the filter_fields_name branch from 72c51b7 to 500276f Compare November 10, 2022 11:03
@bnjjj bnjjj changed the base branch from v0.1.x to master November 10, 2022 11:03
@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 10, 2022

Ok @hawkw and @davidbarsky thanks for your feedback. I already wrote a small FieldFilter visitor. If you have time to take a look at this implementation that would be great. I will continue my work on this by implementing helper methods on Format to be able to use it directly on Format. Tell me if I'm wrong :)

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 16, 2022

Regarding my message on discord here do you have an idea on how I could properly implement it for json format @davidbarsky and @hawkw ? THanks a lot

@synek317
Copy link

The link to the discord discussion is no longer working. Is there still interest in implementing this feature? Could I try to help with that?

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.

4 participants