Skip to content
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

LOGBACK-1337 LogbackMDCAdapter should store mdc in LinkedHashMap to predict order of MDC in %X #473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

motlin
Copy link
Contributor

@motlin motlin commented Dec 4, 2019

No description provided.

@ceki
Copy link
Member

ceki commented Dec 4, 2019

Hi Craig,
So the idea is to iterate over the keys in the order of insertion. Correct?

@motlin
Copy link
Contributor Author

motlin commented Dec 5, 2019

That's right! I noticed that MDC iteration order is pseudorandom when working with logstash/logstash-logback-encoder. This surprised me a bit so I searched to see if anyone had written about it and I found LOGBACK-1337.

…redict order of MDC in %X

Signed-off-by: Craig P. Motlin <cmotlin@gmail.com>
@ceki
Copy link
Member

ceki commented Oct 8, 2024

@motlin Thank you for this PR.

Operations such as get() and put() for LinkedHashMap are known to be slower than that for HashMap. Given that MDC is a performance critical component, the change you propose needs more investigation.

However, the need for ordered iteration is duly noted.

@motlin
Copy link
Contributor Author

motlin commented Oct 8, 2024

I was hoping to revisit this and see if it could be included.

As an example of why it's desirable, I include MDC in my console output...

    <appender name="Console" class="ch.qos.logback.core.ConsoleAppender">
        <encoder>
            <pattern>%highlight(%-5level) %cyan(%date{HH:mm:ss.SSS, ${LOGGING_TIMEZONE}}) %gray(\(%file:%line\)) [%white(%thread)] %blue(%marker) {%magenta(%mdc)} %green(%logger): %message%n%rootException</pattern>
        </encoder>
    </appender>

And I add context to my MDC. It's not usually in one method, but spread across a few methods in the call stack. Usually the context has some sort of order. The pieces of context added later are more specific than the pieces added earlier.

MDC.put("first", "1");
MDC.put("second", "2");
MDC.put("third", "3");
LOGGER.info("Hello, World!");

With the current implementation, MDC gets printed to the console in pseudo-random order.

INFO  19:02:29.201 (Example.java:76) [main]  {third=3, first=1, second=2} com.example.Example: Hello, World!

@ceki
Copy link
Member

ceki commented Oct 8, 2024

Would output in alphabetical order suffice?

@motlin
Copy link
Contributor Author

motlin commented Oct 8, 2024

I think in some cases I could construct MDC key names with numbers inside to make sure they are sortable later.

In other cases I think the keys will be unpredictable in advance.

I suspect the way I use MDC is pretty common. I attach context about a service request, then about data reads/writes, then about the response. Some of the keys only last for a single log statement, others last for the duration of request.

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.

2 participants