fix(metrics/gcp): split each export into requests of at most 200 points

[akshat-kumar-singhal]

Description:

• Fixes metrics/gcp: exporter sends each collection as one request, so long-lived processes exceed the 200-point limit and lose all metrics #4266. The gcp metrics exporter sent each collection as one OTLP request. Google's Telemetry API rejects any request over 200 points, and it rejects the whole request. GMP requires cumulative temporality, so every attribute set a process has seen is re-sent on every interval. A long-lived process therefore grows past the cap with uptime, and from then on it loses every collection until it restarts.
• The exporter is now wrapped (chunk.go), so each Export goes out as a sequence of requests of at most 200 points:
  • Point order, scope and each metric's metadata (temporality, monotonicity, unit) are preserved. The SDK's pooled ResourceMetrics is never mutated: sub-slices share the backing array but are capped at their end ([lo:hi:hi]).
  • A rejected chunk no longer takes the rest of the collection with it; errors are combined with errors.Join. Once the context is done the loop stops, since every later request would fail too.
  • All nine aggregation types are handled (gauge, sum, histogram, exponential histogram, each as int64 and float64, plus summary).
• The export interval now has a 5-second floor. Google requires points of one time series to be at least 5 s apart, and every collection re-sends every series, so a shorter METRICS_EXPORT_INTERVAL / OTEL_METRIC_EXPORT_INTERVAL would have every collection's points rejected. exportInterval raises it to 5 s and warns at startup, naming both env vars. An unset (zero) interval passes through untouched, so the default still applies. The example README gets a short "Export interval" section.
• buildReader builds its reader through newReader(exporter, interval). Nothing outside pkg/gofr/metrics/exporters/gcp and its example README changes.

Why a wrapper rather than the SDK's batcher: sdk/metric v1.46.0 has an equivalent splitter, but it is unexported and only enabled by the experimental env var OTEL_GO_X_METRIC_EXPORT_BATCH_SIZE. The 200-point cap belongs to the destination, so the exporter for that destination enforces it, and users don't each have to rediscover it. If a user also sets the env var, the SDK's smaller batches pass through unchanged, and a test covers that case.

Users on released versions can set OTEL_GO_X_METRIC_EXPORT_BATCH_SIZE=200 as a workaround until this ships.

Other providers and extensibility: the splitter knows nothing about GCP. It works on OTel's metricdata.ResourceMetrics, wraps any metricSdk.Exporter, and takes the limit as a field. It stays unexported in the gcp module because GCP is currently the only exporter with a known point cap, and moving it later is non-breaking. When a second provider needs it, the natural follow-up is a separate PR that moves it into exporters and lets each builder declare its own limit. I surveyed other backends' documented per-request limits to check that nothing here closes that door:

• Point count (what this PR handles): Google Telemetry API 200 per request; Dynatrace OTLP 15,000 per request. A promoted splitter covers these by taking a different limit.
• Payload bytes: Datadog OTLP 512 KiB compressed (HTTP 413); CloudWatch PutMetricData 1 MB; New Relic Metric API 1 MB per POST; Dynatrace OTLP 4 MB uncompressed. Point count doesn't track bytes (attribute sizes vary), so the promoted version should take a small limit struct rather than a bare int, so a byte budget can be added without changing builders.
• Per-point and over-time limits (dimension caps: CloudWatch 30, Dynatrace 50/100, New Relic 150): splitting requests can't fix these, so they're out of scope for any splitter. Cloud Monitoring's minimum of 5 s between points on one series is also beyond a splitter, so this PR handles it separately with the interval floor above.
• GCP's own extra rules, checked against this PR: Cloud Monitoring allows only one point per time series per request, and Google says those limits also apply to Telemetry API ingestion. Within one collection each time series has exactly one point and lands in exactly one chunk, so the split never breaks this rule.

Sources: Telemetry API quotas, Cloud Monitoring quotas, CloudWatch PutMetricData, Datadog OTLP metrics, Dynatrace OTLP limits, New Relic Metric API limits.

Breaking Changes (if applicable):

• None. No new configuration and no change to the public API. The only behavior change beyond splitting: a gcp export interval under 5 s is now raised to 5 s, with a warning. Google was already rejecting those points.

Additional Information:

• No new dependencies. go.opentelemetry.io/otel/metric moves from indirect to direct in the exporter's go.mod, because the new test imports it.
• Tests:
  • Test_splitResourceMetrics: 0, 199, 200 and 201 points; one metric too big for two requests; every aggregation type across three scopes, with a request boundary falling inside a metric.
  • Test_chunkingExporter_Export: a failed chunk in the middle, and a canceled context.
  • Test_newReader: the real SDK pipeline, where 450 attribute sets produce requests of [200 200 50], and [150 150 150] when the SDK's own batcher is also enabled.
  • Test_exportInterval: unset, under, just under, exactly at, and above the 5 s minimum, asserting both the returned interval and whether the warning is logged.
  • Test_buildReader: a 1 s interval now warns at startup.
• Mutation check, splitter: 6 mutants, 5 killed. The survivor changes <= to < on the "already fits" shortcut; it yields the same single 200-point request, so it is behaviorally equivalent.
• Mutation check, interval floor: 4 mutants, 3 killed. The survivor is wiring: passing cfg.Interval rather than the raised interval to newReader in buildReader. The warning still fires, and the reader's period isn't observable from a unit test without waiting on the SDK's ticker. exportInterval itself is fully covered.
• Not verified against the live API: that Google counts one histogram data point as one point. That is how the SDK's own batcher counts, and it matches the one-time-series-per-point model.
• The issue also suggested deriving METRICS_CARDINALITY_LIMIT from the exporter's cap and adding a startup warning. Neither is included here: once requests are split, neither is needed, and the first would needlessly cap legitimate route cardinality.

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage. (exporters/gcp: 90.0% → 95.7%)
• I have reviewed the code comments and documentation for clarity.

[aryanmehrotra]

Reviewed at 7129f0d54 — both commits, 0 behind development. chunk.go and chunk_test.go are byte-identical between 272478ab9 and 7129f0d54, so everything below about the splitter was measured once and still applies. I planned the fix from #4266 before opening the diff, then checked it three ways rather than reading it for plausibility: a randomized property test on the splitter, a mutation run, and the example app running against a stand-in for Google's Telemetry API that enforces the 200-point ceiling the way the real one does — rejecting the whole request.

It works. Nothing below is blocking.

Run as a live app, before and after

examples/using-gcp-metrics built twice — once at the merge-base, once at this head — pointed at a TLS gRPC OTLP receiver that rejects any request over 200 points with Google's own message. Same binary otherwise, same load: 5 requests to /hello plus 250 distinct unmatched paths, the scanner traffic #4266 describes. METRICS_EXPORT_INTERVAL=5.

Before — the issue reproduces exactly, down to the error text:

