-
Notifications
You must be signed in to change notification settings - Fork 159
declarative config: add declaravite config bridge, gcp auth, span stacktrace #2020
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
base: main
Are you sure you want to change the base?
Conversation
🔧 The result from spotlessApply was committed to the PR branch. |
...uth-extension/src/main/java/io/opentelemetry/contrib/gcp/auth/GcpAuthCustomizerProvider.java
Outdated
Show resolved
Hide resolved
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.
notes from discussion with @jaydeluca
* string_key: value | ||
* </pre> | ||
*/ | ||
final class DeclarativeConfigPropertiesBridge implements ConfigProperties { |
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.
the bridge is also part of open-telemetry/opentelemetry-java-instrumentation#14184 - once it's reviewed there it will be updated here
but that can also be done after this PR is merged
...tacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceComponentProvider.java
Show resolved
Hide resolved
...tacktrace/src/main/java/io/opentelemetry/contrib/stacktrace/StackTraceComponentProvider.java
Outdated
Show resolved
Hide resolved
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.
Thanks a lot for the the PR @zeitlinger ! 💯
I left some suggestions and a question. Let me know your thoughts on them.
gcp-auth-extension/src/main/java/io/opentelemetry/contrib/gcp/auth/ConfigurableOption.java
Outdated
Show resolved
Hide resolved
.../main/java/io/opentelemetry/contrib/gcp/auth/GcpAuthAutoConfigurationCustomizerProvider.java
Show resolved
Hide resolved
.../main/java/io/opentelemetry/contrib/gcp/auth/GcpAuthAutoConfigurationCustomizerProvider.java
Outdated
Show resolved
Hide resolved
...uth-extension/src/main/java/io/opentelemetry/contrib/gcp/auth/GcpAuthCustomizerProvider.java
Outdated
Show resolved
Hide resolved
...uth-extension/src/main/java/io/opentelemetry/contrib/gcp/auth/GcpAuthCustomizerProvider.java
Outdated
Show resolved
Hide resolved
return list; | ||
} | ||
|
||
private static void addAuth( |
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.
Question: The addAuth
method seems to add a fixed key-value pair to a list of headers associated with the span exporter / metric exporter.
In this case, I don't see how a token refresh for the auth headers work or am I missing something here ?
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.
great catch - I missed the refresh part completely - I have to check how this is even supposed to work
|
||
GcpAuthCustomizerProvider.customize(model, credentials, properties); | ||
|
||
String header = |
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.
An additional header that is important to check here is the auth header.
[name=Authorization, value=bearer fake-credential-token]
.
Sample setup:
mockedRequestMetadata =
ImmutableMap.of(
"Authorization",
Collections.singletonList("Bearer fake-access-token"),
QUOTA_USER_PROJECT_HEADER,
Collections.singletonList("qp"));
Mockito.when(mockedGoogleCredentials.getRequestMetadata()).thenReturn(mockedRequestMetadata);
PS: I can update the testing later as well.
…auth/GcpAuthAutoConfigurationCustomizerProvider.java Co-authored-by: Pranav Sharma <[email protected]>
…auth/GcpAuthCustomizerProvider.java Co-authored-by: Pranav Sharma <[email protected]>
Blocked by open-telemetry/opentelemetry-configuration#257 for GCP auth
Fixes #2012
Fixes #2028