logstash-logback-encoder icon indicating copy to clipboard operation
logstash-logback-encoder copied to clipboard

Clarify threadNameFormat (and possibly add some words about it in the documentation)

Open brenuart opened this issue 4 years ago • 2 comments

Both the AsyncDisruptorAppender and LogstashTcpSocketAppender appenders allow for the customisation of the name of their processing threads. This customisation is done via a standard Java format pattern set via the threadNamePattern configuration parameter.

Although very handy, this feature does not seem to be documented in the project README. The javadoc however indicates that the following placeholders/arguments are supported:

  • %1: the appender name
  • %2: the numerical thread index
  • %3: the hostname of the connected destination
  • %4: the port of the connected destination

The actual meaning of the numerical thread index is not clear to me. At first it seems to be a unique number incremented every time a new thread is created. This index can be used to differentiate between threads if the remaining part of the name format is "fixed". For instance the pattern logstash-%2 will produce thread names like logstash-1, logstash-2 and so forth. An appender like the TCP appender creates at most 4 threads depending on its configuration. With the previous pattern, threads will be named from logstash-1 to logstash-4. However, after a while, I found threads named with an index higher than 4 - like logstash-12. After some investigation it appears that the thread index is incremented every time a new thread name is computed and not when a thread is created. A new thread name is computed when the appender reconnects to another destination for instance...

Is it expected?

Also, the first thread index is 2 instead of 1 or 0. This is because the counter is initialised with 1 (see AsyncDisruptorAppender.java#L240) and incremented before it's value is used (see AsyncDisruptorAppender#L567).

brenuart avatar Jul 27 '21 18:07 brenuart

Side question: should we allow people to set their own threadFactory via setThreadFactory? What would be the use case?

brenuart avatar Jul 28 '21 19:07 brenuart

Is it expected?

It is expected that logstash-logback-encoder can provide the following guarantees (which it does by default): A) thread names can be easily associated to logging (satisfied by the logback-appender-{appenderName} in the thread name) B) thread names identify the destination, (satisfied by including the destination in the thread name, and updating the thread name when the destination changes) C) all threads created by logstash-logback-encoder have unique names (satisfied by including an incrementing id in the thread name)

However, if the user changes the format pattern, and does not include all the variables, then no guarantees can be made.

Any other behavior is not defined at the moment, and should not be relied upon.

I don't see much of a need to provide any more guarantees. And I'm not terribly concerned with the behavior of the id, since it's only purpose is to guarantee uniqueness.

But having said that, if you feel the current behavior is hurting the usability of the library, feel free to improve it, as long as it doesn't get crazy complex. This is an area where simplicity is preferred.

should we allow people to set their own threadFactory via setThreadFactory? What would be the use case?

This gives users full control over the threads created. For example to set other properties of threads, use a thread group, decorate threads for monitoring, etc. It's an advanced use case.

philsttr avatar Jul 28 '21 23:07 philsttr