feat: add metric headers #1503
feat: add metric headers #1503
Conversation
|
|
||
| package com.google.auth; | ||
|
|
||
| public enum CredentialType { |
There was a problem hiding this comment.
Since these are strictly defined by our metrics implementation, can we make that more clear everywhere?
For instance we might call this "CredentialNameForMetrics" (or something better than that).
That way we know that we can't add enum items without a definition that is established elsewhere, and there won't be Credentials that have a CredentialType of Unknown (when we actually do know the type, we just don't want to score it for metrics.)
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ImpersonatedCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/MockMetadataServerTransport.java
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
| assertTrue(headers.containsKey(MetricsUtils.API_CLIENT_HEADER)); | ||
| String actualMetricsValue = headers.get(MetricsUtils.API_CLIENT_HEADER).get(0); | ||
| String expectedMetricsValue; | ||
| if (requestType.equals("untracked")) { |
There was a problem hiding this comment.
nit: Can we use the enum value here?
There was a problem hiding this comment.
note requestType and credentialType here are String values. They represent part of the expected value in this utils method. I am leaning towards checking against plain text here
| MetricsUtils.getLanguageAndAuthLibraryVersions(), | ||
| MetricsUtils.CRED_TYPE, | ||
| credentialType); | ||
| } else if (credentialType.isEmpty()) { |
There was a problem hiding this comment.
Why isEmpty() and not check for DO NOT SEND?
There was a problem hiding this comment.
updating to be consistent with requestType above.
|
This is POC change in gax-java for auth metrics requirements on token usage. See go/googleapis-auth-metric-design for context. [Credentials](https://github.com/googleapis/google-auth-library-java/blob/main/credentials/java/com/google/auth/Credentials.java) will expose `getMetricsCredentialType()` method, this change appends it to existing `x-goog-api-client` header Note: Currently implement in gax at client level. There are 2 edge cases not covered and will create followups for: if handwritten library overrides credentials at rpc level; If handwritten library does not build on gax. (ref: b/370039645, b/370038458) related change in `google-auth-library`googleapis/google-auth-library-java#1503 included in 1.28.0.
context: b/339259830 and go/send-auth-metrics-java
Changes include:
Credentialstype viagetMetricsCredentialType(). Override this method for UserCredentials, ServiceAccountCredentials, ImpersonatedCredentials, and ComputeEngineCredentials. This is used in both token request and token usage flows.Related pr: adding for cred_type for token usage requests
googleapis/sdk-platform-java#3186