-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Quarkus does not start in dev mode with Otel logs enabled and OIDC enabled #44181
Comments
/cc @pedroigor (oidc), @sberyozkin (oidc) |
I also tried with |
@BogdanAlexa1234 I'll try to have a look tomorrow |
This code, https://github.com/quarkusio/quarkus/blob/main/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/devservices/keycloak/KeycloakDevUIProcessor.java#L42, is indeed repeatedly called, if
Can you please have a quick look, is there something that we can do at the |
Also CC @brunobat, Hi Bruno, can you please link to the deployment code which deals with |
Well, from the I'm not an expert neither in Keycloak nor in OTel so I cannot really say if it's possible to break the cycle easily... |
That seems to be the case. Line 19 in 42e6c32
And AbstractDevUIProcessor is creating a bean here: Line 73 in a8f21be
It is very strange that something like this hasn't happen before because of this synthetic bean creation in the Any extension requiring beans instance at runtime init might get this issue... |
The important part:
|
The reason I'd also like CC @phillip-kruger.
What I'm not sure about, why exactly |
I'm surprised this one doesn't get more attention as it makes OTel Logging mostly unusable for anyone using OIDC - which will concern quite a lot of people out there. For me, there are two alternatives:
I'm not sure if one of this is better or even doable but the current situation is not something we can live with. @mkouba I think your expertise on the first point would be interesting |
I can't say I was able to walk through it all but what I see from the code is that |
@mkouba and we're sure it won't pave the way for weird race conditions? That's what I feared. I'll have a look and prepare a patch with what you're suggesting and make sure it fixes the problem and then we can discuss how safe it is. |
Fixes quarkusio#44181 Co-authored-by: Martin Kouba <mkouba@redhat.com>
I created #44557 with Martin's suggestion. |
I can't say because I don't know much about this domain. But it's should be functionally equivalent, i.e. consuming
👍 |
#44557, from the CI failures will not work because OTel is a Synthetic bean and it dosen't seem yet initialised when Arc is used. |
Yeah, I'm not sure how to solve this. We could maybe discuss with @phillip-kruger if we could come up with a way to produce properties for the Dev UI outside of beans but I wonder if we wouldn't get hit by that in the future. We actually already have a way to delay the logging until the config is properly initialized but my guess is that it might not be easily be delayed more until after the CDI initialization. But TBH, I don't see a lot more options. Because even if we could avoid creating beans for passing Dev UI properties, I'm pretty sure we will again get hit by this issue in other circumstances. |
True, and probably it might be happening elsewhere already but we are not seeing it because OTel logging is off by default. |
I am busy looking at this. Seems like most of the things currently being exposed in the Json RPC service (for Dev UI) in OIDC can be moved to a BuildTimeAction. This seems to fix the problem with Dev UI, but I still get |
I have opened #44590 to change the way that data is made available in the OIDC Dev UI Json-RPC Service. There are various ways that this can be done, I took the approach with the smallest change. So the data is still being served with a Json-RPC Service, but the Json-RPC service is not looking up a bean, the data is directly set on the service with a recorder. Another option would be to use BuildTimeData, that can also take a RuntimeValue, but this is a bigger change. (I did this initially, but the end result is the same). This unfortunately does not fix this issue, it just takes Dev UI out of the picture. The error we get currently:
With the changes in my PR, there is no more Dev UI in the stacktrace, however we now get this:
I hope this help you to get to the root issue. (cc @brunobat @gsmet @mkouba @sberyozkin ) |
@phillip-kruger this is actually good news!
Means you don't have a OTel collector on that endpoint, therefore telemetry could not be exported. |
Ok great! |
I am so glad there is a potential fix here. I had this issues and was about to go insane. |
I believe this issue is resolved on main: see also #45360 |
Describe the bug
I have an application that uses
quarkus-oidc
andquarkus-opentelemetry
extensions withquarkus.otel.logs.enabled=true
property.Start the application using
./gradlew quarkusDev
Expected behavior
Application starts in dev mode with keycloak devservice running.
Actual behavior
Application does not start because of ChainBuildException
How to Reproduce?
Reproducer repository: https://github.com/BogdanAlexa1234/quarkus-repro-otel-log-keycloak
Output of
uname -a
orver
Linux bogdan 5.15.0-124-generic #134-Ubuntu SMP Fri Sep 27 20:20:17 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Output of
java -version
openjdk 17.0.8 2023-07-18 OpenJDK Runtime Environment Temurin-17.0.8+7 (build 17.0.8+7) OpenJDK 64-Bit Server VM Temurin-17.0.8+7 (build 17.0.8+7, mixed mode, sharing)
Quarkus version or git rev
3.16.0
Build tool (ie. output of
mvnw --version
orgradlew --version
)Gradle 8.10
Additional information
No response
The text was updated successfully, but these errors were encountered: