feat(sdk/go): add baseline hand-written Go SDK for v3 meters#4644
feat(sdk/go): add baseline hand-written Go SDK for v3 meters#4644gergely-kurucz-konghq wants to merge 16 commits into
Conversation
Hand-written, idiomatic Go SDK as a reference shape for a future TypeSpec emitter. Standalone module (github.com/openmeterio/openmeter/sdk/go). - net/http public surface; go-retryablehttp hidden behind *http.Client - resource-grouped ops: Meters.Get / List / Query / QueryCSV - query-string serialization (deepObject page/filter, form sort) - typed APIError; bearer auth set at request-build time (survives client injection) - unit tests via httptest + env-gated TestLive (verified vs local and dev cloud)
gergely-kurucz-konghq
added
kind/feature
📝 WalkthroughWalkthroughThis PR adds a brand-new, hand-written Go SDK package ( ChangesGo SDK for OpenMeter v3
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant MetersService
participant Client
participant HTTPClient as http.Client
participant Server
Caller->>Client: New(baseURL, opts...)
Client-->>Caller: *Client (with Meters)
Caller->>MetersService: Query(ctx, meterID, request)
MetersService->>MetersService: validate meterID (ErrEmptyID)
MetersService->>Client: newRequest(POST, /query, body)
Client->>HTTPClient: doRaw(request)
HTTPClient->>Server: POST /openmeter/meters/{id}/query
Server-->>HTTPClient: JSON/CSV response or error
alt non-2xx
HTTPClient-->>Client: newAPIError(response)
Client-->>MetersService: *APIError
else 2xx
HTTPClient-->>Client: response bytes
Client-->>MetersService: MeterQueryResult
end
MetersService-->>Caller: result or error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Assert filter[key][eq] round-trips through a real server: exact-match returns the meter, a bogus key returns none.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Override retryablehttp CheckRetry so a non-idempotent request that gets a response (e.g. 5xx on POST) is not retried, avoiding duplicate side effects; transport errors with no response fall back to the default policy. Adds a table test over method x status/err. Document the single WithHTTPClient injection seam in the README, including custom retry policies and how third-party libraries (go-retryablehttp, imroc/req, Heimdall, Resty, Sling) do or do not compose at the transport layer.
Get/Query/QueryCSV now return ErrEmptyMeterID up front instead of building /openmeter/meters/ (trailing slash), which could decode a list envelope into a zero-value Meter or yield a generic 4xx. Exported sentinel for errors.Is; covered by TestMeters_EmptyMeterID.
Maps and slices are already reference types: nil is the idiomatic "not set" and omitempty omits it, so *map/*slice only forced double-dereference on callers. Scalar optionals keep pointers to distinguish unset from zero.
Move the package to sdk/go/openmeter so the import base matches the package name (github.com/openmeterio/openmeter/sdk/go/openmeter), letting callers use openmeter.New without a goimports-inserted alias. Module root stays sdk/go.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
sdk/go/openmeter/transport.go (2)
21-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo default request timeout; fully relies on caller context.
defaultHTTPClientdoesn't set an overall timeout on the resulting*http.Client(nor doescleanhttp.DefaultPooledClient, used internally byretryablehttp.NewClient(), set one). Combined with up to 3 retries and up to 5s backoff each, a caller that forgets to pass a context deadline could block for a long time. Consider documenting this expectation prominently (e.g. in the README) or applying a sane default via context-wrapping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/transport.go` around lines 21 - 32, The default HTTP client created by defaultHTTPClient currently has no overall timeout and can retry for a long time if callers do not set a context deadline. Update the SDK behavior by either adding a sane default timeout around the retryablehttp client in defaultHTTPClient/retryablehttp.NewClient usage, or make the no-timeout expectation explicit in the SDK docs/README and any public client setup guidance. Keep the change scoped to the default transport path in transport.go and ensure the behavior is clearly documented where the SDK client is introduced.
110-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffUnbounded
io.ReadAllon response bodies.
doRawbuffers the entire response body into memory with no size limit. This is shared by both JSON responses and (per the AI summary)QueryCSV, where usage exports could be large. Consider a size cap (io.LimitReader) or a streaming (io.ReadCloser) return path for the CSV case to avoid unbounded memory growth on large exports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/transport.go` around lines 110 - 127, The Client.doRaw method currently reads the entire HTTP response body into memory without any cap, which can cause unbounded growth for large payloads. Update doRaw to enforce a maximum body size using a limited reader, or introduce a streaming path for large exports such as QueryCSV so CSV responses can be consumed without buffering everything. Keep the existing error handling in doRaw and adjust any callers that rely on the raw byte slice to use the new streaming-safe path where appropriate.sdk/go/openmeter/live_test.go (1)
34-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a bounded context for the live calls.
ctx := context.Background()gets reused for every real network call in this test (List, Get, Query, QueryCSV). If the target server hangs, this test just blocks instead of failing fast with a clear message — acontext.WithTimeoutwould make failures quicker and more diagnosable when running against a flaky/slow live endpoint.💡 Suggested tweak
- ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/live_test.go` around lines 34 - 104, The live test currently reuses a background context for all network calls, so List, Get, Query, and QueryCSV can hang indefinitely against a slow or stuck server. Update the test to create a bounded context with timeout before the first client.Meters call, use that context for the existing List/Get/Query/QueryCSV calls, and ensure it is canceled when the test ends. Use the existing client.Meters methods and the local ctx variable in live_test.go to keep the change localized.sdk/go/openmeter/query_test.go (1)
5-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFill in the untested filter operators.
The table nicely covers
Eq,Contains, andExists, butaddStringFilteralso serializesNeq,Gt,Gte,Lt, andLte— none of those have a test case, so a regression there would slip through silently.As per path instructions, "Make sure the tests are comprehensive and cover the changes."
✅ Suggested additional case
{ name: "filter nested deepObject", params: MeterListParams{Filter: &MeterFilter{Key: &StringFilter{Eq: String("tokens")}}}, want: "filter%5Bkey%5D%5Beq%5D=tokens", }, + { + name: "filter range operators", + params: MeterListParams{Filter: &MeterFilter{Key: &StringFilter{Gte: String("a"), Lte: String("z")}}}, + want: "filter%5Bkey%5D%5Bgte%5D=a&filter%5Bkey%5D%5Blte%5D=z", + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/query_test.go` around lines 5 - 50, The MeterListParams.values test table in TestMeterListParams_Values is missing coverage for string filter operators handled by addStringFilter. Add cases for Neq, Gt, Gte, Lt, and Lte on MeterFilter/StringFilter so their query encoding is asserted alongside the existing Eq, Contains, and Exists coverage. Keep the new cases in the same table-driven style and verify the encoded query string matches the expected deepObject form.Source: Path instructions
sdk/go/openmeter/meters.go (1)
88-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the meter-ID guard + path building.
Get,Query, andQueryCSVall repeat the same empty-ID check andmetersBasePath+"/"+url.PathEscape(meterID)construction. Pulling this into one helper keeps future additions (e.g. aDelete) from forgetting the guard.As per path instructions, "make readability and maintainability a priority, even potentially suggest restructuring the code to improve them."
♻️ Proposed helper extraction
+func meterPath(meterID string) (string, error) { + if meterID == "" { + return "", ErrEmptyMeterID + } + return metersBasePath + "/" + url.PathEscape(meterID), nil +} + func (s *MetersService) Get(ctx context.Context, meterID string) (*Meter, error) { - if meterID == "" { - return nil, ErrEmptyMeterID - } - req, err := s.client.newRequest(ctx, http.MethodGet, metersBasePath+"/"+url.PathEscape(meterID), nil, nil, contentTypeJSON) + path, err := meterPath(meterID) + if err != nil { + return nil, err + } + req, err := s.client.newRequest(ctx, http.MethodGet, path, nil, nil, contentTypeJSON) if err != nil { return nil, err }(similarly for
Query/QueryCSV)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/meters.go` around lines 88 - 148, The meter ID validation and request path construction are duplicated in MetersService.Get, MetersService.Query, and MetersService.QueryCSV. Extract that shared logic into a small helper on MetersService (or a private package helper) that returns the escaped meter path or an error for empty IDs, then have Get, Query, QueryCSV, and any future meter actions reuse it to keep the guard and path formatting consistent.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sdk/go/openmeter/live_test.go`:
- Around line 34-104: The live test currently reuses a background context for
all network calls, so List, Get, Query, and QueryCSV can hang indefinitely
against a slow or stuck server. Update the test to create a bounded context with
timeout before the first client.Meters call, use that context for the existing
List/Get/Query/QueryCSV calls, and ensure it is canceled when the test ends. Use
the existing client.Meters methods and the local ctx variable in live_test.go to
keep the change localized.
In `@sdk/go/openmeter/meters.go`:
- Around line 88-148: The meter ID validation and request path construction are
duplicated in MetersService.Get, MetersService.Query, and
MetersService.QueryCSV. Extract that shared logic into a small helper on
MetersService (or a private package helper) that returns the escaped meter path
or an error for empty IDs, then have Get, Query, QueryCSV, and any future meter
actions reuse it to keep the guard and path formatting consistent.
In `@sdk/go/openmeter/query_test.go`:
- Around line 5-50: The MeterListParams.values test table in
TestMeterListParams_Values is missing coverage for string filter operators
handled by addStringFilter. Add cases for Neq, Gt, Gte, Lt, and Lte on
MeterFilter/StringFilter so their query encoding is asserted alongside the
existing Eq, Contains, and Exists coverage. Keep the new cases in the same
table-driven style and verify the encoded query string matches the expected
deepObject form.
In `@sdk/go/openmeter/transport.go`:
- Around line 21-32: The default HTTP client created by defaultHTTPClient
currently has no overall timeout and can retry for a long time if callers do not
set a context deadline. Update the SDK behavior by either adding a sane default
timeout around the retryablehttp client in
defaultHTTPClient/retryablehttp.NewClient usage, or make the no-timeout
expectation explicit in the SDK docs/README and any public client setup
guidance. Keep the change scoped to the default transport path in transport.go
and ensure the behavior is clearly documented where the SDK client is
introduced.
- Around line 110-127: The Client.doRaw method currently reads the entire HTTP
response body into memory without any cap, which can cause unbounded growth for
large payloads. Update doRaw to enforce a maximum body size using a limited
reader, or introduce a streaming path for large exports such as QueryCSV so CSV
responses can be consumed without buffering everything. Keep the existing error
handling in doRaw and adjust any callers that rely on the raw byte slice to use
the new streaming-safe path where appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc2a4ddc-9e37-4dd1-9ee5-490ee427ebf8
⛔ Files ignored due to path filters (2)
sdk/go/go.modis excluded by!**/*.modsdk/go/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (13)
sdk/go/README.mdsdk/go/openmeter/client.gosdk/go/openmeter/client_test.gosdk/go/openmeter/errors.gosdk/go/openmeter/example_test.gosdk/go/openmeter/live_test.gosdk/go/openmeter/meters.gosdk/go/openmeter/models.gosdk/go/openmeter/option.gosdk/go/openmeter/ptr.gosdk/go/openmeter/query.gosdk/go/openmeter/query_test.gosdk/go/openmeter/transport.go
Set the module path to github.com/openmeterio/openmeter/sdk/go/openmeter so its last segment matches the package name. go get / import use the natural path with no alias and no `go` leaf segment.
The Example and the New doc comment used a bare host, which resolves to the wrong path. Both now include the required /api/v3 prefix.
Add a base-URL table covering OpenMeter Cloud, local, and Kong Konnect. Note Konnect's /v3 prefix (vs /api/v3) and that a kpat_ PAT authenticates through the same WithToken bearer mechanism.
The default transport had no timeout, so a stalled connection could block indefinitely when no context deadline was set. Bound each attempt to 30s (per attempt, not per call, so retries still run). Documented at the const and in the README.
doRaw read the body unbounded via io.ReadAll. Cap buffered paths (JSON, QueryCSV) at 10 MiB via io.LimitReader (1 MiB for error bodies) and add doStream + Meters.QueryCSVStream returning an io.ReadCloser for large CSV exports without buffering. Covered by new tests.
live_test.go used context.Background(), so calls could hang against a slow server — derive ctx from context.WithTimeout(t.Context(), 30s) with defer cancel(). Align client_test.go to t.Context() per repo convention.
Add a table case asserting deepObject encoding of Neq/Gt/Gte/Lt/Lte alongside the existing Eq/Contains/Exists coverage.
meterPath/ErrEmptyMeterID were meter-specific, but the empty-ID guard and URL escaping repeat identically for every resource. Replace with a resource- agnostic resourcePath(base, id) in client.go and a single ErrEmptyID sentinel, so future entities (and emitter-generated resources) reuse one primitive.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
sdk/go/openmeter/client_test.go (1)
226-245: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing empty-ID coverage for
QueryCSVStream.
TestMeters_EmptyMeterIDcoversGet,Query, andQueryCSVbut notQueryCSVStream, which shares the sameresourcePath/ErrEmptyIDguard. Since it's a distinct exported method with its own regression risk, worth adding for completeness.✅ Suggested addition
if _, err := c.Meters.QueryCSV(ctx, "", MeterQueryRequest{}); !errors.Is(err, ErrEmptyID) { t.Errorf("QueryCSV(\"\") error = %v, want ErrEmptyID", err) } + + if _, err := c.Meters.QueryCSVStream(ctx, "", MeterQueryRequest{}); !errors.Is(err, ErrEmptyID) { + t.Errorf("QueryCSVStream(\"\") error = %v, want ErrEmptyID", err) + } }As per path instructions,
**/*_test.go: "Make sure the tests are comprehensive and cover the changes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/client_test.go` around lines 226 - 245, `TestMeters_EmptyMeterID` is missing coverage for the `Meters.QueryCSVStream` guard, even though it should fail fast with `ErrEmptyID` like `Get`, `Query`, and `QueryCSV`. Extend this test to call `c.Meters.QueryCSVStream(ctx, "", MeterQueryRequest{})` and assert `errors.Is(err, ErrEmptyID)`, reusing the existing `newTestClient` setup so no request is sent for an empty meter ID.Source: Path instructions
sdk/go/openmeter/transport.go (1)
179-192: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOversized/failed error-body reads silently produce an empty
APIError.
readAllCappeddiscards the body entirely (nil, err) whenever the read fails or exceedsmax. BothdoRawanddoStreamcall this for the non-2xx path and swallow the error (body, _ := readAllCapped(...)), so any error response that's truncated by a slow/broken connection or that exceedsmaxErrorBodyresults in an*APIErrorwith emptyTitle/Detail/etc. — exactly when you most need diagnostic info (e.g., an HTML error page from a misconfigured proxy).Consider having
readAllCappedreturn the partial bytes it did manage to read alongside the error, sonewAPIErrorcan still be built with whatever was captured up to the cap.♻️ Suggested fix
func readAllCapped(r io.Reader, max int64) ([]byte, error) { body, err := io.ReadAll(io.LimitReader(r, max+1)) if err != nil { - return nil, fmt.Errorf("openmeter: reading response body: %w", err) + return body, fmt.Errorf("openmeter: reading response body: %w", err) } if int64(len(body)) > max { - return nil, fmt.Errorf("openmeter: response body exceeds %d-byte limit; use a streaming method for large payloads", max) + return body[:max], fmt.Errorf("openmeter: response body exceeds %d-byte limit; use a streaming method for large payloads", max) } return body, nil }Also applies to: 145-148, 168-174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/go/openmeter/transport.go` around lines 179 - 192, The error-body handling in readAllCapped currently drops all captured bytes whenever io.ReadAll fails or the payload exceeds the cap, and doRaw/doStream then ignore that error and build an empty APIError. Update readAllCapped to return any partial body it managed to read along with the error, and adjust the non-2xx paths in doRaw and doStream to preserve and pass those bytes into newAPIError instead of discarding them. Use the existing readAllCapped, doRaw, doStream, and newAPIError symbols to keep the failure response populated with whatever diagnostic content was available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/go/openmeter/transport.go`:
- Around line 40-51: The defaultHTTPClient setup in transport.go is applying a
global http.Client timeout that can still expire during QueryCSVStream body
reads. Update defaultHTTPClient (or introduce a separate streaming client used
by QueryCSVStream) so streamed CSV responses do not inherit the overall timeout,
while preserving the existing retry and attempt-level behavior for non-streaming
requests.
---
Nitpick comments:
In `@sdk/go/openmeter/client_test.go`:
- Around line 226-245: `TestMeters_EmptyMeterID` is missing coverage for the
`Meters.QueryCSVStream` guard, even though it should fail fast with `ErrEmptyID`
like `Get`, `Query`, and `QueryCSV`. Extend this test to call
`c.Meters.QueryCSVStream(ctx, "", MeterQueryRequest{})` and assert
`errors.Is(err, ErrEmptyID)`, reusing the existing `newTestClient` setup so no
request is sent for an empty meter ID.
In `@sdk/go/openmeter/transport.go`:
- Around line 179-192: The error-body handling in readAllCapped currently drops
all captured bytes whenever io.ReadAll fails or the payload exceeds the cap, and
doRaw/doStream then ignore that error and build an empty APIError. Update
readAllCapped to return any partial body it managed to read along with the
error, and adjust the non-2xx paths in doRaw and doStream to preserve and pass
those bytes into newAPIError instead of discarding them. Use the existing
readAllCapped, doRaw, doStream, and newAPIError symbols to keep the failure
response populated with whatever diagnostic content was available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 711f4dc0-166f-4b8a-9ca6-b5d7118fabf9
⛔ Files ignored due to path filters (2)
sdk/go/openmeter/go.modis excluded by!**/*.modsdk/go/openmeter/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (11)
sdk/go/openmeter/README.mdsdk/go/openmeter/client.gosdk/go/openmeter/client_test.gosdk/go/openmeter/errors.gosdk/go/openmeter/example_test.gosdk/go/openmeter/live_test.gosdk/go/openmeter/meters.gosdk/go/openmeter/query.gosdk/go/openmeter/query_test.gosdk/go/openmeter/transport.gosdk/go/openmeter/transport_test.go
✅ Files skipped from review due to trivial changes (2)
- sdk/go/openmeter/query.go
- sdk/go/openmeter/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- sdk/go/openmeter/example_test.go
- sdk/go/openmeter/query_test.go
- sdk/go/openmeter/errors.go
- sdk/go/openmeter/client.go
- sdk/go/openmeter/live_test.go
| func defaultHTTPClient() *http.Client { | ||
| rc := retryablehttp.NewClient() | ||
|
|
||
| rc.RetryMax = 3 | ||
| rc.RetryWaitMin = 500 * time.Millisecond | ||
| rc.RetryWaitMax = 5 * time.Second | ||
| rc.Logger = nil // silence the default stdout logger | ||
| rc.CheckRetry = retryIdempotentOnly | ||
| rc.HTTPClient.Timeout = defaultAttemptTimeout | ||
|
|
||
| return rc.StandardClient() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== transport.go ==\n'
sed -n '1,240p' sdk/go/openmeter/transport.go
printf '\n== search timeout/stream symbols ==\n'
rg -n "defaultAttemptTimeout|QueryCSVStream|doStream|WithHTTPClient|HTTPClient.Timeout|retryablehttp.NewClient" sdk/go/openmeter -SRepository: openmeterio/openmeter
Length of output: 2231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' sdk/go/openmeter/transport.go
printf '\n---\n'
rg -n "defaultAttemptTimeout|QueryCSVStream|doStream|WithHTTPClient|HTTPClient.Timeout|retryablehttp.NewClient" sdk/go/openmeter -SRepository: openmeterio/openmeter
Length of output: 2231
🌐 Web query:
Go http.Client Timeout includes reading the response body and cancels in-flight body reads documentation
💡 Result:
Yes, the Timeout field in a Go http.Client includes the time spent reading the response body, and it does cancel in-flight body reads [1][2][3]. According to the official Go documentation, the Timeout specifies a time limit for requests made by the client [1][4]. This duration encompasses: - Connection time [1][2] - Any redirects [1][2] - Reading the response body [1][2] The timer associated with the Timeout remains active even after the client's methods (such as Get, Head, Post, or Do) have returned [1][2]. If the timeout duration is reached while the application is still reading the Response.Body, the timer will interrupt the read operation [1][2]. Internally, the client uses a wrapper (often referred to as cancelTimerBody) around the response body that checks if the timeout has been exceeded during read operations [2][3]. If a timeout occurs, the read operation will return an error marked as a network timeout [2][3]. A Timeout value of zero means there is no timeout [1][3].
Citations:
- 1: https://pkg.go.dev/net/http@go1.26.4
- 2: https://go.dev/src/net/http/client.go?m=text
- 3: https://github.com/golang/go/blob/go1.20.12/src/net/http/client.go
- 4: https://pkg.go.dev/net/http
Drop the overall timeout for streamed CSV responses. http.Client.Timeout keeps running while resp.Body is being read, so QueryCSVStream can still hit the 30s limit mid-download and fail large exports after the request has already returned. A separate streaming client or transport-level timeouts would avoid that.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/go/openmeter/transport.go` around lines 40 - 51, The defaultHTTPClient
setup in transport.go is applying a global http.Client timeout that can still
expire during QueryCSVStream body reads. Update defaultHTTPClient (or introduce
a separate streaming client used by QueryCSVStream) so streamed CSV responses do
not inherit the overall timeout, while preserving the existing retry and
attempt-level behavior for non-streaming requests.
1 participant
What
A hand-written, idiomatic Go SDK for the OpenMeter v3 API, added as a standalone module (
github.com/openmeterio/openmeter/sdk/go/openmeter). It implements the meter endpoints —Get,List,Query,QueryCSV(+ streamingQueryCSVStream) — as a worked reference of the target SDK shape.Why
The existing generated client (
api/client/go) is a 1.7MB oapi-codegen monolith on the v1 cloud spec. We want an idiomatic, dependency-light SDK whose shape a TypeSpec emitter can later reproduce. This PR establishes that gold-standard shape and verifies it against real servers before automating it.How
Clientexposes onlynet/http,context, typed request/response structs, and a typedAPIError. No third-party type leaks; the emitter can mirror it verbatim.client.Meters.Get/List/Query/QueryCSV/QueryCSVStream.list-meters: deepObjectpage, formsort, nested deepObjectfilter.go-retryablehttpclient (hidden behind*http.Client);WithHTTPClientswaps it wholesale. Retries are idempotent-only (no duplicate writes on 5xx), with a 30s per-attempt timeout.QueryCSVStreamreturns anio.ReadCloserfor large exports.ErrEmptyID) fails fast; sharedresourcePath(base, id)centralizes ID validation + escaping for future entities.DateTime→time.Time,Numeric→string, reference-type fields un-pointered).Verification — unit tests via
httptest(retry policy, query encoding, body cap, streaming, errors, auth, injection) + an env-gatedTestLive. Verified end-to-end against a local server (unauthenticated),dev.openmeter.cloud(bearer token), and Kong Konnect Dev (kpatPAT) — including a non-emptyQuery(value 322 on the API Requests meter). README documents base URLs (note Konnect's/v3vs cloud's/api/v3prefix), HTTP-client/retry injection, and how third-party libs (go-retryablehttp, imroc/req, Heimdall, Resty, Sling) do or don't compose.Note: standalone module, not in any
go.work; maingo build ./...doesn't cover it. CI wiring is a follow-up.Summary by CodeRabbit
New Features
Bug Fixes
Documentation