-
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
Fix 'quarkus.otel.exporter.otlp.traces.timeout' default value #33326
Conversation
/cc @brunobat (opentelemetry), @radcortez (opentelemetry) |
✔️ 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. |
This should work once this gets merged; #32929 as I made the check case insensitive in the converter, also since Duration.parse does support both lowercase and uppercase. |
@mazenkhalil what type of error do you have? Because it's not the only duration specific like that and IIRC it works OK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Projects compile ok with this config.
What error do you see?
If we are going to perform this change, we also need to update HibernateSearchOutboxPollingRuntimeConfigPersistenceUnit
and many other classes.
@gsmet @brunobat The following error appears on start only when 2023-05-15 16:21:04,101 ERROR [io.qua.run.Application] (Quarkus Main Thread) Failed to start application (with profile [dev]): io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException: Invalid duration property otel.exporter.otlp.traces.timeout=10S. Invalid duration string, found: S
at io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties.getDuration(DefaultConfigProperties.java:173)
at io.opentelemetry.exporter.internal.otlp.OtlpConfigUtil.configureOtlpExporterBuilder(OtlpConfigUtil.java:101)
at io.opentelemetry.exporter.otlp.internal.OtlpSpanExporterProvider.createExporter(OtlpSpanExporterProvider.java:37)
at io.opentelemetry.sdk.autoconfigure.SpiUtil.lambda$loadConfigurable$0(SpiUtil.java:48)
at io.opentelemetry.sdk.autoconfigure.NamedSpiManager.tryLoadImplementationForName(NamedSpiManager.java:46)
at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708)
at io.opentelemetry.sdk.autoconfigure.NamedSpiManager.getByName(NamedSpiManager.java:36)
at io.opentelemetry.sdk.autoconfigure.SpanExporterConfiguration.configureExporter(SpanExporterConfiguration.java:91)
at io.opentelemetry.sdk.autoconfigure.SpanExporterConfiguration.configureSpanExporters(SpanExporterConfiguration.java:66)
at io.opentelemetry.sdk.autoconfigure.TracerProviderConfiguration.configureTracerProvider(TracerProviderConfiguration.java:51)
at io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder.build(AutoConfiguredOpenTelemetrySdkBuilder.java:356)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer.getOpenTelemetry(OpenTelemetryProducer.java:139)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer_ProducerMethod_getOpenTelemetry_93822cfa4b2dfbcfc394a796cc51d5c75c33d0e9_Bean.doCreate(Unknown Source)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer_ProducerMethod_getOpenTelemetry_93822cfa4b2dfbcfc394a796cc51d5c75c33d0e9_Bean.create(Unknown Source)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer_ProducerMethod_getOpenTelemetry_93822cfa4b2dfbcfc394a796cc51d5c75c33d0e9_Bean.create(Unknown Source)
at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:113)
at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:37)
at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:34)
at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:26)
at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer_ProducerMethod_getOpenTelemetry_93822cfa4b2dfbcfc394a796cc51d5c75c33d0e9_Bean.get(Unknown Source)
at io.quarkus.opentelemetry.runtime.OpenTelemetryProducer_ProducerMethod_getOpenTelemetry_93822cfa4b2dfbcfc394a796cc51d5c75c33d0e9_Bean.get(Unknown Source)
at io.quarkus.arc.impl.InstanceImpl.getBeanInstance(InstanceImpl.java:323)
at io.quarkus.arc.impl.InstanceImpl.getInternal(InstanceImpl.java:307)
at io.quarkus.arc.impl.InstanceImpl.get(InstanceImpl.java:188)
at io.quarkus.opentelemetry.runtime.OpenTelemetryRecorder.createOpenTelemetry(OpenTelemetryRecorder.java:38)
at io.quarkus.deployment.steps.OpenTelemetryProcessor$createOpenTelemetry624023256.deploy_0(Unknown Source)
at io.quarkus.deployment.steps.OpenTelemetryProcessor$createOpenTelemetry624023256.deploy(Unknown Source)
at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
at io.quarkus.runtime.Application.start(Application.java:101)
at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:111)
at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
at io.quarkus.runner.GeneratedMain.main(Unknown Source)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:104)
at java.base/java.lang.Thread.run(Thread.java:833) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that's the OTel SDK failing because it doesn't recognise the pattern.
Let's fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that's the right fix.
Isn't the problem that we do not format properly the duration we push to OTel? We need to support the user setting the duration to 5S
. That's supported by Quarkus.
So somehow, we need to make sure that the Duration we have in this field is pushed to OTel in a format OTel can understand.
@brunobat this comment is mostly for you ^ |
@gsmet Ideally yes, that should be done around here: https://github.com/quarkusio/quarkus/blob/de16f435b6e3112fc73b0f032a98e0e838739dab/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/OpenTelemetryProducer.java#LL157C26-L157C26 Or use converters in each prop... This would be cleaner. However, support for the stock OTLP exporter is not yet official. (edited) |
Well, my problem with this is that we have a If OTel doesn't support that, we need to format the Duration in a way that is understandable by it. Sure, we can merge this PR and fix the default value but that won't solve the problem for the user-defined values. I will remove my |
We can merge this workaround but we will need a proper fix.
Thanks for raising the issue and providing a PR! |
The default duration for timeout property is wrongly defined and causes runtime error.