adding marker property to ExLogRecord

[dfa1]

Hi @dmlloyd,

@jamezp pointed out to move jboss-logging/jboss-logmanager#333 here, so please have a look and let's discuss. This change should be backward compatible (adding a new field).

This new property will be used to propagate the Marker to the actual
logging library. Since both SLF4J and Log4j define a different Marker
class without any common interface, here an Object is used.

This is a proposal to be able to receive the SLF4J (or log4j) Marker instance in the actual logging "driver". My use-case is to use it in a custom JsonProvider for quarkus:

@Singleton
public class MyJsonProvider implements JsonProvider {

    public void writeTo(JsonGenerator generator, ExtLogRecord event) throws IOException {
          if (event.getMetadata() instanceof org.slf4j.Marker) { 
               // special logic here
          }
    }
}

Also slf4j-jboss-logmanager should be adapted too (i.e. to avoid MarkerIgnoringBase):

 private void log(final Marker marker, final java.util.logging.Level level,  final String fqcn, final String message, final Throwable t, final Object[] params) {
        final ExtLogRecord rec = new ExtLogRecord(level, message, ExtLogRecord.FormatStyle.NO_FORMAT, fqcn);
        rec.setThrown(t);
        rec.setParameters(params);
        rec.setMarker(marker); <--- new code 
        logger.logRaw(rec);
    }

[jamezp]

I do wonder if we should call it something other than xxxMarker() in case we do ever decide to implement a marker. We could use something similar to MDC for this I would think as well.

[dfa1]

I'm open to suggestions! :-)

[jamezp]

I was trying to think of one and really couldn't :) Maybe it could be something like "userContext" or "recordContext". Maybe that's not great though.

[dmlloyd]

I think having it just be "marker" makes sense. By explicitly being agnostic about it, we don't constrain the user at all. If we ever come up with a separate marker concept, I'd probably look for a different name than "marker" - or just overload the existing marker one more time.

[dfa1]

hi @dmlloyd,

thanks for your time! I reverted Marker to be just Object. The only downside I see is that, since ExtLogMarker is serializable, a non-serializable Marker will break it, right?

[dmlloyd]

hi @dmlloyd,

thanks for your time! I reverted Marker to be just Object. The only downside I see is that, since ExtLogMarker is serializable, a non-serializable Marker will break it, right?

Right, but only if the record is actually serialized and there is a non-serializable marker on it. The same thing could happen with an MDC value, for example. But since the majority of users do not serialize their log records (at least, not using Java serialization), the possibility is of minimal concern.

[dmlloyd]

Thanks!

[dfa1]

thank you @dmlloyd @jamezp

Now I would need a release of this artifact to be able to use in the SLF4J binding for jboss-logging. What is the dependency/repository used by quarkus?

[dmlloyd]

This is it. But IIUC you'd need more than a release of this project; you'd also need @jamezp to accept the corresponding change to the slf4j-jboss-logmanager and log4j-jboss-logmanager projects - which I can't envision without jboss-logging/jboss-logmanager#333 being revived. So where does that leave us?

[dfa1]

I will revive that PR :) So, I will add the same method `setMarker(Object)` as here, right?

…

On Tue, 16 Mar 2021 at 13:35, David M. Lloyd ***@***.***> wrote: This is it. But IIUC you'd need more than a release of this project; you'd also need @jamezp <https://github.com/jamezp> to accept the corresponding change to the slf4j-jboss-logmanager and log4j-jboss-logmanager projects - which I can't envision without jboss-logging/jboss-logmanager#333 <jboss-logging/jboss-logmanager#333> being revived. So where does that leave us? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMHWKRLCPHRZB4XFFUP6JTTD5GAPANCNFSM4ZE6V7SA> .

-- -- Davide Angelocola

[dmlloyd]

Yeah you'd need it to be the same. And James will have to agree as well.