How to implement a filtering middleware core?

[tchap]

Hi,

I implemented a core that sends log entries to Sentry. But I want to skip entries containing particular fields from being sent to Sentry. For that reason I tried to implement a core that would wrap the Sentry core and simply skip writing the entry when certain field is present.

The thing is, I don't see how the filtering core can be done in a generic way since calling Check on the filtering core should obviously call Check on the core being wrapped. But that will add the underlying core into the checked entry, causing it to always contain the log entry. In other words, this does not work:

https://github.com/tchap/zapext/blob/a5d992351555de581c6d36e2829287a6d1c2f16a/middleware/core_filtering.go

The problem seems to be that fields are not available in Check so I cannot run the filter function without really calling the underlying core's Check first.

Any ideas how to do this? Am I missing something obvious?

[akshayjshah]

@tchap This is a great question - I'm going to add it to the FAQ whenever I can free up the time to write decent docs (hopefully soon!).

You're right that Core doesn't have access to the fields. There's a simple reason for this - most of them have already been serialized into []byte, and the core doesn't always know what the encoding is!

However, you should still be able to accomplish this. Rather than trying to do the filtering in Check, do it in With and Write. In With, you can inspect each of the fields - if any of them make you want to filter out the entry, return zapcore.NewNopCore(). In Write, do the same - look at the fields, and if any of them make you want to drop the log entry, short-circuit and return nil.

I suspect that this can actually be quite generic - if you have time, I'd love to see a PR that adds

zap.NewFilteringCore(filter func(Field) bool, core zapcore.Core) zapcore.Core

[tchap]

@akshayjshah I can send the PR in the evening or tomorrow, no problem.

I would personally swap the arguments, i.e. I would make it

zap.NewFilteringCore(next zapcore.Core, filter func(Field) bool) zapcore.Core

But I will make it as you wish.

[tchap]

Also from the naming and signature we have chosen it may seem that the core is just filtering fields and then sending the entry containing the fields not filtered out. I will think about a better naming.

[tchap]

Perhaps we could add EntryFilteringCore and FieldFilteringCore separately.

zap.NewEntryFilteringCore(next zapcore.Core, filter func(Field) bool) zapcore.Core
zap.NewFieldFilteringCore(next zapcore.Core, filter func(Field) bool) zapcore.Core

The former would drop the whole entry on filter returning false on any field. The latter would just drop the fields where filter returns false.

[akshayjshah]

Sure, sounds good.

[tchap]

@akshayjshah This does not seem to be working, the test is failing:

tchap@9a7e9cb

What am I doing wrong?

[tchap]

Ok, I see where the problem is...

[tchap]

Actually I don't :-)

tchap@10dbb79

This doesn't work either. Anyway, I guess that I will open a pull request and we can discuss there...

[akshayjshah]

Discussed on the PR; after looking more carefully, it's possible to do this reasonably well, but not in a way that can wrap all zapcore.Core implementations. Since it's not possible to make it fully generic, I've recommended that @tchap take the code he wrote (which is quite nice, and works well with zap's built-in Core types) and put it into an extension package or into the project that needs it.