feat(telemetry): add fga-client.request.count metric for tracking HTT…#286
Open
feat(telemetry): add fga-client.request.count metric for tracking HTT…#286
Conversation
…P requests Record a counter metric on every HTTP attempt in recordTelemetry(), alongside the existing request.duration and query.duration metrics. This enables users to track the total number of HTTP requests made by the SDK via OpenTelemetry. Closes #282
WalkthroughThe PR adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Executor as api_executor
participant Metrics as metrics.Metrics
participant Telemetry as OpenTelemetry Backend
Client->>Executor: Execute HTTP Request
Executor->>Executor: Build telemetry attributes
Executor->>Metrics: RequestCount(1, attrs)
Metrics->>Metrics: GetCounter()
Metrics->>Metrics: PrepareAttributes(METRIC_COUNTER_REQUEST_COUNT)
Metrics->>Telemetry: counter.Add(1, attrs)
Telemetry-->>Metrics: Success
Metrics-->>Executor: Return counter
Executor->>Executor: Continue request execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #286 +/- ##
==========================================
+ Coverage 33.63% 33.78% +0.14%
==========================================
Files 115 115
Lines 9831 9859 +28
==========================================
+ Hits 3307 3331 +24
- Misses 6260 6264 +4
Partials 264 264 ☔ 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@@ -1,20 +1,22 @@
## OpenTelemetry Metrics in the OpenFGA SDKs
-This document describes the OpenTelemetry metrics supported by the OpenFGA JavaScript and Java SDKs, including metric names, types, default enablement, descriptions, supported attributes, configuration instructions, Prometheus examples, and integration guidance for observability tools.
+This document describes the OpenTelemetry metrics supported by the OpenFGA JavaScript, Java, and Go SDKs, including metric names, types, default enablement, descriptions, supported attributes, configuration instructions, Prometheus examples, and integration guidance for observability tools.
---
### Supported Metrics
-Both the JavaScript and Java SDKs emit the following OpenTelemetry metrics by default:
+The JavaScript, Java, and Go SDKs emit the following OpenTelemetry metrics by default:
| 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 FGA client SDK |
| `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 | Time taken for a single HTTP request to complete, including retries (milliseconds) |
[See JavaScript SDK documentation](https://github.com/openfga/js-sdk/blob/main/docs/opentelemetry.md#metrics)
-[See Java SDK documentation](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md#metrics)
+[See Java SDK documentation](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md#metrics)
+[See Go SDK documentation](https://github.com/openfga/go-sdk/blob/main/docs/opentelemetry.md#metrics)
---
@@ -23,25 +25,26 @@
| Attribute Name | Type | Enabled by Default | Description | Used By Metrics |
|--------------------------------------- |---------|--------------------|-----------------------------------------------------------------------------------------------|-----------------------------------------|
-| `fga-client.response.model_id` | string | Yes | Authorization model ID used by the FGA server | request.duration, query.duration |
-| `fga-client.request.method` | string | Yes | FGA method/action performed (e.g., Check, ListObjects) | request.duration, query.duration |
-| `fga-client.request.store_id` | string | Yes | Store ID sent in the request | request.duration, query.duration |
-| `fga-client.request.model_id` | string | Yes | Authorization model ID sent in the request, if any | request.duration, query.duration |
-| `fga-client.request.client_id` | string | Yes | Client ID associated with the request, if any | request.duration, query.duration |
-| `fga-client.user` | string | No | User associated with the action (for check and list users); disabled by default | request.duration, query.duration |
-| `http.status_code` / `http.response.status_code` | int | Yes | HTTP response status code | request.duration, query.duration, http_request.duration |
-| `http.request.method` | string | (JS: No, Java: Yes)| HTTP method for the request | request.duration, query.duration, http_request.duration |
-| `http.host` | string | Yes | Host identifier of the origin | request.duration, query.duration, http_request.duration |
-| `user_agent.original` | string | Yes | User agent used in the query | request.duration, query.duration, http_request.duration |
-| `url.full` | string | (JS: No, Java: Yes)| Full URL of the request | request.duration, query.duration, http_request.duration |
-| `url.scheme` | string | (JS: No, Java: Yes)| HTTP scheme (`http`/`https`) | request.duration, query.duration, http_request.duration |
-| `http.request.resend_count` | int | Yes | Number of retries attempted | request.duration, query.duration |
-| `fga-client.request.batch_check_size` | int | No (Java only) | Number of objects in a batch check request | request.duration, query.duration |
+| `fga-client.response.model_id` | string | Yes | Authorization model ID used by the FGA server | request.duration, query.duration, request.count |
+| `fga-client.request.method` | string | Yes | FGA method/action performed (e.g., Check, ListObjects) | request.duration, query.duration, request.count |
+| `fga-client.request.store_id` | string | Yes | Store ID sent in the request | request.duration, query.duration, request.count |
+| `fga-client.request.model_id` | string | Yes | Authorization model ID sent in the request, if any | request.duration, query.duration, request.count |
+| `fga-client.request.client_id` | string | Yes | Client ID associated with the request, if any | request.duration, query.duration, request.count |
+| `fga-client.user` | string | No | User associated with the action (for check and list users); disabled by default | request.duration, query.duration, request.count |
+| `http.status_code` / `http.response.status_code` | int | Yes | HTTP response status code | request.duration, query.duration, request.count, http_request.duration |
+| `http.request.method` | string | (JS: No, Java: Yes, Go: Yes)| HTTP method for the request | request.duration, query.duration, request.count, http_request.duration |
+| `http.host` | string | Yes | Host identifier of the origin | request.duration, query.duration, request.count, http_request.duration |
+| `user_agent.original` | string | Yes | User agent used in the query | request.duration, query.duration, request.count, http_request.duration |
+| `url.full` | string | (JS: No, Java: Yes, Go: Yes)| Full URL of the request | request.duration, query.duration, request.count, http_request.duration |
+| `url.scheme` | string | (JS: No, Java: Yes, Go: Yes)| HTTP scheme (`http`/`https`) | request.duration, query.duration, request.count, http_request.duration |
+| `http.request.resend_count` | int | Yes | Number of retries attempted | request.duration, query.duration, request.count |
+| `fga-client.request.batch_check_size` | int | No (Java only) | Number of objects in a batch check request | request.duration, query.duration, request.count |
| `http.client.request.duration` | int | No (JS only) | Time taken by the FGA server to process and evaluate the request (rounded to ms) | |
| `http.server.request.duration` | int | No (JS only) | Number of retries attempted | |
[See JavaScript SDK attribute table](https://github.com/openfga/js-sdk/blob/main/docs/opentelemetry.md#attributes)
-[See Java SDK attribute table](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md#attributes)
+[See Java SDK attribute table](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md#attributes)
+[See Go SDK attribute table](https://github.com/openfga/go-sdk/blob/main/docs/opentelemetry.md#attributes)
---
@@ -172,6 +175,7 @@
- `fga_client_request_duration_bucket` (request duration histogram)
- `fga_client_query_duration_bucket` (query duration histogram)
- `fga_client_http_request_duration_bucket` (HTTP request duration histogram)
+- `fga_client_request_count_total` (request count counter)
- `fga_client_credentials_request_total` (credentials request counter)
Example Prometheus queries:
@@ -185,6 +189,9 @@
# Request rate by HTTP status code
rate(fga_client_request_duration_count[5m])
+
+# Total request count rate
+rate(fga_client_request_count_total[5m])
# 95th percentile HTTP request duration
histogram_quantile(0.95, rate(fga_client_http_request_duration_bucket[5m]))
@@ -224,8 +231,9 @@
### Additional Notes
- If OpenTelemetry is not configured, metrics are not sent (no-op).
- The Java SDK supports both manual configuration and Java Agent (automatic instrumentation with zero code changes).
+- The Go SDK records `fga-client.request.count` on every HTTP attempt in the `recordTelemetry()` function, providing visibility into total request volume.
- High-cardinality attributes (like `fga-client.user`) are disabled by default to avoid excessive costs with some metric collectors. Enable only if necessary.
---
-For further details and up-to-date examples, refer to the [OpenFGA JavaScript SDK OpenTelemetry documentation](https://github.com/openfga/js-sdk/blob/main/docs/opentelemetry.md) and [OpenFGA Java SDK OpenTelemetry documentation](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md).
+For further details and up-to-date examples, refer to the [OpenFGA JavaScript SDK OpenTelemetry documentation](https://github.com/openfga/js-sdk/blob/main/docs/opentelemetry.md), [OpenFGA Java SDK OpenTelemetry documentation](https://github.com/openfga/java-sdk/blob/main/docs/OpenTelemetry.md), and [OpenFGA Go SDK OpenTelemetry documentation](https://github.com/openfga/go-sdk/blob/main/docs/opentelemetry.md).Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
telemetry/attributes.go (1)
65-70: Add direct tests for the newPrepareAttributesrequest-count branch.The new switch branch is correct, but this path should get explicit tests (including nil config and enabled config cases) to cover the newly introduced lines and prevent regression drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telemetry/attributes.go` around lines 65 - 70, Add unit tests covering the PrepareAttributes function's METRIC_COUNTER_REQUEST_COUNT branch: write tests that call PrepareAttributes (or the exported wrapper used in tests) with (1) a config where METRIC_COUNTER_REQUEST_COUNT == nil and assert it returns attribute.EmptySet() (or equivalent) and no error, and (2) a config where METRIC_COUNTER_REQUEST_COUNT is set (enabled) and assert the returned allowed set equals that config value; reference the METRIC_COUNTER_REQUEST_COUNT switch branch in telemetry/attributes.go to locate the code under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api_executor.go`:
- Around line 363-364: Move the metrics.RequestCount(1, attrs) call out of
recordTelemetry and invoke it immediately after the HTTP response is available
(right after http.Do/resp != nil), before any status handling or body reads, so
every attempt (including 3xx/4xx/5xx and read-body failures) is counted; keep
recordTelemetry for success-specific fields but remove the RequestCount call
there and ensure the attrs variable used for RequestCount is prepared at that
earlier point so the counter is emitted for every response attempt.
In `@telemetry/metrics.go`:
- Around line 75-87: The method Metrics.RequestCount shadows the outer err by
using := when calling m.PrepareAttributes, so PrepareAttributes errors are
discarded; change the call in RequestCount to assign into the existing err (use
= not :=) or use a new variable name (e.g., preparedAttrs) and propagate that
err, then return the actual error instead of nil; apply the same fix to the
other methods (CredentialsRequest, RequestDuration, QueryDuration,
HTTPRequestDuration) that call m.PrepareAttributes so all PrepareAttributes
failures are returned properly and no err is shadowed when calling GetCounter or
PrepareAttributes.
---
Nitpick comments:
In `@telemetry/attributes.go`:
- Around line 65-70: Add unit tests covering the PrepareAttributes function's
METRIC_COUNTER_REQUEST_COUNT branch: write tests that call PrepareAttributes (or
the exported wrapper used in tests) with (1) a config where
METRIC_COUNTER_REQUEST_COUNT == nil and assert it returns attribute.EmptySet()
(or equivalent) and no error, and (2) a config where
METRIC_COUNTER_REQUEST_COUNT is set (enabled) and assert the returned allowed
set equals that config value; reference the METRIC_COUNTER_REQUEST_COUNT switch
branch in telemetry/attributes.go to locate the code under test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73f84c7f-e936-4a2e-a9ae-87dcfb97fdff
📒 Files selected for processing (10)
api_executor.gotelemetry/attributes.gotelemetry/configuration.gotelemetry/configuration_test.gotelemetry/counters.gotelemetry/counters_test.gotelemetry/metrics.gotelemetry/metrics_test.gotelemetry/telemetry.gotelemetry/telemetry_test.go
Comment on lines
+363
to
+364
| _, _ = metrics.RequestCount(1, attrs) | ||
|
|
There was a problem hiding this comment.
Request counter is undercounting HTTP attempts.
RequestCount is emitted inside recordTelemetry, but recordTelemetry is only called on the success path (Line 413 onward). As a result, attempts returning HTTP 3xx/4xx/5xx (and read-body failures) are not counted, which conflicts with the per-attempt objective.
Proposed fix (emit telemetry once an HTTP response exists, before status handling)
@@
// Execute HTTP request
httpResponse, err := e.client.callAPI(req)
if err != nil || httpResponse == nil {
return nil, err
}
+
+ // Record telemetry for every HTTP attempt that produced a response.
+ e.recordTelemetry(request.OperationName, storeID, request.Body, req, httpResponse, requestStarted, attemptNum)
// Read response body
responseBody, err := io.ReadAll(httpResponse.Body)
_ = httpResponse.Body.Close()
httpResponse.Body = io.NopCloser(bytes.NewBuffer(responseBody))
@@
- // Record telemetry for successful requests
- e.recordTelemetry(request.OperationName, storeID, request.Body, req, httpResponse, requestStarted, attemptNum)
-
return response, nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api_executor.go` around lines 363 - 364, Move the metrics.RequestCount(1,
attrs) call out of recordTelemetry and invoke it immediately after the HTTP
response is available (right after http.Do/resp != nil), before any status
handling or body reads, so every attempt (including 3xx/4xx/5xx and read-body
failures) is counted; keep recordTelemetry for success-specific fields but
remove the RequestCount call there and ensure the attrs variable used for
RequestCount is prepared at that earlier point so the counter is emitted for
every response attempt.
Comment on lines
+75
to
+87
| func (m *Metrics) RequestCount(value int64, attrs map[*Attribute]string) (metric.Int64Counter, error) { | ||
| var counter, err = m.GetCounter(RequestCount.Name, RequestCount.Description) | ||
|
|
||
| if err == nil { | ||
| attrs, err := m.PrepareAttributes(RequestCount, attrs, m.Configuration) | ||
|
|
||
| if err == nil { | ||
| counter.Add(context.Background(), value, metric.WithAttributeSet(attrs)) | ||
| } | ||
| } | ||
|
|
||
| return counter, err | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify short-declaration shadowing sites in telemetry/metrics.go
# Expected: matches at Line 79 (new code), and possibly similar legacy patterns.
rg -n -C2 'attrs,\s*err\s*:=' telemetry/metrics.goRepository: openfga/go-sdk
Length of output: 726
🏁 Script executed:
sed -n '75,87p' telemetry/metrics.goRepository: openfga/go-sdk
Length of output: 457
Shadowing err causes PrepareAttributes failures to be dropped.
On line 79, the short declaration attrs, err := m.PrepareAttributes(...) shadows the outer scope's err variable from line 76. If PrepareAttributes fails, the error is discarded and the function incorrectly returns nil error while returning a zero-value counter.
Note: This same pattern occurs in other methods in this file (CredentialsRequest, RequestDuration, QueryDuration, HTTPRequestDuration) and should be fixed consistently.
Suggested fix
func (m *Metrics) RequestCount(value int64, attrs map[*Attribute]string) (metric.Int64Counter, error) {
var counter, err = m.GetCounter(RequestCount.Name, RequestCount.Description)
- if err == nil {
- attrs, err := m.PrepareAttributes(RequestCount, attrs, m.Configuration)
-
- if err == nil {
- counter.Add(context.Background(), value, metric.WithAttributeSet(attrs))
- }
- }
-
- return counter, err
+ if err != nil {
+ return counter, err
+ }
+
+ preparedAttrs, err := m.PrepareAttributes(RequestCount, attrs, m.Configuration)
+ if err != nil {
+ return counter, err
+ }
+
+ counter.Add(context.Background(), value, metric.WithAttributeSet(preparedAttrs))
+ return counter, nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@telemetry/metrics.go` around lines 75 - 87, The method Metrics.RequestCount
shadows the outer err by using := when calling m.PrepareAttributes, so
PrepareAttributes errors are discarded; change the call in RequestCount to
assign into the existing err (use = not :=) or use a new variable name (e.g.,
preparedAttrs) and propagate that err, then return the actual error instead of
nil; apply the same fix to the other methods (CredentialsRequest,
RequestDuration, QueryDuration, HTTPRequestDuration) that call
m.PrepareAttributes so all PrepareAttributes failures are returned properly and
no err is shadowed when calling GetCounter or PrepareAttributes.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new OpenTelemetry counter metric (fga-client.request.count) to track the number of HTTP requests made by the Go SDK, aligning metrics parity across SDKs and addressing #282.
Changes:
- Introduces the
RequestCountcounter definition and default configuration wiring for attributes. - Implements
MetricsInterface.RequestCount+ factory helper, and records the counter inapiExecutor.recordTelemetry. - Adds unit tests in the telemetry package for the new counter and metric plumbing.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
api_executor.go |
Increments the new request count metric during telemetry recording. |
telemetry/attributes.go |
Allows attribute filtering/config for the new counter. |
telemetry/configuration.go |
Adds default config block for the new counter and its enabled attributes. |
telemetry/configuration_test.go |
Verifies default configuration includes the new counter. |
telemetry/counters.go |
Defines the fga-client.request.count counter constant + metadata. |
telemetry/counters_test.go |
Tests counter name/description initialization. |
telemetry/metrics.go |
Adds RequestCount to the metrics interface and implements the counter emission. |
telemetry/metrics_test.go |
Tests RequestCount calls Add on the counter. |
telemetry/telemetry.go |
Adds RequestCountMetricParameters and RequestCountMetric factory wrapper. |
telemetry/telemetry_test.go |
Adds a basic factory-level test for RequestCountMetric. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+363
to
366
| _, _ = metrics.RequestCount(1, attrs) | ||
|
|
||
| if requestDuration > 0 { | ||
| _, _ = metrics.RequestDuration(requestDuration, attrs) |
Comment on lines
+363
to
+364
| _, _ = metrics.RequestCount(1, attrs) | ||
|
|
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.
…P requests
Record a counter metric on every HTTP attempt in recordTelemetry(), alongside the existing request.duration and query.duration metrics. This enables users to track the total number of HTTP requests made by the SDK via OpenTelemetry.
Closes #282
Description
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
Tests