Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (38.05%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #310 +/- ##
============================================
+ Coverage 38.02% 38.05% +0.03%
- Complexity 1257 1259 +2
============================================
Files 198 198
Lines 7643 7646 +3
Branches 885 885
============================================
+ Hits 2906 2910 +4
Misses 4598 4598
+ Partials 139 138 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: OpenFGA's Space OpenTelemetry Metrics IntegrationView Suggested Changes@@ -4,13 +4,13 @@
---
### Supported Metrics
-The JavaScript and Java SDKs emit the following OpenTelemetry metrics by default. The Go SDK requires explicit configuration to enable `fga-client.http_request.duration`.
+The JavaScript SDK emits the following OpenTelemetry metrics by default. The Java SDK requires explicit configuration to enable `fga-client.request.count`. The Go SDK requires explicit configuration to enable `fga-client.http_request.duration`.
| Metric Name | Type | Enabled by Default | Description |
|--------------------------------------|-----------|--------------------|----------------------------------------------------------------------------------------------|
| `fga-client.request.duration` | Histogram | Yes | Total request time for FGA requests (milliseconds) |
| `fga-client.query.duration` | Histogram | Yes | Time taken by the FGA server to process and evaluate the request (milliseconds) |
-| `fga-client.request.count` | Counter | Yes | Total number of HTTP requests made by the SDK |
+| `fga-client.request.count` | Counter | Yes (JS/Go), No (Java) | Total number of HTTP requests made to the FGA server |
| `fga-client.credentials.request` | Counter | Yes | Total number of new token requests initiated using the Client Credentials flow |
| `fga-client.http_request.duration` | Histogram | Yes (JS/Java), No (Go) | Time taken for a single HTTP request to complete, including retries (milliseconds) |
@@ -116,6 +116,8 @@
#### Java SDK
Configure telemetry by supplying a custom `TelemetryConfiguration` when constructing the client. Map metrics to attributes to control which telemetry data is emitted.
+The `fga-client.request.count` metric is disabled by default in the Java SDK and must be explicitly enabled. To enable it, include `Counters.REQUEST_COUNT` in your telemetry configuration:
+
```java
import dev.openfga.sdk.api.configuration.TelemetryConfiguration;
import dev.openfga.sdk.telemetry.Attribute;
@@ -145,6 +147,7 @@
Map<Metric, Map<Attribute, Optional<Object>>> metrics = new HashMap<>();
metrics.put(Counters.CREDENTIALS_REQUEST, attributes);
+ metrics.put(Counters.REQUEST_COUNT, attributes); // Explicitly enable request count metric
metrics.put(Histograms.QUERY_DURATION, attributes);
metrics.put(Histograms.REQUEST_DURATION, attributes);
@@ -272,6 +275,7 @@
- The Java SDK supports both manual configuration and Java Agent (automatic instrumentation with zero code changes).
- High-cardinality attributes (like `fga-client.user`) are disabled by default to avoid excessive costs with some metric collectors. Enable only if necessary.
- In the Go SDK, `fga-client.http_request.duration` is disabled by default. Enable it through telemetry configuration if per-HTTP request monitoring is required.
+- In the Java SDK, `fga-client.request.count` is disabled by default and must be explicitly enabled via `TelemetryConfiguration`.
---
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new OpenTelemetry counter intended to track HTTP request volume at the SDK’s HTTP layer, with supporting configuration/tests and changelog entry.
Changes:
- Introduces
Counters.REQUEST_COUNT(fga-client.request.count) and aMetrics.requestCount(...)helper. - Emits the new metric from
HttpRequestAttemptalongside existing request-duration telemetry. - Adds tests asserting the metric is disabled by default and can be enabled explicitly; updates
CHANGELOG.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/dev/openfga/sdk/telemetry/Counters.java | Adds the new REQUEST_COUNT counter definition. |
| src/main/java/dev/openfga/sdk/telemetry/Metrics.java | Adds requestCount(...) API to record the counter. |
| src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java | Records the new counter during HTTP response processing. |
| src/test/java/dev/openfga/sdk/telemetry/CountersTest.java | Verifies counter name/description. |
| src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java | Verifies requestCount is null unless explicitly configured. |
| src/test/java/dev/openfga/sdk/api/configuration/TelemetryConfigurationTest.java | Verifies default config does not enable REQUEST_COUNT and that it can be enabled. |
| CHANGELOG.md | Documents the newly added counter metric and that it’s disabled by default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/java/dev/openfga/sdk/api/configuration/TelemetryConfigurationTest.java
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java (1)
172-223:⚠️ Potential issue | 🟠 Major
requestCountis undercounted for HTTP error responses.Line 222 emits the metric only in the success path, but Line 192 returns early for error responses, so failed HTTP responses are never counted. For a “total requests made” counter, this should increment once per response regardless of status.
💡 Proposed fix
private CompletableFuture<ApiResponse<T>> processHttpResponse( HttpResponse<String> response, int retryNumber, Throwable previousError) { + addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials())); + + if (retryNumber > 0) { + addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber)); + } + + telemetry.metrics().requestCount(1L, this.getTelemetryAttributes()); + Optional<FgaError> fgaError = FgaError.getError(name, request, configuration, response, previousError); if (fgaError.isPresent()) { FgaError error = fgaError.get(); int statusCode = error.getStatusCode(); @@ - addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials())); - - if (retryNumber > 0) { - addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber)); - } - if (response.headers() .firstValue(FgaConstants.QUERY_DURATION_HEADER_NAME) .isPresent()) { @@ Double requestDuration = (double) (System.currentTimeMillis() - requestStarted); telemetry.metrics().requestDuration(requestDuration, this.getTelemetryAttributes()); - telemetry.metrics().requestCount(1L, this.getTelemetryAttributes()); return deserializeResponse(response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java` around lines 172 - 223, processHttpResponse currently returns early on error (when FgaError.isPresent()) so telemetry.metrics().requestCount(...) is only executed on the success path; increment the request counter for every response by calling telemetry.metrics().requestCount(1L, this.getTelemetryAttributes()) before the FgaError check (or ensure it's invoked in the error branch prior to returning/CompletableFuture.failedFuture), and likewise ensure request duration (requestStarted -> now) is recorded for error responses if you want consistent timing metrics; update references in processHttpResponse and handleHttpErrorRetry as needed so every HTTP response triggers telemetry.metrics().requestCount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java`:
- Around line 172-223: processHttpResponse currently returns early on error
(when FgaError.isPresent()) so telemetry.metrics().requestCount(...) is only
executed on the success path; increment the request counter for every response
by calling telemetry.metrics().requestCount(1L, this.getTelemetryAttributes())
before the FgaError check (or ensure it's invoked in the error branch prior to
returning/CompletableFuture.failedFuture), and likewise ensure request duration
(requestStarted -> now) is recorded for error responses if you want consistent
timing metrics; update references in processHttpResponse and
handleHttpErrorRetry as needed so every HTTP response triggers
telemetry.metrics().requestCount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 532d1466-1bf5-43fd-87cf-863f2f20379d
📒 Files selected for processing (7)
CHANGELOG.mdsrc/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.javasrc/main/java/dev/openfga/sdk/telemetry/Counters.javasrc/main/java/dev/openfga/sdk/telemetry/Metrics.javasrc/test/java/dev/openfga/sdk/api/configuration/TelemetryConfigurationTest.javasrc/test/java/dev/openfga/sdk/telemetry/CountersTest.javasrc/test/java/dev/openfga/sdk/telemetry/MetricsTest.java
curfew-marathon
approved these changes
Mar 27, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds #118
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features