-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Filter metadata along with messages #551
Conversation
@dbmikus this looks great! @indexzero you good with this being a minor bump? cc @pose |
What do you guys think of including the log level as an optional third argument to the function? It should never be returned or modified by the function, so we would just pass it in as a string so that nothing done to it would matter. This would just allow for some extra conditional filtering based on the log level. |
@dbmikus why an Array instead of an Object with two properties? I would prefer the latter if you don't have any big reservations against it. |
…with msg and meta properties
Yeah, it should be an object. I was originally thinking I could just return and array and unpack it to two variables during assignment, but I forgot JS does not do that. I just made the changes. |
So, the object it returns has the properties:
Are those names fine? Or should it be:
|
So, I actually noticed that there is an undocumented set of options called "rewriters". These look to be filters for metadata. They are functions that accept the
I think that comment is just wrong. I actually think that everything should go through the filters and we should drop rewriters. The motivation for this is that consolidating all filtering/rewriting into one set of functions will allow meta and msg to influence each other when filtering. This comes into play more-so if we have multiple filters. Otherwise, all rewriters will run before the filter has a chance to change the message or meta. I'm going to add What do you guys think? |
Cherry-picked. Thank you for your contribution. FYI filters are likely going away in 1.0.0 once we get custom formatters (i.e. streams) properly integrated 👍 |
Cool. So are filters and formatters going to become merged into streams? |
I added support for filtering metadata along with messages. You can still add filter functions that only accept the message and return the filtered message, but now you can also add filters that will accept the metadata and return a list of
[filtered_msg, filtered_meta]
.Also added a unit test for the new filter method.