FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: add metric headers by zhumin8 · Pull Request #1503 · googleapis/google-auth-library-java · GitHub

This repository was archived by the owner on May 12, 2026. It is now read-only.
/ google-auth-library-java Public archive

feat: add metric headers - #1503

Merged
zhumin8 merged 27 commits into
mainfrom
metrics
Oct 2, 2024
ConversationCommits (27)ChecksFiles changed
Merged

feat: add metric headers#1503
zhumin8 merged 27 commits into
mainfrom
metrics

Conversation

zhumin8 commented Sep 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

context: b/339259830 and go/send-auth-metrics-java

Changes include:

  • expose Credentials type via getMetricsCredentialType(). Override this method for UserCredentials, ServiceAccountCredentials, ImpersonatedCredentials, and ComputeEngineCredentials. This is used in both token request and token usage flows.
  • add metric headers for each of the in-scope token requests. Below are examples of each request flow with added metrics:
    • User credentials request (at/id): “gl-java/19.0.1 auth/1.24.3 cred-type/u”
    • SA credentials, VM credentials or Impersonated credentials requests (at/id): “gl-java/19.0.1 auth/1.24.3 auth-request-type/at cred-type/sa”
    • MDS ping (This is used in ADC during the credential detection): “gl-java/19.0.1 auth/1.24.3 auth-request-type/mds”
  • What is not tracked: ComputeEngineCredentials getUniverseDomain and getAccount does not send metrics header; TPC flows does not send metrics headers.

Related pr: adding for cred_type for token usage requests
googleapis/sdk-platform-java#3186

product-auto-label Bot added the size: l Pull request size is large. label Sep 13, 2024
zhumin8 marked this pull request as ready for review September 25, 2024 18:08
zhumin8 requested review from a team September 25, 2024 18:08
westarle reviewed Sep 27, 2024
View reviewed changes
Comment thread credentials/java/com/google/auth/CredentialType.java Outdated

package com.google.auth;

public enum CredentialType {

westarle Sep 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.)

lqiu96 reviewed Sep 27, 2024
View reviewed changes
Comment thread credentials/java/com/google/auth/Credentials.java Outdated
lqiu96 reviewed Sep 27, 2024
View reviewed changes
Comment thread credentials/java/com/google/auth/Credentials.java Outdated
lqiu96 reviewed Sep 27, 2024
View reviewed changes
Comment thread credentials/java/com/google/auth/Credentials.java Outdated
zhumin8 requested a review from westarle September 30, 2024 21:45
zhumin8 requested a review from lqiu96 October 1, 2024 17:22
lqiu96 reviewed Oct 1, 2024
View reviewed changes
Comment thread oauth2_http/javatests/com/google/auth/oauth2/TestUtils.java
assertTrue(headers.containsKey(MetricsUtils.API_CLIENT_HEADER));
String actualMetricsValue = headers.get(MetricsUtils.API_CLIENT_HEADER).get(0);
String expectedMetricsValue;
if (requestType.equals("untracked")) {

lqiu96 Oct 1, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nit: Can we use the enum value here?

zhumin8 Oct 1, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

lqiu96 reviewed Oct 1, 2024
View reviewed changes
Comment thread oauth2_http/javatests/com/google/auth/oauth2/TestUtils.java Outdated
MetricsUtils.getLanguageAndAuthLibraryVersions(),
MetricsUtils.CRED_TYPE,
credentialType);
} else if (credentialType.isEmpty()) {

lqiu96 Oct 1, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why isEmpty() and not check for DO NOT SEND?

zhumin8 Oct 1, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

updating to be consistent with requestType above.

lqiu96 approved these changes Oct 1, 2024
View reviewed changes

lqiu96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. Added two questions in the tests. Thanks for addressing the comments and questions!

sonarqubecloud Bot commented Oct 2, 2024

Copy link
Copy Markdown

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

zhumin8 merged commit 7f0c1d3 into main Oct 2, 2024
zhumin8 deleted the metrics branch October 2, 2024 15:04
release-please Bot mentioned this pull request Oct 2, 2024
Merged
zhumin8 mentioned this pull request Oct 3, 2024
Merged
zhumin8 added a commit to googleapis/sdk-platform-java that referenced this pull request Oct 4, 2024
feat(gax): append cred-type header for auth metrics (#3186)
ca3ec24 
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.
release-please Bot mentioned this pull request Dec 11, 2025
Merged
chingor13 pushed a commit to googleapis/google-cloud-java that referenced this pull request Mar 24, 2026
feat(gax): append cred-type header for auth metrics (#3186)
415bfa2 
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

sai-sunder-s Awaiting requested review from sai-sunder-s
westarle Awaiting requested review from westarle
1 more reviewer
lqiu96 lqiu96 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size: l Pull request size is large.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL