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

Prevent the usage of OpenTelemetry in GcpDefaultsConfigSourceFactory #487

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Aug 23, 2023

This is needed because GcpDefaultsConfigSourceFactory is called very early in the application startup
and can thus mess up the OpenTelemetry configuration performed later by Quarkus

Fixes: quarkusio/quarkus#35500

P.S. This pattern is also used in Quarkus code here.

This is needed because GcpDefaultsConfigSourceFactory
is called very early in the application startup
and can thus mess up the OpenTelemetry configuration
performed later by Quarkus

Fixes: quarkusio/quarkus#35500
Copy link

@brunobat brunobat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the same fix used in quarkusio/quarkus#22864
for the Google client.

Copy link
Collaborator

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@loicmathieu loicmathieu merged commit 8197daf into quarkiverse:main Aug 24, 2023
1 check passed
@geoand geoand deleted the quarkus-#35500 branch August 24, 2023 07:07
@loicmathieu
Copy link
Collaborator

@all-contributors please add @geoand for code

@allcontributors
Copy link
Contributor

@loicmathieu

I've put up a pull request to add @geoand! 🎉

@gsmet
Copy link
Member

gsmet commented Aug 24, 2023

@loicmathieu could you cut a release so that we have one fully compatible with 3.3?

@loicmathieu
Copy link
Collaborator

@gsmet yes, I'll do it today if I find the time.

@loicmathieu
Copy link
Collaborator

@snazy
Copy link
Contributor

snazy commented Aug 30, 2023

Sorry, but this still happens w/ 3.3.1 (via the platform-bom). Reproducer in this manually updated PR (use ./gradlew :nessie-quarkus:quarkusBuild ; java -jar nessie/servers/quarkus-server/build/quarkus-app/quarkus-run.jar to cross-check)

2023-08-30 22:19:11,120 ERROR [io.qua.run.Application] (main) Failed to start application (with profile [prod]): java.lang.RuntimeException: Failed to start quarkus
	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.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.doRun(QuarkusEntryPoint.java:61)
	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.main(QuarkusEntryPoint.java:32)
Caused by: jakarta.enterprise.inject.CreationException: Error creating synthetic bean [2deeb341bce4a230183bf3a228841625fe2519ae]: io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException: Unexpected configuration error
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_Bean.doCreate(Unknown Source)
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_Bean.create(Unknown Source)
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_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:32)
	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_Bean.get(Unknown Source)
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_Bean.get(Unknown Source)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:557)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:537)
	at io.quarkus.arc.impl.ArcContainerImpl.beanInstanceHandle(ArcContainerImpl.java:570)
	at io.quarkus.arc.impl.ArcContainerImpl$3.get(ArcContainerImpl.java:334)
	at io.quarkus.arc.impl.ArcContainerImpl$3.get(ArcContainerImpl.java:331)
	at io.quarkus.arc.runtime.BeanContainerImpl$1.create(BeanContainerImpl.java:46)
	at io.quarkus.arc.runtime.BeanContainer.beanInstance(BeanContainer.java:25)
	at io.quarkus.opentelemetry.runtime.tracing.intrumentation.InstrumentationRecorder.setupVertxTracer(InstrumentationRecorder.java:33)
	at io.quarkus.deployment.steps.OpenTelemetryProcessor$setupVertx280172193.deploy_0(Unknown Source)
	at io.quarkus.deployment.steps.OpenTelemetryProcessor$setupVertx280172193.deploy(Unknown Source)
	... 11 more
Caused by: io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException: Unexpected configuration error
	at io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder.build(AutoConfiguredOpenTelemetrySdkBuilder.java:424)
	at io.quarkus.opentelemetry.runtime.OpenTelemetryRecorder$1.apply(OpenTelemetryRecorder.java:79)
	at io.quarkus.opentelemetry.runtime.OpenTelemetryRecorder$1.apply(OpenTelemetryRecorder.java:52)
	at io.opentelemetry.api.OpenTelemetry_2deeb341bce4a230183bf3a228841625fe2519ae_Synthetic_Bean.createSynthetic(Unknown Source)
	... 32 more
Caused by: java.lang.IllegalStateException: GlobalOpenTelemetry.set has already been called. GlobalOpenTelemetry.set must be called only once before any calls to GlobalOpenTelemetry.get. If you are using the OpenTelemetrySdk, use OpenTelemetrySdkBuilder.buildAndRegisterGlobal instead. Previous invocation set to cause of this exception.
	at io.opentelemetry.api.GlobalOpenTelemetry.set(GlobalOpenTelemetry.java:104)
	at io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder.build(AutoConfiguredOpenTelemetrySdkBuilder.java:401)
	... 35 more
