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.

Sign up for GitHub

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

feat(gax): append cred-type header for auth metrics #3186

Merged
merged 14 commits into from
Oct 4, 2024
Merged

feat(gax): append cred-type header for auth metrics #3186

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2dbd2d8
feat: expose credential type as part of x-goog-api-client header.
zhumin8 Sep 3, 2024
ee2624c
test: add test correctly appends cred-type to header.
zhumin8 Sep 10, 2024
fe3ed54
use enum for credential types.
zhumin8 Sep 13, 2024
0cd0ad8
update type.
zhumin8 Oct 1, 2024
b1dd5b0
fix naming and comment.
zhumin8 Oct 2, 2024
781f161
Merge branch 'main' into auth-metrics
zhumin8 Oct 2, 2024
01adbde
Merge branch 'main' into auth-metrics
zhumin8 Oct 3, 2024
32f8934
fix credential is null case.
zhumin8 Oct 3, 2024
51bed23
add test case.
zhumin8 Oct 3, 2024
0424fd2
update test, extract test setup steps.
zhumin8 Oct 3, 2024
0e69f72
Merge branch 'main' into auth-metrics
zhumin8 Oct 4, 2024
b50896a
address feedback: encapsulate credentials metrics logic inside getHea…
zhumin8 Oct 4, 2024
6e60633
address feedback.
zhumin8 Oct 4, 2024
33e24d9
update comment.
zhumin8 Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public static ClientContext create(StubSettings settings) throws IOException {

String settingsGdchApiAudience = settings.getGdchApiAudience();
Credentials credentials = settings.getCredentialsProvider().getCredentials();
String credentialType = credentials.getCredentialType().getLabel();
boolean usingGDCH = credentials instanceof GdchCredentials;
if (usingGDCH) {
// Can only determine if the GDC-H is being used via the Credentials. The Credentials object
Expand Down Expand Up @@ -223,7 +224,8 @@ public static ClientContext create(StubSettings settings) throws IOException {
if (transportChannelProvider.needsExecutor() && settings.getExecutorProvider() != null) {
transportChannelProvider = transportChannelProvider.withExecutor(backgroundExecutor);
}
Map<String, String> headers = getHeadersFromSettings(settings);
Map<String, String> headers =
getHeadersFromSettingsAndAppendCredentialType(settings, credentialType);
if (transportChannelProvider.needsHeaders()) {
transportChannelProvider = transportChannelProvider.withHeaders(headers);
}
Expand Down Expand Up @@ -293,9 +295,10 @@ public static ClientContext create(StubSettings settings) throws IOException {

/**
* Getting a header map from HeaderProvider and InternalHeaderProvider from settings with Quota
* Project Id.
* Project Id. Then append credential type to x-goog-api-client header.
*/
private static Map<String, String> getHeadersFromSettings(StubSettings settings) {
private static Map<String, String> getHeadersFromSettingsAndAppendCredentialType(
StubSettings settings, String credentialType) {
// Resolve conflicts when merging headers from multiple sources
Map<String, String> userHeaders = settings.getHeaderProvider().getHeaders();
Map<String, String> internalHeaders = settings.getInternalHeaderProvider().getHeaders();
Expand All @@ -322,6 +325,9 @@ private static Map<String, String> getHeadersFromSettings(StubSettings settings)
effectiveHeaders.putAll(userHeaders);
effectiveHeaders.putAll(conflictResolution);

effectiveHeaders.computeIfPresent(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think handwritten libraries and/or customers can override credentials on request level, can we do more investigation? Maybe we can add this header on request level not client level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update on this: Consider this is a relatively niche case, to unblock this current change, I will look into this separately and create a followup if needed. Updated description to reflect.

ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),
(key, value) -> value + " cred-type/" + credentialType);
return ImmutableMap.copyOf(effectiveHeaders);
}

Expand Down
33 changes: 33 additions & 0 deletions gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.api.core.ApiClock;
import com.google.api.gax.core.BackgroundResource;
Expand All @@ -52,6 +53,7 @@
import com.google.api.gax.rpc.testing.FakeClientSettings;
import com.google.api.gax.rpc.testing.FakeStubSettings;
import com.google.api.gax.rpc.testing.FakeTransportChannel;
import com.google.auth.CredentialType;
import com.google.auth.Credentials;
import com.google.auth.oauth2.ComputeEngineCredentials;
import com.google.auth.oauth2.GdchCredentials;
Expand Down Expand Up @@ -679,6 +681,37 @@ void testUserAgentConcat() throws Exception {
.containsEntry("user-agent", "user-supplied-agent internal-agent");
}

@Test
void testApiClientHeaderAppendsCredType() throws Exception {
TransportChannelProvider transportChannelProvider =
new FakeTransportProvider(
FakeTransportChannel.create(new FakeChannel()),
null,
true,
null,
null,
DEFAULT_ENDPOINT);
GoogleCredentials googleCredentials = Mockito.mock(GoogleCredentials.class);
when(googleCredentials.getCredentialType()).thenReturn(CredentialType.USER_CREDENTIALS);

ClientSettings.Builder builder =
new FakeClientSettings.Builder()
.setExecutorProvider(
FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class)))
.setTransportChannelProvider(transportChannelProvider)
.setCredentialsProvider(FixedCredentialsProvider.create(googleCredentials));

builder.setInternalHeaderProvider(
FixedHeaderProvider.create("x-goog-api-client", "internal-agent"));

ClientContext clientContext = ClientContext.create(builder.build());
FakeTransportChannel transportChannel =
(FakeTransportChannel) clientContext.getTransportChannel();

assertThat(transportChannel.getHeaders())
.containsEntry("x-goog-api-client", "internal-agent cred-type/u");
}

private static String endpoint = "https://foo.googleapis.com";
private static String mtlsEndpoint = "https://foo.mtls.googleapis.com";

Expand Down
Loading