Extension logging-gelf: Adding missing GELF MDC Config

[donkon]

Adding additional MDC Config :

• mdcFields
• dynamicMdcFields
• dynamicMdcFieldTypes

Example application.properties:
quarkus.log.handler.gelf.dynamic-mdc-field-types=jobId=String

Is there a reason why this config is not included in the quarkus logging-gelf extension?

[quarkus-bot]

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

_{This message is automatically generated by a bot.}

[geoand]

Thanks for this!

Can you please squash the commit?

[loicmathieu]

@donkon

Is there a reason why this config is not included in the quarkus logging-gelf extension?

For simplicity, we never include all possible configuration knots when integrating a library.

Also, note that the underlying library is no longer maintained, I have a plan to offer a replacement for centralized logging, certainly via opentelemetry logging as I already have a prototype.

[geoand]

I have a plan to offer a replacement for centralized logging, certainly via opentelemetry logging as I already have a prototype.

Nice! You should probably talk to @brunobat about this

[loicmathieu]

Did you test that the MDC implementation used by the library is compatible with the Quarkus one?

You should idealy enhance the existing test with a message using the MDC and verify that it's correctly rendered.

You should also check in reactive as it should have special MDC handling, I'm not sure it's OOTB.

[loicmathieu]

@geoand yes, I planned to open a discussion on Zulip ... then forgot ... Will start a discussion on Zulip with my prototype as we need a replacement for this library.

[brunobat]

@donkon can you please add a test?

[donkon]

@brunobat I see no (JUnit)Tests at all (extensions/logging-gelf/). Can you point me to the right direction what Tests you expect?

Please note that I'm just delegating additional config to the underlying library (like the actual code does) without any special logic

[loicmathieu]

@donkon tests are inside the integration test directory, see https://github.com/quarkusio/quarkus/tree/main/integration-tests/logging-gelf.

You should add a log with a dynamic MDC here: https://github.com/quarkusio/quarkus/blob/main/integration-tests/logging-gelf/src/main/java/io/quarkus/logging/gelf/it/GelfLogHandlerResource.java

Then assert in the test that the log has been set and the MDC parameters resolved here: https://github.com/quarkusio/quarkus/blob/main/integration-tests/logging-gelf/src/test/java/io/quarkus/logging/gelf/it/GelfLogHandlerTest.java

[donkon]

@brunobat done! I extended the existing testcase. Can you please check!

[loicmathieu]

LGTM, thanks

[loicmathieu]

Can you rebase on main and squash your commit please?

[donkon]

@loicmathieu done

[quarkus-bot]

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.