Caused by: java.lang.Throwable
	at io.opentelemetry.api.GlobalOpenTelemetry.set(GlobalOpenTelemetry.java:112)
	at io.opentelemetry.api.GlobalOpenTelemetry.get(GlobalOpenTelemetry.java:82)
	at io.opentelemetry.api.GlobalOpenTelemetry.getTracer(GlobalOpenTelemetry.java:148)
	at io.opentelemetry.opencensusshim.OpenTelemetrySpanBuilderImpl.<clinit>(OpenTelemetrySpanBuilderImpl.java:55)
	at io.opentelemetry.opencensusshim.OpenTelemetryTracerImpl.spanBuilderWithExplicitParent(OpenTelemetryTracerImpl.java:42)
	at io.opencensus.trace.Tracer.spanBuilder(Tracer.java:308)
	at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:865)
	at com.google.cloud.ServiceOptions.getAppEngineProjectIdFromMetadataServer(ServiceOptions.java:507)
	at com.google.cloud.ServiceOptions.getAppEngineProjectId(ServiceOptions.java:476)
	at com.google.cloud.ServiceOptions.getDefaultProjectId(ServiceOptions.java:383)
	at io.quarkiverse.googlecloudservices.common.GcpDefaultsConfigSourceFactory.getConfigSources(GcpDefaultsConfigSourceFactory.java:36)
	at io.smallrye.config.ConfigurableConfigSource.getConfigSources(ConfigurableConfigSource.java:50)
	at io.smallrye.config.SmallRyeConfig$ConfigSources.mapLateSources(SmallRyeConfig.java:670)
	at io.smallrye.config.SmallRyeConfig$ConfigSources.<init>(SmallRyeConfig.java:554)
	at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:70)
	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:646)
	at io.quarkus.runtime.generated.Config.readConfig(Unknown Source)
	at io.quarkus.runtime.generated.Config.createRunTimeConfig(Unknown Source)
	at io.quarkus.deployment.steps.RuntimeConfigSetup.deploy(Unknown Source)
	... 11 more

Edit: quarkiverse/googlecloudservices 2.4.0 is being used

@geoand
Copy link
Contributor Author

geoand commented Aug 31, 2023

The odd thing is that I can't reproduce the problem in the integration tests of this module...

In any case, we might also need to set the otel.java.global-autoconfigure.enabled System Property in the same fashion that this PR did.

@brunobat
Copy link

Please doublecheck if the opensensus-shim lib is aligned with OTel 1.28.0... The error lines don't precisely match the current code.
I think this is a bug in the opencensus shim here: https://github.com/open-telemetry/opentelemetry-java/blob/409976a422d749ed4dd457dc60f7305417da53bf/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanBuilderImpl.java#L53

This forces the static init of the OTel SDK before we bootstrap the Quarkus SDK... This seems the same problem we had to solve with the JDBC drivers instrumentation.

@snazy
Copy link
Contributor

snazy commented Aug 31, 2023

The opencensus shim is at 1.28.0-alpha (via the Quarkus platform bom).
(Tried to force it to use 1.29.0-alpha, but then Gradle CMEs... 🤦)

@snazy
Copy link
Contributor

snazy commented Aug 31, 2023

(Just thinking loud:)
Seems that the "offending" code path goes via:

  • io.quarkiverse.googlecloudservices.common.GcpDefaultsConfigSourceFactory#getConfigSources
  • --> com.google.cloud.ServiceOptions#getDefaultProjectId
  • --> com.google.cloud.ServiceOptions#getAppEngineProjectId
  • --> com.google.cloud.ServiceOptions#getAppEngineProjectIdFromMetadataServer

Maybe it's feasible to copy the code to get the default project ID but use another HTTP client, because that seems to be the thing that implicitly creates the "offending" global tracer. I don't see an option to tell the Google HTTP client to omit tracing for a particular request.

@snazy
Copy link
Contributor

snazy commented Aug 31, 2023

An ugly workaround could be to call GlobalOpenTelemetry.resetForTest() in the finally block in GcpDefaultsConfigSourceFactory.getConfigSources().

I'm not sure whether it could work to always return a single ConfigSource from GcpDefaultsConfigSourceFactory.getConfigSources() but let that one evaluate its value lazily, so that it can run after the "legit" global tracer has been configured.

@geoand
Copy link
Contributor Author

geoand commented Aug 31, 2023

An ugly workaround could be to call GlobalOpenTelemetry.resetForTest() in the finally block in GcpDefaultsConfigSourceFactory.getConfigSources().

Setting the System Property I mentioned above should avoid the need for this

@loicmathieu
Copy link
Collaborator

