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

Global logging feature, deprecate the logging extension and move its functionality to the core extension #1102

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Nov 8, 2023

Fix #270

@ppalaga ppalaga changed the title Deprecate the logging extension and move its functionality to the core extension Global logging feature, deprecate the logging extension and move its functionality to the core extension Nov 10, 2023
@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 10, 2023

This is quite a major change. @shumonsharif @famod, would you like to express your opinion, whether this is a good idea?

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 13, 2023

Any chance to review or (n)ack?

@shumonsharif
Copy link
Contributor

shumonsharif commented Nov 13, 2023

Hi @ppalaga Overall, I think it's a great idea, and the PR looks great as well. Please feel free to merge it.

Digging into this, I was left wondering a few things on logging, even though they may not be directly tied to the PR - some docs on the below may help others.

  • Can we specify the format for the CXF specific log messages easily?
  • Can we change the log levels at runtime (using -D JVM arguments)?

@ppalaga ppalaga mentioned this pull request Nov 13, 2023
@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 13, 2023

Thanks @shumonsharif , those are good ideas, I have filed #1107

@famod
Copy link
Contributor

famod commented Nov 14, 2023

@ppalaga first off, I finally answered you initial question: #270 (comment)
And of course thanks for addressing this topic!

One question: That quarkus.cxf.logging.enabled-for = clients|services|clients-and-services|none idea didn't make it in the end, right?
So you can only enable/disable logging for clients and services globally, but you can disable/enable for clients or services via two distinct properties or even per client or service, right?

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 14, 2023

One question: That quarkus.cxf.logging.enabled-for = clients|services|clients-and-services|none idea didn't make it in the end, right?

No, because I forgot 😂. I can implement it if you find it useful?

So you can only enable/disable logging for clients and services globally, but you can disable/enable for clients or services via two distinct properties or even per client or service, right?

Yes, with current impl, if you enable logging globally, you can still mute it for individual clients and services by passing quarkus.cxf.[client.myClient|endpoint."/foo"].logging.enabled = false

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 15, 2023

3c0e443: Added quarkus.cxf.logging.enabled-for = clients|services|clients-and-services|none.

@ppalaga ppalaga merged commit 530e5a4 into quarkiverse:main Nov 15, 2023
17 checks passed
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.

Global features
3 participants