Skip to content

fix: record the credentials request metric with the token request's attributes - #410

Open
cvas22 wants to merge 2 commits into
openfga:mainfrom
cvas22:fix/149-credentials-request-attributes
Open

cvas22 wants to merge 2 commits into
openfga:mainfrom
cvas22:fix/149-credentials-request-attributes

Conversation

@cvas22

@cvas22 cvas22 commented Nov 5, 2026 •

Copy link
Copy Markdown

Description

OAuth2Client recorded the fga-client.credentials.request counter with a hard-coded empty attributes map (#149, #208), so the metric had none of the configured attributes: no host, URL, method, status code or resend count.

HttpRequestAttempt already collects those attributes for the token request, but they never made it back to getAccessToken(), where the counter was recorded. The counter is now recorded in exchangeToken(), right after the token request succeeds, with attempt.getTelemetryAttributes(). As before, it's recorded once per successful exchange, and the attribute filtering from the telemetry configuration still applies in Metrics.

Fixes #149
Fixes #208

Testing

New credentialsRequestMetricHasTheTokenRequestAttributes in OAuth2ClientTest: it intercepts the Metrics instance with Mockito's mockConstruction, exchanges a token, and checks that credentialsRequest is called once with the token request's http.host, http.request.method and http.response.status_code. That avoids adding the OpenTelemetry SDK as a test dependency.

I haven't run the Gradle build locally, so I'm relying on CI for compilation and the test run.

Summary by CodeRabbit

  • Bug Fixes
    • Credential request reporting now reflects successful token exchanges and excludes requests that return a cached access token. This provides a more accurate view of token-request activity without changing how access tokens are obtained or used.

…ttributes

OAuth2Client recorded fga-client.credentials.request with a hard-coded
empty attributes map, so the metric had no host, status code or other
attributes. Record it in exchangeToken with the attributes that
HttpRequestAttempt collected for the token request.

Fixes openfga#149
Fixes openfga#208

Signed-off-by: Srinivas Chippagiri <[email protected]>
@cvas22
cvas22 requested a review from a team as a code owner November 5, 2026 09:39
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

OAuth2Client now records credential-request metrics with telemetry attributes from the successful token-exchange HTTP attempt. A new test checks the host, response status, and request method attributes.

Changes

Credential request metrics

Layer / File(s) Summary
Record token exchange attributes
src/main/java/dev/openfga/sdk/api/auth/OAuth2Client.java, src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java
exchangeToken records credentials-request metrics using the successful HTTP attempt’s telemetry attributes. The test verifies the host, response status, and request method. The cached-token path no longer records this metric.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • openfga/java-sdk#399: Changes the export type for http.response.status_code, an attribute that this PR passes to the credentials-request metric.

Suggested reviewers: jimmyjames, cportcvent, ewanharris

Merge Risk: Minimal · up to 93091

This change records the credentials-request metric with the token request's attributes instead of an empty map. No actionable merge-blocking risk was found. CI should confirm the build and the new test.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title says the credentials request metric now uses the token request's attributes. The changeset does this: exchangeToken() records fga-client.credentials.request after a successful attempt and pa…
Linked Issues check Passed [#149, #208] Both issues require the fga-client.credentials.request counter to carry the configured or applicable attributes instead of a hard-coded empty map. At the reviewed head, OAuth2Client.excha…
Out of Scope Changes check Passed The OAuth2Client.java changes are limited to the credentials-request metric path: removing the unused HashMap import, removing the metric call from getAccessToken(), and recording the metric in exchan…
Full details: Title check

Explanation

The title says the credentials request metric now uses the token request's attributes. The changeset does this: exchangeToken() records fga-client.credentials.request after a successful attempt and passes attempt.getTelemetryAttributes(), and the test checks the host, method, and status attributes. The title is specific and matches the main change. It uses the conventional 'fix:' prefix.

Full details: Linked Issues check

Explanation

[#149, #208] Both issues require the fga-client.credentials.request counter to carry the configured or applicable attributes instead of a hard-coded empty map. At the reviewed head, OAuth2Client.exchangeToken() records the counter inside the thenApply callback of attempt.attemptHttpRequest(), using attempt.getTelemetryAttributes(). The map is populated by HttpRequestAttempt.attemptHttpRequest() (HTTP host, URL scheme, URL full, request method, user agent) and by processHttpResponse() (Attributes.fromHttpResponse, resend count). The removed hard-coded new HashMap<>() call in getAccessToken() no longer exists. The telemetry filtering in Metrics is unchanged, so configured defaults still apply. The new test credentialsRequestMetricHasTheTokenRequestAttributes in OAuth2ClientTest checks host, method and status code on the credentialsRequest call. The test was not run in this review, so its pass status is unknown, but the code path that the requirements need is present and reads correctly.

Full details: Out of Scope Changes check

Explanation

The OAuth2Client.java changes are limited to the credentials-request metric path: removing the unused HashMap import, removing the metric call from getAccessToken(), and recording the metric in exchangeToken() with the attempt's attributes. The test change adds Mockito construction utilities and a single test for the same metric. These support the #149 and #208 objectives. No unrelated behavior change was found in the reviewed diff. The whole-PR diff was compared from merge base 523e06f to head 93091f6.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fa226ccf-cf0f-4872-a03c-7f3f4ea79514
Commits

Reviewing files that changed from the base of the PR and between 523e06f and 93091f6.

Files selected for processing: 2

  • src/main/java/dev/openfga/sdk/api/auth/OAuth2Client.java
  • src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

…equest-attributes

# Conflicts:
#	src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty Telemetry Attributes in OAuth2Client telemetry attributes not sent on token exchange request

1 participant