@geoand if I understand it correctly, we need to set otel.java.global-autoconfigure.enabled=true in the GcpDefaultsConfigSourceFactory? Should it be erased after?

@geoand
Copy link
Contributor Author

geoand commented Aug 31, 2023

otel.java.global-autoconfigure.enabled=false actually.

But I have not tested it, I just think it will help just by looking at the code

snazy added a commit to snazy/quarkus-google-cloud-services that referenced this pull request Sep 14, 2023
As described in [this issue](quarkusio/quarkus#35500), Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.

`com.google.cloud.ServiceOptions` is used by the extension to get the Google cloud default-project-ID, which uses Google's HTTP client, which uses OpenCensus, which triggers an initialization of OpenTelemetry, which conflicts with the OTel init from Quarkus.

This change updates `GcpDefaultsConfigSourceFactory` to use Java's HTTP client.

See also quarkiverse#487, the alternatives mentioned in [this comment](quarkiverse#487 (comment)) to implement a `ConfigSource` and in [this comment](quarkiverse#487 (comment)) to set `otel.java.global-autoconfigure.enabled=false)` end in the same OpenCensus/OpenTracing init race.

Fixes quarkusio/quarkus#35500
snazy added a commit to snazy/quarkus-google-cloud-services that referenced this pull request Sep 14, 2023
As described in [this issue](quarkusio/quarkus#35500), Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.

`com.google.cloud.ServiceOptions` is used by the extension to get the Google cloud default-project-ID, which uses Google's HTTP client, which uses OpenCensus, which triggers an initialization of OpenTelemetry, which conflicts with the OTel init from Quarkus.

This change updates `GcpDefaultsConfigSourceFactory` to use Java's HTTP client.

See also quarkiverse#487, the alternatives mentioned in [this comment](quarkiverse#487 (comment)) to implement a `ConfigSource` and in [this comment](quarkiverse#487 (comment)) to set `otel.java.global-autoconfigure.enabled=false)` end in the same OpenCensus/OpenTracing init race.

Fixes quarkusio/quarkus#35500
snazy added a commit to snazy/quarkus-google-cloud-services that referenced this pull request Sep 14, 2023
As described in [this issue](quarkusio/quarkus#35500), Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.

`com.google.cloud.ServiceOptions` is used by the extension to get the Google cloud default-project-ID, which uses Google's HTTP client, which uses OpenCensus, which triggers an initialization of OpenTelemetry, which conflicts with the OTel init from Quarkus.

This change updates `GcpDefaultsConfigSourceFactory` to use Java's HTTP client.

See also quarkiverse#487, the alternatives mentioned in [this comment](quarkiverse#487 (comment)) to implement a `ConfigSource` and in [this comment](quarkiverse#487 (comment)) to set `otel.java.global-autoconfigure.enabled=false)` end in the same OpenCensus/OpenTracing init race.

Fixes quarkusio/quarkus#35500
snazy added a commit to snazy/quarkus-google-cloud-services that referenced this pull request Sep 18, 2023
As described in [this issue](quarkusio/quarkus#35500), Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.

`com.google.cloud.ServiceOptions` is used by the extension to get the Google cloud default-project-ID, which uses Google's HTTP client, which uses OpenCensus, which triggers an initialization of OpenTelemetry, which conflicts with the OTel init from Quarkus.

This change updates `GcpDefaultsConfigSourceFactory` to use Java's HTTP client.

See also quarkiverse#487, the alternatives mentioned in [this comment](quarkiverse#487 (comment)) to implement a `ConfigSource` and in [this comment](quarkiverse#487 (comment)) to set `otel.java.global-autoconfigure.enabled=false)` end in the same OpenCensus/OpenTracing init race.

Fixes quarkusio/quarkus#35500
snazy added a commit to snazy/quarkus-google-cloud-services that referenced this pull request Sep 18, 2023
As described in [this issue](quarkusio/quarkus#35500), Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.

`com.google.cloud.ServiceOptions` is used by the extension to get the Google cloud default-project-ID, which uses Google's HTTP client, which uses OpenCensus, which triggers an initialization of OpenTelemetry, which conflicts with the OTel init from Quarkus.

This change updates `GcpDefaultsConfigSourceFactory` to use Java's HTTP client.

See also quarkiverse#487, the alternatives mentioned in [this comment](quarkiverse#487 (comment)) to implement a `ConfigSource` and in [this comment](quarkiverse#487 (comment)) to set `otel.java.global-autoconfigure.enabled=false)` end in the same OpenCensus/OpenTracing init race.

Fixes quarkusio/quarkus#35500
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.

OpenTelemetry / OpenCensus race (since Quarkus 3.3.0)
5 participants