Do not use Google's HTTP client to get the default project ID #502
Conversation
|
/cc @geoand |
|
Manually verified that Quarkus starts w/ the OpenCensus-shim. |
|
Also manually verified (in |
| .header("Metadata-Flavor", "Google").build(); | ||
| HttpResponse.BodyHandler<String> bodyHandler = HttpResponse.BodyHandlers.ofString(); | ||
| HttpResponse<String> response = client.send(request, bodyHandler); | ||
| return headerContainsMetadataFlavor(response) ? response.body() : ""; |
There was a problem hiding this comment.
Shouldn't it respond null and not empty if not metadata (as the remaining if chain check for nullity)
| } | ||
|
|
||
| @Test | ||
| void staticOpenCensusOpenTelemetryInit() { |
There was a problem hiding this comment.
What is the purpose of this test? Does it crash without the change (so it's a non-regression test?)
There was a problem hiding this comment.
Yes, fails without the prod code change. Added a comment.
| @@ -21,6 +21,7 @@ | |||
| <compiler-plugin.version>3.8.1</compiler-plugin.version> | |||
| <enforcer-plugin.version>3.4.1</enforcer-plugin.version> | |||
| <assertj.version>3.24.2</assertj.version> | |||
| <opentelemetry-alpha.version>1.28.0-alpha</opentelemetry-alpha.version> | |||
There was a problem hiding this comment.
| <opentelemetry-alpha.version>1.28.0-alpha</opentelemetry-alpha.version> | |
| <opentelemetry.version>1.28.0-alpha</opentelemetry.version> |
And use the same under
There was a problem hiding this comment.
Took the approach that is used in Quarkus, differing between the "alpha" and "non-alpha" artifacts.
snazy
force-pushed
the
default-project-id-no-google-http
branch
from
8ec386e
to
80d7368
Compare
common/runtime/pom.xml
Outdated
| <!-- <plugin>--> | ||
| <!-- <artifactId>maven-surefire-plugin</artifactId>--> | ||
| <!-- <configuration>--> | ||
| <!-- <systemProperties>--> | ||
| <!-- <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>--> | ||
| <!-- <gcloud.test>true</gcloud.test>--> | ||
| <!-- </systemProperties>--> | ||
| <!-- </configuration>--> | ||
| <!-- </plugin>--> |
There was a problem hiding this comment.
Why do you comment this? If it's not needed you can remove it but as far as I remember this is needed to have logs property formatted on tests
There was a problem hiding this comment.
Left over (sorry)
I initially added it, because I was thinking of some @QuarkusTest, but then realized that it's good enough to check the static-init race.
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
force-pushed
the
default-project-id-no-google-http
branch
from
80d7368
to
14e3bc6
Compare
|
@loicmathieu I personally don't mind (can wait for 3.4, if the 3.4 platform-bom references it), but could be useful for other users to have this also in 3.3.x. |
|
@all-contributors please add @snazy for code |
|
I've put up a pull request to add @snazy! 🎉 |
|
2.5.0 is released with the fix. |
As described in this issue, Quarkus w/ the Google clooud services and OpenCensus shim does not startup due to a static initialization issue.
com.google.cloud.ServiceOptionsis 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
GcpDefaultsConfigSourceFactoryto use Java's HTTP client.See also #487, the alternatives mentioned in this comment to implement a
ConfigSourceand in this comment to setotel.java.global-autoconfigure.enabled=false)end in the same OpenCensus/OpenTracing init race.Fixes quarkusio/quarkus#35500