REQ #1   points=253  metrics=3  scopes=1   REJECT_OVER_CAP
REQ #2   points=253  metrics=3  scopes=1   REJECT_OVER_CAP
...
{"level":"ERROR","message":"failed to upload metrics: rpc error: code = InvalidArgument
 desc = A maximum of 200 points can be written in a single request."}

0 points accepted. 100% loss, every cycle, while the app served all 255 requests and logged no other fault.

After — the same collection, same load:

REQ #1   points=200  metrics=2  scopes=1   ACCEPT
REQ #2   points=53   metrics=2  scopes=1   ACCEPT

506/506 points accepted, 0 lost, 0 ERROR lines, max points in any one request = 200.

Note metrics=2 in both: the boundary falls inside app_http_response, so the real app exercises the split-a-single-metric path, not just the unit test. Pushing cardinality further (700 distinct paths, ~504 points) gives [200, 200, 104] and the middle request carries metrics=1 — one metric spanning three requests, all accepted. Eight consecutive cycles: 4,032 points, 0 rejected, 0 app errors.

Chaos probe — a rejected chunk no longer takes the rest. With the receiver failing every second request:

REQ #6   points=200   REJECT_INJECTED
REQ #7   points=200   ACCEPT      <- the rest of the same collection still lands
REQ #8   points=104   REJECT_INJECTED

704 points delivered against a 50% server-side rejection rate, where before a single rejection cost the entire collection. The joined error surfaces intact (errors.Join producing two injected failure for this chunk lines in one log entry).

Cost, measured: 46.7 ns / 8 B / 1 alloc when the collection already fits, 568 ns / 1,168 B / 19 allocs at 504 points, 1.34 µs / 2,866 B / 51 allocs at 2,002. Once per export interval. Nothing to discuss.

The second commit — the 5-second floor — is right, and the doc claim holds

7129f0d54 clamps a sub-5s interval and warns. The number is not folklore: I opened Google's own page, and the sentence in your minExportInterval comment is theirs verbatim — "The Cloud Monitoring API requires that the end times of points written to a time series be at least 5 seconds apart." (Cloud Monitoring quotas, which also states the rate as one point per time series per 5 seconds.)

Live at this head, METRICS_EXPORT_INTERVAL=1:

WARN  gcp metrics: export interval 1s is shorter than the 5s Google requires between points
      of one time series; using 5s. Set METRICS_EXPORT_INTERVAL (seconds) or
      OTEL_METRIC_EXPORT_INTERVAL (milliseconds) to at least that
INFO  exporting metrics to Google Cloud at 127.0.0.1:19443 every 5s via keyless ADC

06:51:40 REQ #3 points=200 ACCEPT   06:51:40 REQ #4 points=53 ACCEPT
06:51:45 REQ #5 points=200 ACCEPT   06:51:45 REQ #6 points=53 ACCEPT
06:51:50 REQ #7 points=200 ACCEPT   06:51:50 REQ #8 points=53 ACCEPT

Requests land exactly 5s apart — 8 in 22 seconds, not the 22 an unclamped 1s interval would have produced. 1,012 points accepted, 0 lost, 0 ERROR lines.

I went looking for a hole in d <= 0 and there isn't one on this path. WithInterval(0) is a no-op at the pin (periodic_reader.go), so the SDK would fall back to envDuration(OTEL_METRIC_EXPORT_INTERVAL, 60s) (:40) — which an operator can set to 1000. That would make your README's claim that the exporter raises OTEL_METRIC_EXPORT_INTERVAL false. It is true, because GoFr resolves that variable itself and never hands you a zero: metricsExportInterval returns METRICS_EXPORT_INTERVAL, else OTEL_METRIC_EXPORT_INTERVAL, else 30s (pkg/gofr/container/metrics_exporter.go:91-109). So the README is right and the clamp sees a real value in every GoFr path.

One line worth adjusting: the comment says "Zero is returned as is: it asks for the SDK's default, not for a short interval." At the pin zero asks for whatever OTEL_METRIC_EXPORT_INTERVAL says, defaulting to 60s — so for a caller that builds an exporters.Config directly (the registry is exported) rather than through GoFr, zero can still mean one second. Either say that, or clamp on the zero path too.

The splitter itself

A 20,000-case randomized property test (limits 1–8, 0–3 scopes, 0–3 metrics each, 0–11 points each, three aggregation types, empty metrics and empty scopes included) asserting six invariants — every point preserved in order with its scope and metric, no chunk over the limit, source never mutated, resource preserved, each scope at most once per chunk, and cap == len on every sliced DataPoints. All six hold, 20,000/20,000. Packing is optimal too: exactly ceil(total/limit) requests, never a wasted one.

The shape is right, and it is the only one available

I checked whether the split belongs in the Reader instead, since that is where the SDK puts its own. It does not, and it cannot: metricSdk.Reader (sdk/metric@v1.46.0 reader.go) has four unexported methods — register, temporality, aggregation, cardinalityLimit — so no package outside sdk/metric can implement it. Wrapping the Exporter is the only layer a third party has. Worth saying explicitly, because finding 1 is a consequence of that constraint rather than of a choice you made.

Your exhaustiveness claim also checks out at the pin: metricdata.Aggregation is sealed by an unexported privateAggregation() (metricdata/data.go:47-48), and the five implementing types — Gauge, Sum, Histogram, ExponentialHistogram generic over int64|float64, plus Summary — are exactly the nine cases you handle.

---

1 — Every chunk shares one export timeout, and the SDK's own batcher deliberately does not

PeriodicReader.collectAndExport wraps collect and export in a single r.timeout (periodic_reader.go:244, default 30s at :24), then calls r.exporter.Export(ctx, rm) once (:262). Your loop runs inside that one call, so all N requests draw on one budget. Three lines above, the SDK's own batcher does the opposite, and says why:

// periodic_reader.go:257-259
// The export timeout is applied individually to each batch by using
// the original context.
err = errors.Join(err, r.exportWithTimeout(originalCtx, batch))

This is reachable at defaults. otlpmetricgrpc@v1.44.0 retries by default — Enabled: true, InitialInterval: 5s, MaxInterval: 30s, MaxElapsedTime: 1m (internal/retry/retry.go:21-26) — on Unavailable, Aborted, DataLoss, OutOfRange, DeadlineExceeded, and on ResourceExhausted when the server returns RetryInfo (client.go:187-197). One retried chunk costs at least 5s of the shared 30s, and Google's ingest is exactly the kind of endpoint that answers RESOURCE_EXHAUSTED with RetryInfo.

A/B on the same live app, using the SDK's own batcher as the control — identical binary, identical 700-path load (~504 points, 3 chunks), receiver taking 1s per request, OTEL_METRIC_EXPORT_TIMEOUT=2500:

	requests per cycle	points per cycle	series that reached the backend
A — this PR's wrapper	[200, 200], third aborted	400/504	app_http_response, app_info
B — SDK batcher at 200	[200, 200, 104]	504/504	app_http_response, app_info, requests_total

Three cycles in arm A, and each one the third request is cut at the shared deadline:

06:34:00 REQ #1  points=200  ACCEPT
06:34:01 REQ #2  points=200  ACCEPT
06:34:02 REQ (aborted by client after 1s): context canceled
06:34:10 REQ #3  points=200  ACCEPT
06:34:11 REQ #4  points=200  ACCEPT
06:34:12 REQ (aborted by client after 1s): context deadline exceeded

requests_total — the application's own counter — never arrived at all in arm A, across the whole run. Arm B, same everything, delivered it every cycle and logged zero errors.

The part that matters is not the loss, it is that the split is deterministic, so it is always the same tail that goes — the metrics late in scope/metric/point order, every cycle, for as long as the condition lasts. That is the signature #4266 opens with: "metrics are arriving" and "metrics are complete" are different claims. It is still a large improvement on today's 0/504, which is why this is a note and not a blocker.

Three ways to bound it, cheapest first — your call, and documenting the trade is a fine outcome on its own:

• Turn off transport retry: otlpmetricgrpc.WithRetry(otlpmetricgrpc.RetryConfig{Enabled: false}) (exported, config.go:37,258). Under cumulative temporality the next interval re-sends every cumulative value, so an in-cycle retry recovers one sample while now costing the tail of the same collection. One line, and the argument is specific to this exporter.
• Rotate the starting chunk between cycles, so a persistently short budget loses a different slice each time instead of the same one.
• Say it in the doc comment: that the wrapper's position forces one shared budget, that the SDK's batcher does not share it, and that the loss under a short budget is ordered rather than random.

2 — metricPoints's default: return 1 silently reopens this bug on an SDK bump

chunk.go:165-167 counts an unhandled aggregation as one point and sliceMetric passes it through whole. If a future sdk/metric adds a tenth Aggregation, a metric of N points is budgeted as 1, the chunk reaches up to 199 + N, and the Telemetry API rejects the whole request — the exact failure #4266 reports, arriving silently on a dependency bump.

Nothing in the repo catches it. I checked both directions:

• The linter does not. exhaustive is enabled (.golangci.yml:14), but it covers enum switches, not type switches over an interface — I deleted the metricdata.Summary case and golangci-lint run reported 0 issues.
• The tests catch a removed case but not an added one. The same deletion fails Test_splitResourceMetrics/spans_metrics,_scopes_and_every_aggregation, so the nine you handle are genuinely pinned; there is no construct in Go that fails when a tenth appears.

Cheapest bound: give an unknown aggregation a chunk to itself rather than a seat in someone else's — degradation stays one metric wide instead of taking the request it lands in. A metricPoints that returns (n int, known bool) and a flush on !known is about six lines.

3 — Both coverage figures in the checklist are high

The checklist says exporters/gcp: 90.0% → 95.3%. Measured:

7129f0d54  plain / -race / atomic / count / set / GOWORK=off / -coverpkg  ->  94.7%   (7 invocations, identical)
272478ab9  the same seven                                            ->  94.4%
baseline, chunk.{go,_test.go} removed, package restored to a313baf7f ->  87.8%

87.8% → 94.7%, not 90.0% → 95.3%. The improvement is actually larger than claimed — +6.9 points rather than +5.3 — so nothing about the review changes. But this is the fourth consecutive round where the stated figure does not reproduce (#4206 97.2/95.4, #4207 r2 97.3/96.9, #4207 r3 98.0/97.5, here 95.3/94.7), and it comes after you wrote on #4207 that figures would be quoted from go test -cover in the same session as the claim. Something in how the number is produced is off by roughly a point each time; worth finding that, rather than correcting the number again.

Uncovered at head: metricPoints 90.9%, buildReader 90.0%, Detect 0.0%.

4 — Two more mutants survive than the PR reports

You report 6 mutants, 5 killed. I ran 10; 6 killed, 4 survived. Your disclosed survivor is accurate, and one of mine is equally equivalent:

Mutant	Result
always append a new ScopeMetrics	killed
new chunk resets lastScope to 0	killed
drop min(), hi := lo + s.room()	killed
room(): drop the len(s.out) == 0 guard	killed
room() returns the full limit	killed
swallow a failing chunk's error	killed
resourcePoints(rm) <= limit → <	survived — equivalent, as you say. Identical bytes on the wire
ctx-done break → continue	survived — equivalent bar the number of joined errors
[lo:hi:hi] → [lo:hi]	survived — and this one has a claim attached
unknown aggregation counts 0 instead of 1	survived — dead today; see finding 2

The capacity cap is the right instinct and its comment states a real property — "nothing appended downstream can overwrite a point that belongs to the next chunk" — but no test checks it, so the cap could be dropped in a later refactor with a green suite. One cap(d.DataPoints) != len(d.DataPoints) assertion in checkChunk closes it; my property test's sixth invariant is that line and it kills the mutant.

5 — #4266's own measurement answers your open question

You list as unverified: "that Google counts one histogram data point as one point." The issue settles it. app_http_response is a histogram (pkg/gofr/container/container.go:402), and the reporter measured 191 attribute sets on the instance that was hitting the 200-point ceiling. If Google counted a histogram's buckets, 191 sets would be several thousand points and the cap would have bound at roughly 18 sets, not 191. One data point, one point — confirmed by field data rather than by the SDK's convention. Worth folding into the PR body so the caveat does not outlive its answer.

My receiver counts the same way and the arithmetic closes: 250 scanner paths + /hello = 251 sets of app_http_response, plus app_info and requests_total, = 253 points, which is exactly what the pre-fix binary put in one request.

---

Not flagged

• Declining the issue's suggestions 2 and 3. Right call, and your reason is the better one: once requests are split there is nothing for a derived METRICS_CARDINALITY_LIMIT to protect, and capping legitimate route cardinality to fit a transport limit is the wrong layer paying.
• Keeping the splitter unexported in the gcp module. Moving it later is non-breaking, and the survey of other backends' limits — point caps versus byte caps versus per-point dimension caps, and the note that a promoted version should take a limit struct rather than a bare int — is the kind of thing that makes the follow-up cheap instead of a rewrite. None of it binds this PR.
• Composing with OTEL_GO_X_METRIC_EXPORT_BATCH_SIZE rather than deferring to it. A limit that belongs to the destination should not sit behind an experimental env var every deployment has to discover, and the live arm B above is that composition working on a real app.
• Reading the points back by reflection in pointIDs, independently of the production counter. That is what makes the size assertions mean something rather than restate the code.
• Returning rm unchanged when it already fits. 46.7 ns and one allocation on the common path, and the pooled object reaches the exporter exactly as before.
• Promoting go.opentelemetry.io/otel/metric to a direct require. It is a test-only import; that is what go mod tidy is supposed to do with one.

Adjacent, not this PR — but you have just written the fix next door

Two things the live runs turned up in this module, neither introduced here, both the metrics twin of something #4207 is closing for traces.

Startup blocks for ~95 seconds on a wedged metadata server. Booted with GCE_METADATA_HOST pointed at a listener that accepts and never answers, and no key file, so ADC must reach the metadata server. Matched pair, timestamps from the app's own log:

	resource detector	ADC	port bound after
merge-base a313baf7f	63.48 s	31.33 s	94.82 s
this PR 272478ab9	62.66 s	32.01 s	94.67 s

Identical, so this PR neither causes nor worsens it — but it is the same unbounded google.FindDefaultCredentials and the same context-ignoring detector that #4207 bounds at 2 × metadataTimeout = 10s for traces. buildReader calls FindDefaultCredentials(ctx, ...) with the app's startup context (gcp.go:147) and nothing caps it. Once #4207 lands, its metadataTimeout/awaitWithin shape transfers here almost verbatim.

The scheme-bearing endpoint scar is still here too. docs/memory/scars.md:28-29 records it and says "the pattern still exists in both gcp exporter modules." That code is in buildReader — the same function whose last line this PR changes: it defaults only on "" and hands the value to otlpmetricgrpc.WithEndpoint unvalidated (gcp.go:152-158). #4207 is closing the traces half, including the strings.TrimSpace case. After it lands this is the last place the scar lives.

Fold them in or file them, either is fine; neither is this PR's job.

Evidence

go vet ./...                                     clean
golangci-lint run ./...                          0 issues
go mod tidy -diff                                exit 0
go test -count=1 -race -cover ./...              ok, 94.7%
CI at 7129f0d54                                  15 pass, 1 skipping, 0 fail
property test, 20,000 cases, 6 invariants        pass
mutants                                          10 run, 6 killed, 4 survived
live e2e, before                                 0/506 points accepted, 100% loss
live e2e, after                                  506/506, max 200 per request, 0 errors
live e2e, 8 cycles at ~504 points                4,032 accepted, 0 rejected
live e2e, 50% injected chunk failures            704 accepted, rest of each collection still lands
live A/B on the export timeout                   400/504 vs 504/504; requests_total absent in arm A
live at 7129f0d54, METRICS_EXPORT_INTERVAL=1      clamped to 5s, requests 5s apart, 1,012 accepted, 0 lost
BenchmarkSplit fits/200                          46.69 ns/op   8 B/op   1 alloc/op
BenchmarkSplit splits/504                        567.8 ns/op   1168 B/op  19 allocs/op
BenchmarkSplit splits/2002                       1336 ns/op    2866 B/op  51 allocs/op

The receiver is a TLS gRPC MetricsService that counts OTLP data points and rejects any request over 200 with InvalidArgument: A maximum of 200 points can be written in a single request., plus a local OAuth token endpoint so the exporter's ADC path runs unchanged. The app is examples/using-gcp-metrics built from each commit, unmodified.

Pins read for this review: sdk/metric@v1.46.0 (periodic_reader.go:24,244,257-262, reader.go, metricdata/data.go:47-48), otlpmetricgrpc@v1.44.0 (config.go:37,258, client.go:187-197, internal/retry/retry.go:21-26), x/oauth2@v0.37.0 (google/google.go:157,161-162).

[aryanmehrotra]

Approved, and the three notes that shouldn't get lost on merge are filed together as #4293 — the shared export timeout, the unknown-aggregation case, and the pre-existing ~95s metadata block. Each one has the measurement behind it; none blocks this.

Two things you can fix in a comment rather than a push: the coverage line reads 90.0% → 95.3%, measured 87.8% → 94.7% (seven invocations at 7129f0d54, identical every time — the improvement is bigger than claimed, not smaller); and the "not verified: Google counts one histogram data point as one point" caveat is settled by #4266's own data — app_http_response is a histogram and the reporter hit the cap at 191 attribute sets, which only works if a data point is one point.