Javaagent auth extension #4
Conversation
psx95
force-pushed
the
javaagent-auth-extension
branch
from
50e113e to
a55d826
Compare
...gcp-auth/src/main/java/com/google/cloud/opentelemetry/extension/auth/ConfigurableOption.java
Show resolved
Hide resolved
...om/google/cloud/opentelemetry/extension/auth/GcpAuthAutoConfigurationCustomizerProvider.java
Outdated
Show resolved
Hide resolved
| * @see AutoConfigurationCustomizerProvider | ||
| * @see GoogleCredentials | ||
| */ | ||
| @AutoService(AutoConfigurationCustomizerProvider.class) |
There was a problem hiding this comment.
It might be better to split the provider which adds resource attributes from the one which adds auth headers, so users can enable them individually.
There was a problem hiding this comment.
In this case, the gcp.project_id is essential for successful export, so I don't think giving users individual control would be beneficial.
| The OpenTelemetry Java Agent Extension can be easily added to any Java application by modifying the startup command to the application. | ||
| For more information on Extensions, see the [documentation here](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/examples/extension/README.md). | ||
|
|
||
| Below is a snippet showing how to add the extension to a Java application using the Gradle build system. |
There was a problem hiding this comment.
Do users typically configure the agent through gradle?
There was a problem hiding this comment.
I'm not sure about the typical use case, however in this case, the command line args would look extremely similar.
The integration test showcases the agent configuration directly on command line.
There was a problem hiding this comment.
Use cases vary as broadly as the ecosystem does.
I think it's good to include this here.
| throw new GoogleAuthException(Reason.FAILED_ADC_REFRESH, e); | ||
| } | ||
| gcpHeaders.put(QUOTA_USER_PROJECT_HEADER, credentials.getQuotaProjectId()); | ||
| gcpHeaders.put("Authorization", "Bearer " + credentials.getAccessToken().getTokenValue()); |
There was a problem hiding this comment.
Since the map is just made once, it seems like this won't support refresh?
There was a problem hiding this comment.
No, the function getRequiredHeaderMap is invoked through a header supplier which means it gets recreated during export.
There might be some performance concerns about recreating Map on every export, which I intend to discuss in the SIG.
| throw new RuntimeException(e); | ||
| } | ||
| backendServer.start(); | ||
| Process p = |
There was a problem hiding this comment.
Got it working now - for some unknown reason, I could not get the @WithSpan annotation to work, so I switched to a different library which I know is auto-instrumented.
PS - The tests on CI are failing due to absence of GCP credentials, still need to work around that, but the tests are passing locally.
The com.sun.net.httpserver does not seem to be auto-instrumented by the Java Agent and @WithSpan was not producing a trace span for the handler. This commit changes the test app to use Apache HttpServer client which is auto-instrumented by the agent.
psx95
force-pushed
the
javaagent-auth-extension
branch
from
a55d826 to
c6049e3
Compare
Yes, this was manually tested against the trace endpoint. |
psx95
force-pushed
the
javaagent-auth-extension
branch
from
3c61a96 to
250b108
Compare
psx95
force-pushed
the
javaagent-auth-extension
branch
from
2dd2e13 to
122c7d7
Compare
psx95
force-pushed
the
javaagent-auth-extension
branch
2 times, most recently
from
7b3ef22 to
942977b
Compare
psx95
force-pushed
the
javaagent-auth-extension
branch
from
942977b to
a267bbb
Compare
psx95
force-pushed
the
javaagent-auth-extension
branch
2 times, most recently
from
9639551 to
d7d4bf9
Compare
psx95
force-pushed
the
javaagent-auth-extension
branch
from
d7d4bf9 to
6036833
Compare
DO NOT MERGE
This PR is only to gather reviews/feedback.