MDC properties inconsistency in health checks after recent update

[atanasnik]

Hello,

We have recently updated a project to the LTS (3.20.0). To summarise, the distribution of MDC properties across health check logs seems non-deterministic. As this has happened before, here is a link to the previous discussion where the topic is explained in detail (the issue was resolved). I cannot reproduce the problem when switching to 3.12.3, for example, as this was the version where the problem was fixed.

I am not sure whether the same problem has reoccurred, or if anything else has happened, that is leading to this inconsistency.
Any advice is appreciated.

[quarkus-bot[bot]]

/cc @jmartisk (health), @xstefank (health)

[xstefank]

Hi @atanasnik, that's strange, nothing change in this code. I will double-check the original reproducer but I'm also working on a major update of SR Health that might help with this too.

[xstefank]

@atanasnik I updated my reproducer to the latest Quarkus 3.21.4 - https://github.com/xstefank/quarkus-reproducers/tree/main/mdc-propagation and it still seems to work.

I tried also with 3.20.0 and I also can't reproduce it.

EDIT: I also added a test that calls all health check 100000 times and it passes - https://github.com/xstefank/quarkus-reproducers/blob/main/mdc-propagation/src/test/java/org/acme/MDCLoggingTest.java.

Can you reproduce it all the time?

[atanasnik]

Hi, thank you for reaching out! I tried to reproduce the problem here: xstefank/quarkus-reproducers#2
It seems to fail only when adding the MDC.put in the mentioned endpoint, in this case.

[xstefank]

Hi @atanasnik, unfotunatelly, I can't figure out what MDC does to mess up Vertx context that is otherwise provided by Quarkus. But I was able to fix your use case by overriding the QuarkusAsyncHealtchCheckFactory to use a new vertx instance - xstefank/quarkus-reproducers@fddaf23.

I must say that I don't think this is a correct solution but I don't understand VertxMDC enough to be sure that any changes that I make won't break other behaviours which are now tight to threads. Let me know if this will work for you or not please.

[cescoffier]

Do you have the 2 code snippets?

[atanasnik]

I used the code from the last commit provided in this discussion, the only thing I did was to replace line 38 with Context duplicatedContext = VertxContext.newNestedContext().

[cescoffier]

As I said in the other thread, you should never call Vertx.vertx() yourself.

[atanasnik]

Yes, I understand, though I am not sure what we can do next.

[cescoffier]

Remove that call and retrieve the current duplicated context—make sure you stay on this one and nothing creates a new one.

[xstefank]

@atanasnik can you try this PR in your application please? #48009

[atanasnik]

Hi! I cannot guarantee this 100%, but I am currently unable to reproduce the problem, after building with this PR's changes.

[xstefank]

@atanasnik, the PR was rejected for valid reasons, and I closed it. But in the end, I think I found out what the issue is. If you can give it another spin with this new PR, I would like to have your ack -> #48057.

[xstefank]

PR is merged so please mark this as an answer if it is solved for you. Thanks.