feat(tracing): keyless trace export to Google Cloud (TRACE_EXPORTER=gcp)

[akshat-kumar-singhal]

When traces actually reach GCP: the module's go.mod requires gofr.dev v1.60.1 with a local replace, and a replace in a dependency's go.mod is ignored by the consuming main module. So an external service can export only once the trace exporter registry (#4206, now merged) ships in a tagged gofr.dev release, this module's go.mod is bumped to that release with the replace dropped, and the submodule is tagged pkg/gofr/traces/exporters/gcp/vX.Y.Z. That release-ordering constraint is tracked in #4210 — it needs action at tag time, not just at merge time. Inside this repo it works today via go.work, which is what examples/using-gcp-traces exercises.

Description:

Closes #4204.

Adds pkg/gofr/traces/exporters/gcp, an optional module that exports spans directly to Google Cloud's Telemetry (OTLP) API using Application Default Credentials. On Cloud Run a GoFr service now gets traces into Cloud Trace with an IAM role alone — no key file, no Collector sidecar:

TRACE_EXPORTER=gcp

A vendor exporter is needed here rather than a static TRACER_HEADERS value because Google's tokens expire in about an hour: a static header authenticates once and then silently stops working. The per-RPC token source from ADC refreshes on its own.

The module also sets the gcp.project_id resource attribute from the ambient credentials, falling back to GOOGLE_CLOUD_PROJECT, and warns when neither resolves. Google's guidance indicates spans may be routed by that attribute but does not pin it down for a plain Go gRPC exporter; setting it is correct either way, and it is exactly what the registry's resource-detector seam from PR 2 exists for.

Docs name roles/telemetry.tracesWriter. roles/cloudtrace.agent — which the issue asks for — is not sufficient: it authorizes the older Cloud Trace API, which this exporter never calls.

Also included: examples/using-gcp-traces with a Cloud Run deploy recipe, the new module in go.work and in dependabot's gomod globs, and gcp / TRACER_INSECURE entries across the tracing docs.

Breaking Changes (if applicable):

None. This is a new opt-in module; gofr.dev's own go.mod gains no Google dependencies. Existing TRACE_EXPORTER values are untouched.

Additional Information:

• New dependencies live only in pkg/gofr/traces/exporters/gcp/go.mod — golang.org/x/oauth2/google for ADC and the OTLP gRPC exporter. Users who do not import the module pay nothing.
• The module carries a temporary replace gofr.dev => ../../../../.. because no tagged gofr.dev release yet contains pkg/gofr/traces/exporters. It must be removed once feat(tracing): add a trace exporter registry, resource and shutdown flush #4206 ships in a release; that is tracked in gcp traces submodule: drop the replace gofr.dev once a release contains pkg/gofr/traces/exporters #4210 and referenced from the // TODO(#4210): in the module's go.mod. An earlier revision of this description said it was "tracked for removal" when no issue existed — raised in review, now true.
• The dependabot comment's module count is corrected from 37 to 34, with the command that produces it (git ls-files '*go.mod' | wc -l) named alongside so the number stays checkable — that comment's job is to assert the globs cover every module, so a wrong count sends the next reader hunting for modules that do not exist.
• Not yet exercised against a live GCP project — the IAM role and the gcp.project_id routing question are documented from Google's own docs, and confirming them end to end needs a real Cloud Run deploy of examples/using-gcp-traces.

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.
• I have reviewed the code comments and documentation for clarity.

[aryanmehrotra]

Reviewed at 238fea7be. The module boundary is exactly right and I measured it rather than trusting it. Two small factual corrections and one inherited defect.

Verified

Claim	Result
gofr.dev gains no Google dependencies	✅ main go.mod/go.sum untouched by the whole stack
Users who do not import it pay nothing	✅ binary byte-identical to #4206 — 61,118,450 bytes at both commits, +0
Dependabot extended	✅ /pkg/gofr/traces/exporters/* added; every module in the tree matches a glob
gofmt / build / vet / module tests	✅ ok gofr.dev/pkg/gofr/traces/exporters/gcp

For scale: the whole three-PR stack costs a non-GCP user +16.5 KB on a 61 MB binary (+0.027%), and this PR — the largest of the three — costs zero. The submodule-plus-blank-import pattern does what it promises. The new module is also lighter than the precedent it copies: 7 direct / 21 indirect / 71 go.sum lines against metrics/exporters/gcp's 8 / 27 / 91.

The no-credentials path is genuinely well built. With ADC unavailable and the metadata server unreachable:

WARN  gcp traces: no gcp.project_id could be resolved from the ambient credentials and this host
      is not on Google Cloud; spans may not reach a project. Set GOOGLE_CLOUD_PROJECT=<project-id>
ERROR failed to initialize "gcp" trace exporter: gcp traces: resolving application default
      credentials: ... open /nonexistent/creds.json: no such file or directory; tracing is disabled
BOOT SURVIVED: recording=false traceIDvalid=true spanIDvalid=true shutdownFunc=true

The app starts, the message names the remediation, and correlation IDs stay valid. That is the behaviour I would want and it is not the default one gets for free.

1. The dependabot comment count is wrong

comment: "The tree has 37 go.mod directories"
measured: df192fc08 = 32  ->  this PR adds 2  ->  34

I counted at all three stack commits: 32 / 32 / 34. Off by three. Normally a nit, except that comment's entire job is "the globs below must cover all of them" — an inflated count tells the next reader three modules are unaccounted for and sends them looking for something that is not there. (I checked: every module does match a glob.)

2. "It is tracked for removal" is not true yet

The replace gofr.dev => ../../../../.. carries a // TODO: and the description says it "is tracked for removal". I searched open and closed issues — there is none.

This matters more than a normal TODO because the constraint is load-bearing: a replace in a dependency module is ignored by consumers, so go get gofr.dev/pkg/gofr/traces/exporters/gcp resolves gofr.dev v1.60.1 from the proxy, which does not contain pkg/gofr/traces/exporters. Nobody can use this module until a tagged release ships #4206. That is a release-ordering constraint someone has to remember at tag time, and a // TODO: in a go.mod is the weakest possible place for it — the same argument that moved #3940's deferred gate onto #4155. Could you file it?

3. buildExporter panics on a nil Logger

buildExporter(ctx, cfg, nil) -> panic: nil pointer dereference

Same root cause I have described on #4206: noopLogger exists and is documented as the fallback for callers that supply no logger, but nothing substitutes it. Inherited from metrics/exporters rather than introduced here — I confirmed that package panics identically — so it is a three-package fix, not a defect of this PR. Noting it here only so the GCP module is on the list.

Not verifiable from here

Your own caveat that this has not been exercised against a live GCP project is the right thing to have written down, and it is the part I cannot close: the roles/telemetry.tracesWriter over roles/cloudtrace.agent claim and the gcp.project_id routing question both rest on Google's docs. That is a maintainer call on whether to merge ahead of a real Cloud Run deploy of examples/using-gcp-traces, not a review finding.

Items 1 and 2 are small and worth fixing before merge; item 3 is shared with #4206.

[akshat-kumar-singhal]

Thanks — measuring the binary at both commits rather than taking the claim is the check I would have wanted, and "+0 bytes, byte-identical" is a better statement of the module boundary than anything in my description. Both of your items are fixed in cd33afcb; the branch is also rebased onto the updated #4206.

1. The dependabot count. You are right, and I re-derived it independently rather than just taking the number: git ls-files '*go.mod' | wc -l = 34 at this commit. (A plain find says 37 here, but three of those are vendored go.mod files under my local .scratch/ — worth mentioning because it is the kind of thing that makes two people disagree about a count and both be sure.) The comment now says 34 and names the command that produces it, so the next person can re-check it in one line instead of trusting it. Your point about why it matters is the reason I did not just bump the digit: the comment's whole job is to assert the globs cover every module.

2. "It is tracked for removal." Fair hit — that was not true when I wrote it. Filed as #4210, which spells out the ordering (#4206 merges → a tagged gofr.dev contains pkg/gofr/traces/exporters → bump and drop the replace → tag the submodule) and the done-when: a go get of the module from outside this repo succeeds. The go.mod comment is now // TODO(#4210): and also states the constraint itself rather than just the cleanup, and the PR description points at the issue instead of asserting the tracking.

3. Nil Logger. Closed from the other end, in #4206: Build now substitutes noopLogger, and Builder's doc states that the logger passed to a builder is never nil. buildExporter is unexported and only ever reached through Build, so it needs no guard of its own once that contract holds — the panic you hit came from calling it directly. pkg/gofr/metrics/exporters is untouched and still has it; that is its own PR.

On the live-GCP caveat — agreed that is a maintainer call, and I would rather it be made with the caveat visible than have me quietly drop it. If you would prefer the deploy evidence first, I am happy to run examples/using-gcp-traces on Cloud Run and post what Cloud Trace actually shows, including which project the spans land in, before this merges.

[aryanmehrotra]

Reviewed by planning #4204 independently first, freezing that plan, then reading the diff and comparing the two blind. Base origin/development @ 4f3e02dfc, branch @ e9ce0f807. Only this PR's own commits (60e15bdf1, ecfe5f371) — 355bfc4f4/0fd9cb5b7 are #4206's.

The module is a faithful port and the wiring is complete. Same ADC lookup, same WithTLSCredentials(credentials.NewTLS(nil)) + WithPerRPCCredentials(oauth.TokenSource{...}), same init() double registration, same caching detector returning a partial resource plus its error. go.work gets both entries, the dependabot glob was genuinely required (nothing matched pkg/gofr/traces/*), and the count correction to 34 is right — I measured 32 go.mod files at base, +2 here. go test -race -cover → ok, 93.9%; the example builds.

Two things to fix before merge, one comment to correct, and one thing I got wrong that you should ignore.

---

1 — a scheme-bearing TRACER_URL drops every span, and this PR's own docs promise it works

gcp.go:173-179 passes cfg.Endpoint straight to otlptracegrpc.WithEndpoint. Measured at the pinned otlptracegrpc v1.44.0:

endpoint "telemetry.googleapis.com:443"
  -> dial attempted (expected)

endpoint "https://telemetry.googleapis.com:443"
  -> rpc error: code = Unavailable
     desc = invalid target address https://telemetry.googleapis.com:443,
     error info: address https://telemetry.googleapis.com:443:443: too many colons in address

It fails at export, not at startup — so the app boots healthy, logs exporting traces to Google Cloud at …, and silently exports nothing. That is the exact failure class #4204 was filed about.

And this is the documented usage, not a misuse. This PR's own edit to the TRACER_URL row in docs/references/configs/page.md reads:

Required if TRACE_EXPORTER is set to zipkin, jaeger or otlp; optional for gcp, which defaults to telemetry.googleapis.com:443. An http:// or https:// scheme selects the transport; a schemeless host:port is governed by TRACER_INSECURE.

One sentence tells the operator that TRACER_URL applies to gcp and that a scheme selects the transport. For gcp it does not. The most natural path to this is switching TRACE_EXPORTER from otlp to gcp and leaving TRACER_URL in place — and the module's own doc comment invites a custom endpoint for regional residency (gcp.go:17-19).

The fix already exists 40 lines away in the same package: traces/exporters/otlp.go:44-85, whose comment spells out this exact failure, and #4205 merged it for the otlp builder four days before this branch.

Cheapest fix: reuse resolveOtlpTransport, or reject/normalise a scheme in buildExporter and return an error so it fails at startup instead of silently at export.

Note this is inherited, not introduced — metrics/exporters/gcp/gcp.go:149-155 has the identical pattern, so it is one fix across two modules.

2 — the IAM role is asserted as fact, contradicts #4204, and omits the role its metrics twin requires

gcp.go:9-15 states roles/telemetry.tracesWriter (or roles/telemetry.writer) and that roles/cloudtrace.agent is "NOT sufficient". Three sources, three answers:

source	says
issue #4204 (yours)	telemetry.traces.write is "the sole permission in roles/telemetry.tracesWriter, and included in roles/cloudtrace.agent"
this PR	roles/cloudtrace.agent is "NOT sufficient"
this repo, same telemetry.googleapis.com endpoint (metrics/exporters/gcp/gcp.go:9-13)	roles/telemetry.writer plus roles/serviceusage.serviceUsageConsumer on the quota project

To be clear: I could not verify this and I am not claiming the PR is wrong. Google's IAM role pages are JS-rendered and returned no permission list; gcloud iam roles describe needs an interactive reauth I did not have. What I am reporting is that a control is stated as fact while this PR's own issue says the opposite, and the repo's nearest precedent names a different role plus a second one.

The serviceUsageConsumer omission is the part that worries me more than the cloudtrace.agent line. The module already knows quota projects matter — gcp.go:186-190 warns about x-goog-user-project and points at GOOGLE_CLOUD_QUOTA_PROJECT. If the quota-project grant is needed here as it demonstrably is for metrics, an operator following these docs gets a PERMISSION_DENIED that reads nothing like a missing role, while three separate places (package doc comment, README, deploy recipe) tell them the grant was complete. Role strings get copied verbatim into production IAM bindings.

Cheapest fix: one command settles all three —

gcloud iam roles describe roles/cloudtrace.agent      --format='value(includedPermissions)'
gcloud iam roles describe roles/telemetry.tracesWriter --format='value(includedPermissions)'

Until then, mark it documented-not-verified (the PR body is already admirably honest about not having run this against a live project — the doc comment should match that tone), and say explicitly whether serviceUsageConsumer applies.

3 — the replace comment's history is wrong (keep the directive, fix the two sentences)

gcp/go.mod:6-8:

The metrics exporter module carries no replace for exactly this reason — it was added only after the core it needs had shipped.

It shipped with one:

$ git show 5e00b8c6d:pkg/gofr/metrics/exporters/gcp/go.mod
 8:  gofr.dev v1.57.0
44:  replace gofr.dev => ../../../../../

It was dropped 16 days later in ceb503900 (#3926) by pinning v1.59.0 — i.e. only once a release contained the parent package, which is precisely the four-step ordering #4210 describes. So the conclusion is right and only the stated history is wrong; the correct history actually supports what you did more strongly than the version in the comment.

⚠️ A finding I withdrew — please ignore any earlier impression that the replace is removable

I initially concluded the replace gofr.dev directive was dead code, having measured that go list -m and go test both ignore it because go.work wins. That was wrong, and you should not act on it.

CI runs cd "$dir" && go mod tidy -diff for every find pkg -name go.mod (.github/workflows/go.yml:604-616), and go mod tidy is workspace-blind:

with the replace, as you shipped it:
  go mod tidy -diff  ->  exit=0

with the line deleted:
  go mod tidy -diff  ->  exit=1
  go: gofr.dev/pkg/gofr/traces/exporters/gcp imports
      gofr.dev/pkg/gofr/traces/exporters: module gofr.dev@latest found (v1.61.0),
      but does not contain package gofr.dev/pkg/gofr/traces/exporters

Removing it would turn the tidiness job red on the first push while the submodule test job stayed green — which is exactly why my first measurement missed it. The directive is load-bearing and #4210 is correctly scoped. Recording the correction here rather than quietly dropping it, because the transferable lesson is that go test and go mod tidy have different blindnesses and only the latter covers a submodule's own replace.

---

Notes, not blocking

• The ADC-failure path is structurally untestable as written. buildExporter uses the package global defaultADC (gcp.go:168) rather than the overridable field the detector has (cachingDetector.adc), so the one failure path that affects application start — ADC unresolvable, must degrade not abort — cannot be driven from a test. The metrics precedent never tests it either: every case in its gcp_test.go calls writeADC(t) first, which makes FindDefaultCredentials succeed. If you want it covered, injecting adc into the builder the way the detector already allows is the small change; unsetting GOOGLE_APPLICATION_CREDENTIALS alone is not enough on a GCE runner, GCE_METADATA_HOST has to go too.
• Rebase needed. The branch is 4 commits behind development and conflicts in pkg/gofr/otel.go and pkg/gofr/otel_test.go. It is missing #4247 (redact tracer config in logs), a security fix — so the redaction behaviour should be re-verified after the rebase rather than assumed from this review.

Things I checked and am deliberately not flagging

Including two where your approach beat my independent plan — expand

• Honouring cfg.Endpoint with a defaultEndpoint fallback. My independent plan proposed a fixed endpoint that ignores Config.Endpoint entirely. A blind comparison of the two plans called yours the simpler shape and it was right: mine would have created a second endpoint policy for the same destination and contradicted docs/references/configs/page.md's documented METRICS_URL behaviour for the metrics twin. Your shape is correct — finding 1 is about validating the value, not about rejecting the knob.
• Correctly not porting the metrics-only constraints. The prometheus_target location/instance label lists and the cumulative-temporality guard are Managed Prometheus concerns; Cloud Trace has no prometheus_target. Porting them would have produced warnings an operator cannot act on. You kept the reusable half — Config.Resource published before builders run, consumed by warnMissingProject — which is exactly the right half.
• TRACER_INSECURE ignored under gcp. My plan wanted a warning; I withdrew it. The config reference already scopes the key ("Supported for otlp, jaeger"), the repo's convention is a doc comment (metrics/exporters/config.go:38-40), and nothing is downgraded since TLS is forced. A confused operator at worst, not a security regression.
• gcp.project_id resolution — ADC → GOOGLE_CLOUD_PROJECT → warn, with all three branches tested. The authorized_user-carries-no-project rationale in the comment is accurate.
• Token expiry — grpc.WithPerRPCCredentials returns an opaque DialOption with no public accessor, so correct-by-construction is the honest bar here; not unit-assertable.
• Zero Google dependencies in gofr.dev's go.mod — confirmed unchanged.
• The stated limitations — "not yet exercised against a live GCP project" and the go get blocker deferred to #4210 are both stated plainly in the PR body. That is the right disposition and it is why finding 2 is about the doc comment's tone, not the PR's.
• CI needs no edit — submodules are discovered by find pkg -name "go.mod" (go.yml:433-437), so the new module is tested for free once the go.work entry exists.

---

Evidence: go test -race -cover ./... in the module ok 93.9% · go build ./... in examples/using-gcp-traces ok · go mod tidy -diff exit 0 as shipped · git merge-tree vs development → conflicts in otel.go, otel_test.go.

Merge #4206 first, then rebase this.

[aryanmehrotra]

Re-reviewed at dee9d6728, now that #4206 is on development. I ran the gates locally rather than reading the badge, and checked out the branch to do it.

The mechanical half is fully closed, and the rebase is better than I asked for.

• git merge-base is 0711243b5 — the tip of development itself, 0 commits behind, conflicts 2 → 0, three commits, no merge commits. That also means #4247 landed under this, so I re-verified rather than assumed: redactExporterName is live on the path this module registers through (provider.go:122).
• All 28 pkg/ submodules pass CI's own tidiness loop, exit 0. dee9d6728 fixed the one that was red, and the diagnosis was right — the merge from development bumped x/oauth2, x/sys and x/text in the root, and the replace means this module inherits them, so its go.mod went stale without anything in the module changing. Worth recording as a standing cost: every root dependency bump lands here until #4210 closes.
• The dependabot correction checks out. git ls-files '*go.mod' | wc -l is 34 (28 pkg/ + 5 examples/ + root) against 32 at base, and I confirmed all 34 directories are matched by a glob — /pkg/gofr/traces/exporters/* closes the new one. Citing the command in the comment so the number stays checkable is the right instinct.
• go vet clean, module tests -race ok 93.9%, ./pkg/gofr/traces/... ok, ./pkg/gofr/ ok, examples/using-gcp-traces builds and vets clean, CI 16 pass / 1 skip / 0 fail.

Two new things this pass, and a correction I owe you. gcp.go is byte-identical to the version I reviewed, so findings 1–3 still stand as written — my earlier review shows as stale against e9ce0f807 but isn't.

---

New 1 — gcp.go:200 logs TRACER_URL unredacted

This one I missed the first time and it is the one I would fix first.

RedactURL's own doc comment (redact.go:17-19) is addressed at precisely this situation:

Every log line in this package that names an endpoint goes through it, and a Builder registered from outside should do the same: TRACER_URL is operator input and routinely carries a credential.

otlp.go:130, otlp.go:77 and zipkin.go:37 all comply. gcp.go:200 is the one endpoint log in the tree that does not:

logger.Infof("exporting traces to Google Cloud at %s via keyless ADC", endpoint)

Measured against the real function, same three inputs through both paths:

TRACER_URL  "https://svc:s3cr3t@telemetry.googleapis.com:443"
  otlp/zipkin -> ... at https://REDACTED@telemetry.googleapis.com:443
  gcp.go:200  -> ... at https://svc:s3cr3t@telemetry.googleapis.com:443

TRACER_URL  "https://telemetry.googleapis.com:443?api-key=AIzaLIVEKEY"
  otlp/zipkin -> ... at https://telemetry.googleapis.com:443?REDACTED
  gcp.go:200  -> ... at https://telemetry.googleapis.com:443?api-key=AIzaLIVEKEY

TRACER_URL  "telemetry.googleapis.com:443\nlevel=INFO msg=\"forged line\""
  otlp/zipkin -> ... at telemetry.googleapis.com:443\x0alevel=INFO msg="forged line"
  gcp.go:200  -> ... at telemetry.googleapis.com:443
                 level=INFO msg="forged line"

The third is the one the doc comment specifically calls out — "a raw newline would otherwise forge a second log line in the terminal's pretty output" — and it does.

I want to be accurate about the severity: TRACER_URL is operator config, not remote input, so this is not a remote-attacker path. But #4247 judged exactly this class worth a security fix four days ago, and the rule is written down in the function the module already links against. A new module landing on the other side of it is how a just-closed hole reopens.

Fix: RedactURL(endpoint). One call, already exported, already imported.

New 2 — examples/using-gcp-traces/go.mod is untidy, and CI cannot see it

go mod tidy -diff exits 1 with a 72-line diff. The module carries indirects older than the root it replaces:

dependency	this example	root go.mod	examples/using-gcp-metrics
cloud.google.com/go/auth	v0.22.0	v0.23.2	v0.23.2
googleapis/gax-go/v2	v2.23.0	v2.24.0	v2.24.0
enterprise-certificate-proxy	v0.3.19	v0.3.20	v0.3.20

Same root cause as the pkg/ module dee9d6728 fixed — it is downstream of the root through a replace, so the merge moved it. It was missed because CI's gate is find pkg -name go.mod, and this one is under examples/. It is the only untidy module of the five examples.

Two things follow, and the second is arguably the more useful:

1. go build hides it, because go.work overrides the stale requirements. An example is the one artifact people copy out of the workspace, and outside it the first go mod tidy rewrites 72 lines.
2. The exclusion's stated reason is no longer true. go.yml:601-603 says examples are skipped because examples/using-gcp-metrics is currently untidy — I measured it tidy today, along with the other three. So examples are ungated by accident rather than by decision, and this PR's own example is now the only thing that exclusion is hiding. Worth a follow-up to re-scope the gate to find pkg examples -name go.mod; not this PR's job, but this PR is what surfaced it.

Correction — my suggested fix for finding 1 was wrong

I wrote "reuse resolveOtlpTransport". That is not available to you. It is unexported, and gcp is a separate module — the exported surface of traces/exporters is exactly Build, Register, RegisterResourceDetector, RedactURL, RedactMessage.

It would also be the wrong shape. This destination is always TLS on 443, so there is no transport to select; the scheme is a value to reject, not interpret. The cheapest fix is local to buildExporter: return an error when cfg.Endpoint carries a scheme, so it fails at startup. Exporting a helper from the parent is a reasonable alternative, but it is a bigger change than I implied and I should not have described it as the cheap one.

---

Findings 1–3, still open

1 — a scheme-bearing TRACER_URL drops every span. Unchanged at gcp.go:173-179. Two things sharpen it since I filed it.

It is now provably outside the test suite's reach. I replaced otlptracegrpc.WithEndpoint(endpoint) with a hardcoded invalid target, discarding cfg.Endpoint entirely:

go test -run Test_buildExporter ./...   ->  ok
go test ./...                           ->  ok      (the full 93.9% suite)

The mutant survives. Test_buildExporter_endpoint asserts only err == nil && exp != nil, and both of its endpoint cases are schemeless — so the table covers the lines without constraining them. This is the same shape as the Test_buildResource blind spot in #4206, where the case picked was the one key that could not collide. A case is not enough here; the assertion has to be on the resolved endpoint.

And the first symptom is worse than I said. Re-measured at the pinned otlptracegrpc v1.44.0:

"telemetry.googleapis.com:443"
  -> PermissionDenied: unregistered callers        (socket opened, reached Google — the good path)

"https://telemetry.googleapis.com:443"
  -> rpc error: code = Unavailable
     desc = invalid target address https://telemetry.googleapis.com:443,
     error info: address https://telemetry.googleapis.com:443:443: too many colons in address

That second error only surfaces after the exporter's ~30s export timeout. Under a shorter context the operator gets context deadline exceeded and nothing else — a message that points at the network rather than at the config.

The docs contradiction is also unchanged: this PR's own TRACER_URL row still reads "optional for gcp … An http:// or https:// scheme selects the transport", and production-tracing/page.md repeats "ignored when the URL carries a scheme" in the same table that lists gcp.

2 — the IAM role is asserted as fact in four places, not three. gcp.go:11,14, production-tracing/page.md:136-137, quick-start/observability/page.md:450-451, and examples/using-gcp-traces/README.md:37,40,42,58 — the README puts it inside a copy-pasteable gcloud ... --role="roles/telemetry.tracesWriter". Meanwhile roles/serviceusage.serviceUsageConsumer, which the metrics twin's README names at :48 for the same telemetry.googleapis.com endpoint, appears nowhere in this module — even though gcp.go:186-190 already warns about x-goog-user-project and GOOGLE_CLOUD_QUOTA_PROJECT, so the module plainly knows quota projects matter.

I still could not verify this and I am not claiming the PR is wrong. gcloud iam roles describe fails here with Reauthentication failed. cannot prompt during non-interactive execution. What I am reporting is that a control is stated as fact in four places, one of them executable, while #4204 says the opposite. Either run the two describe commands, or soften all four to documented-not-verified and say explicitly whether serviceUsageConsumer applies. Happy to run them the moment I have auth.

3 — the replace comment's history. Keep the directive — I withdrew that finding, and this round's tidy failure is a second demonstration that it is load-bearing. Only the two sentences of history are wrong: the metrics module did ship one (5e00b8c6d line 44, pinning v1.57.0), dropped in ceb503900 once a release contained the parent. That history supports what you did more strongly than the version currently in the comment.

---

Checked and deliberately not flagging

Including where the coverage gaps turned out to be exactly the right ones — expand

• The 6.1% uncovered maps precisely onto the two things already known. withProjectID 85.7% and buildExporter 88.2%, nothing else below 100%. No hidden gap.
• withProjectID's error branch is unreachable, and that is fine. At the pinned otel/sdk v1.46.0, resource.Merge cannot return an error when b is schemaless — case b.schemaURL == "": return NewWithAttributes(a.schemaURL, combine...), nil. Defensive, not dead weight. One latent nit if it ever becomes reachable: on that branch Merge still returns the full combine set, and the code discards it in favour of attrs alone, which would drop every platform attribute. A one-word change if you are touching the function anyway.
• The example's own config is right. configs/.env and README.md:21,105 all use schemeless forms and explain why TRACER_URL is left unset. Finding 1 is about the config reference page, not about the example.
• Dependabot glob coverage is complete, not just incremented — I matched all 34 module directories against the eight globs.
• main.go is the right size for an example: blank import, one custom span, nothing else.
• resolveOtlpTransport being merged on development makes finding 1 a divergence from shipped in-repo behaviour rather than from a sibling branch. That is a reason to fix it, not a reason to reuse it — see the correction above.

---

Evidence: worktree checked out at dee9d6728, every command above run locally, worktree restored afterwards. 28 pkg/ modules tidy · gcp module -race -cover 93.9% · ./pkg/gofr/traces/... and ./pkg/gofr/ ok · example builds and vets clean · endpoint mutation survives the full suite · examples/using-gcp-traces go mod tidy -diff exit 1, 72 lines.

New 1 is a one-line fix and New 2 is a go mod tidy. Findings 2 and 3 are documentation. Finding 1 is the only one with design in it.

[akshat-kumar-singhal]

Pushed as 9c796a54. All five items are closed, and two of them resolved differently from how they were filed — details below, since in both cases the difference matters more than the fix.

New 1 — unredacted endpoint: fixed, and there was a second one

RedactURL(endpoint) on the startup line, as you said. But writing the test for it turned up a second leak on the same input, which is the one I'd have missed without your framing that the rule belongs to the package, not to the call site:

TRACER_URL  "svc:s3cr3t@telemetry.googleapis.com:443\nlevel=INFO msg=\"forged\""

never reaches the startup log at all. otlptracegrpc.New prepends dns:///, parses the result, and fails on the control character — and its error quotes the raw value straight back:

gcp traces: creating OTLP exporter: parse "dns:///svc:s3cr3t@telemetry.googleapis.com:443\nlevel=INFO...": net/url: invalid control character in URL

So the forged-log-line case you measured survives a fix to line 200 alone: it just takes the error path instead. That is exactly the situation RedactMessage's doc comment describes ("an exporter's own error ... where redacting at the point the endpoint is logged is not enough"), so the constructor error now goes through it. Rewritten rather than %w-wrapped, since %w would re-expose the endpoint through Error() — the same reasoning, and the same //nolint:err113, as redactEndpointInError.

Two tests: Test_buildExporter_redactsEndpointInLog (userinfo, query) and Test_buildExporter_redactsEndpointInError (the control-character case).

Finding 1 — scheme rejected at startup

Taking your correction: resolveOtlpTransport is not reachable and would be the wrong shape anyway. buildExporter now calls a local resolveEndpoint, which returns errSchemeInEndpoint for any scheme.

I considered normalising instead — strip https://, error on http:// — and rejected it. It buys a working boot for one spelling at the cost of two behaviours for one input, and it would silently accept https://some-other-host:4317 and hand it a Google OAuth token. Rejecting is one rule with one message that names the fix, and the registry already degrades a builder error to tracing is disabled rather than killing the app, so the cost of being strict is bounded.

Scheme detection is on :// plus an RFC 3986 §3.1 check on the prefix, so telemetry.googleapis.com:443 is untouched and a malformed value doesn't get an error naming a scheme nobody wrote.

On the vacuous test. You were right that a case wasn't enough, and the obvious repair isn't either: asserting on the startup log doesn't help, because the log reads the same variable the option does, so your mutant survives it too. The one observable that constrains it is a socket. Test_buildExporter_dialsTheResolvedEndpoint listens on an ephemeral port, points TRACER_URL at it, and forces the lazy dial; a connection arriving there proves WithEndpoint got the resolved value. The TLS handshake then fails against a bare socket (authentication handshake failed: EOF), which is fine — the socket is the assertion, not the export.

Re-ran your mutation and two others:

mutant	result
WithEndpoint("telemetry.googleapis.com:443") — discard cfg.Endpoint	FAIL (2 tests)
drop RedactURL from the startup log	FAIL (3 tests)
disable the scheme check	FAIL (6 tests)

Coverage 93.9% → 97.3%.

Docs: the TRACER_URL row in the configs reference, the table row and TLS section in production-tracing, the observability quick-start and the example README all now say gcp takes a schemeless host:port.

Finding 2 — I have auth, and the claim was wrong, not just unverified

You were careful to say you weren't claiming the PR was wrong. It was. gcloud iam roles describe, 2026-09-21:

roles/telemetry.tracesWriter   -> telemetry.traces.write
roles/telemetry.writer         -> logging.logEntries.create, monitoring.timeSeries.create,
                                  telemetry.traces.write
roles/cloudtrace.agent         -> cloudtrace.traces.patch, telemetry.traces.write

roles/cloudtrace.agent does carry telemetry.traces.write. The "not sufficient" assertion was false in all four places, including the copy-pasteable one. The reasoning behind it wasn't — cloudtrace.agent is named for the older API — but the role's permission list doesn't follow its name, which is the whole reason you asked for the describe.

All four sites now say what's measured: telemetry.googleapis.com checks telemetry.traces.write; tracesWriter is the least-privilege predefined role carrying it; telemetry.writer and cloudtrace.agent carry it too and grant more besides. The README shows the three describe commands so the next reader can re-check rather than trust the date. I've kept an explicit hedge that the permission an endpoint checks is Google's to change.

On serviceUsageConsumer: you're right that it was missing, and the metrics twin's README already scopes it correctly — it applies to user credentials, not to an attached service account, which resolves its quota project automatically. Carried over in that form to the module doc comment, the tracing guide and the example README.

Finding 3 — directive kept, history corrected

Confirmed both halves of your account:

5e00b8c6d  pkg/gofr/metrics/exporters/gcp/go.mod:44  replace gofr.dev => ../../../../../
ceb503900  fix(metrics): require gofr.dev v1.59.0 in the gcp exporter, drop the local replace (#3926)

The comment now tells that history instead of claiming the metrics module never needed one.

New 2 — example tidied

go mod tidy in examples/using-gcp-traces. All 34 modules now pass go mod tidy -diff, examples included — I ran it over git ls-files '*go.mod' rather than CI's find pkg, so the count is the check.

Your second point is the more useful one and I've filed it rather than folded it in: the exclusion at go.yml:601-603 says examples are skipped because examples/using-gcp-metrics is untidy, and all five are tidy today, so the gate is ungated by accident. Re-scoping it to find pkg examples -name go.mod belongs in its own CI PR off development.

Also

withProjectID's unreachable error branch now keeps merged when it's non-nil, so it wouldn't drop the platform attributes if resource.Merge ever starts failing there.

#4210

Not closeable yet, and I checked rather than assumed: v1.61.0 was tagged 2026-09-17 and contains no pkg/gofr/traces/ at all; #4206 merged 2026-09-18. Step 2 hasn't happened, so the replace stays. I've recorded the state and your standing-cost note on the issue.

Gates at 9c796a54: 34/34 modules tidy · gcp module -race -cover 97.3% · golangci-lint 0 issues · go vet and ./pkg/gofr/traces/... clean · examples/using-gcp-traces builds and vets · 3 mutants all caught.

[aryanmehrotra]

Re-reviewed at 9c796a54b. I built the example as a real binary and ran it against a socket rather than reasoning from the diff, because the thing under test is whether a connection arrives — and that is not something a unit test asserted before this push.

Four of the five items are closed properly, and two of them are better than what I asked for. The second leak you found on the error path is one I measured and missed: I checked the log line and stopped there, and your framing — that the rule belongs to the package, not the call site — is the correct reading of that doc comment. RedactMessage handles it because it replaces the strconv.Quote form as well as the raw one, which is exactly the case the SDK's parse "…" error produces.

The fifth is not closed, and it is in the code this push added. One leading space defeats the new guard entirely.

Edited after posting. I first filed the metadata-server hang below as a note. That was wrong and I have upgraded it to blocking. My reasoning was "the unbounded context is inherited" — but I used that same word about finding 1 in my first review and blocked on it anyway, so the classification was inconsistent with my own standard. What decides it is whether this PR makes a new failure mode reachable, not whose line the root cause sits on. Nothing else in this review changed.

---

Blocking — resolveEndpoint never trims, so the guard is one space wide

schemeOf looks for :// and then validates the prefix against RFC 3986 §3.1. A leading space fails isSchemeRune at position 0, so the function returns "" and the value is accepted as schemeless — and lands in WithEndpoint unchanged.

I ran the built example against a listener that counts connections. Fake ADC, TRACE_EXPORTER=gcp, TRACER_RATIO=1, one GET /hello, 6s for the batch to flush.

TRACER_URL	startup log	dials
127.0.0.1:34317	INFO exporting traces … at 127.0.0.1:34317	4
https://127.0.0.1:34318	ERROR … must be a schemeless host:port … tracing is disabled	0
␣https://127.0.0.1:34319	INFO exporting traces …	0
127.0.0.1:34320␣	INFO exporting traces …	0

Row 3, verbatim:

11:26:15 INFO  exporting traces to Google Cloud at  https://127.0.0.1:34319 via keyless ADC
11:26:15 INFO  {"trace_id":"e9e59af2…","uri":"/hello","response":200}
         … 15 seconds …
11:26:30 ERROR invalid target address  https://127.0.0.1:34319,
               address  https://127.0.0.1:34319:443: too many colons in address
                                                            TOTAL_DIALS=0

That is the original failure, unchanged, straight through the new guard: boots healthy, logs that it is exporting, exports nothing, and the only diagnostic arrives after the export timeout.

Row 4 is the one I would lead with. No scheme is involved at all — a correctly spelled host:port with one trailing space gives dial tcp: lookup tcp/34320 : unknown port, and zero dials. So this is not really a hole in the scheme check; it is that the endpoint is never normalised, and the scheme check is only the most visible thing that falls through it.

Nothing upstream trims. config.Get is a bare os.Getenv (pkg/gofr/config/godotenv.go:93-95). godotenv v1.5.1 trims unquoted .env values (parser.go:173) but for a quoted value trims only the quote character, so TRACER_URL=" https://…" keeps the space. And the plain environment-variable path — Cloud Run, GKE, docker -e, a K8s manifest with a stray space after the colon — has no trimming anywhere. That path is this exporter's whole reason to exist.

The same file already does this for the other operator-supplied value: gcp.go:145, strings.TrimSpace(os.Getenv(projectEnv)).

Fix, and I verified it is sufficient rather than assuming:

func resolveEndpoint(cfg *exporters.Config) (string, error) {
	endpoint := strings.TrimSpace(cfg.Endpoint)
	if endpoint == "" {
		return defaultEndpoint, nil
	}
	...
}

Rebuilt the example with that one line and re-ran the same two cases:

TRACER_URL	with TrimSpace	dials
␣https://127.0.0.1:34321	ERROR … must be a schemeless host:port … tracing is disabled	0
127.0.0.1:34322␣	INFO exporting traces … at 127.0.0.1:34322 (trimmed)	4

Worth a case in Test_resolveEndpoint for each: a padded scheme-bearing value must reject, a padded schemeless one must resolve to the trimmed form.

Blocking — this PR makes an unbounded startup context reachable, and a wedged metadata server stops the app from serving at all

I went looking for the performance cost of the exporter and found this instead.

Normal startup is free: TRACE_EXPORTER=gcp boots in 33–107 ms off-GCP, indistinguishable from tracing off, because the metadata probe is refused instantly. But if something accepts the connection and never replies, boot blocks for exactly as long as it hangs:

GCE_METADATA_HOST	boot to first 200
absent (connection refused)	33–107 ms
wedged, 12s — four runs	11652 / 11585 / 11567 / 11625 ms
wedged, 20s	19614 ms
wedged, tracing off (control)	99 / 34 ms

The control is what isolates it. The HTTP server never binds; there is no deadline anywhere on the path:

otel.go:74        exporters.Build(context.Background(), ...)   <- no deadline
  provider.go:56    buildResource(ctx, ...)
  provider.go:199   resource.New(ctx, opts...)
  provider.go:192   resource.WithDetectors(d)
  gcp.go:71         cachingDetector
                    gcpdetect...Detect(ctx) -> HTTP to GCE_METADATA_HOST, unbounded

The context.Background() is not yours — it is otel.go:74, merged in #4206. But until this PR there was no trace resource detector for lookupDetector to find, so nothing on that path ever opened a socket. Registering the first one is what turns a latent unbounded context into a startup hang.

Why I am blocking on this rather than filing it. In the environment this exporter is built for, a container that never binds its port never passes its startup probe — so the failure presents as a Cloud Run deploy that will not go green, with nothing in it that points at tracing. That is a worse failure than the dropped spans I blocked on above, and this PR is what makes it reachable.

The metrics side already ships it — container/container.go:141 is also context.Background() and metrics/exporters/gcp/gcp.go:65 registers its own cachingDetector, so METRICS_EXPORTER=gcp has this today on development. That parent fix belongs in its own PR off development, and I am not asking for it here. What I am asking is that this PR not be the thing that ships the hang to the traces path: a deadline on the context the detector is given, or on the detector itself, is local to this module and enough.

Reproduction: a TCP listener that accepts and never replies, GCE_METADATA_HOST pointed at it, and the built example. Four runs at a 12s hang, the same four with tracing off as the control.

---

Your mutation work — reproduced, and the vacuous test is genuinely fixed

Test_buildExporter_dialsTheResolvedEndpoint is the right answer, and your reasoning for it is the part worth keeping: asserting on the startup log would not have worked, because the log reads the same variable the option does. The socket is the only observable that constrains it. The comment explaining why the wait needs its own timer is the kind of thing that stops someone "simplifying" it back into flakiness later — I ran it 8 times and the full suite under -race 3 times, all green.

The mutant from my last review is now caught. I re-ran the exact one — hardcode WithEndpoint to an invalid target, discarding cfg.Endpoint:

before this push:  go test ./...  ->  ok     (survived the whole 93.9% suite)
at 9c796a54b:      go test ./...  ->  FAIL   Test_buildExporter_dialsTheResolvedEndpoint
                                             Test_buildExporter_redactsEndpointInError

I also ran two of my own beyond your three — disable the scheme check by making the branch unreachable, and drop RedactMessage from the constructor error. Both caught, 3 tests and 1 test respectively. Five mutants, five kills.

(Our failure counts differ on the shared three only because we mutated at different sites; the kills are what matter and they agree.)

One number does not reproduce. Coverage is 96.9%, not 97.3% — deterministic across six invocations (-race, plain, -coverpkg, GOWORK=off, three repeats). It changes nothing about the review; the tests are real and the mutation results are the stronger evidence anyway. Flagging it because this is the second time a coverage figure in this stack has been a few tenths high — 97.2% against a measured 95.4% on #4206 — and a number offered as evidence costs more later than the fraction of a point is worth. Worth checking what is producing it, since twice is a habit rather than a slip.

---

Finding 2 — you settled it, and the answer is that I was too generous

I said I was not claiming the PR was wrong. It was, and gcloud iam roles describe is what shows it: roles/cloudtrace.agent does carry telemetry.traces.write, so "NOT sufficient" was false in all four places including the copy-pasteable one. I still cannot run that command — my auth is expired, and it is unchanged — so I am accepting your output rather than verifying it. Publishing the three describe commands in the README is what makes that acceptable: the next reader re-checks in ten seconds instead of trusting either of us, and the hedge about the permission being Google's to change is the right amount of caution.

The serviceUsageConsumer scoping is better than what I asked for. I flagged it as missing; you found that it applies to user credentials rather than to an attached service account, which is a distinction I did not make and which the metrics README already had right.

Findings 1 and 3, and New 2 — closed

• Scheme rejected at startup. Your reasoning for rejecting rather than normalising is right and is the part I would not have written: stripping https:// would silently accept https://some-other-host:4317 and hand it a Google OAuth token. One rule beats two behaviours. Confirmed live — row 2 above is a clean tracing is disabled with the fix named in the message, the app still serves 200, and trace IDs are still generated, so correlation survives.
• go.mod history now tells the real sequence. Directive kept.
• 34/34 modules tidy, examples included — I ran it over git ls-files '*go.mod' and got exit 0. Filing the CI re-scope separately is the right call; it is a development change, not this PR's.
• withProjectID now keeps merged when non-nil. Correct: Merge returns the combined set alongside the error, so the old branch would have dropped every platform attribute.

---

Notes, not blocking

• The x-goog-user-project warning is case-sensitive, so the README's promise is conditional. parseHeaders stores keys verbatim (pkg/gofr/otel.go:160-165, no lowercasing), and the check is cfg.Headers["x-goog-user-project"] — a case-sensitive Go map lookup. TRACER_HEADERS=X-Goog-User-Project=p fires nothing, while README:62 says "the exporter warns if you do". gRPC lowercases metadata on the wire, so the header still reaches Google; only the warning is silent. Inherited, not introduced — metrics/exporters/gcp/gcp.go:169 is identical, so this belongs with the other cross-module follow-ups rather than here.
• production-tracing/page.md:43 still reads "Ignored when TRACER_URL has a scheme" for TRACER_INSECURE, in the table that lists gcp. For gcp a scheme is rejected, not ignored. Line 42 clarifies it two rows up, so this is cosmetic.
• warnMissingProject runs before resolveEndpoint, so a rejected endpoint still emits a missing-project warning first. Harmless, slightly confusing ordering.
• errSchemeInEndpoint is unexported, so a caller cannot errors.Is it. Low impact — the registry degrades to tracing is disabled either way — but worth a thought if that sentinel is meant to be part of the contract.
• One commit behind development (2411b1804, unrelated router work). No conflict.

Checked and deliberately not flagging

Including the two places your approach beat mine — expand

• The builder-error path does what you said. Verified in the running app, not just in the source: provider.go:130 logs the already-redacted message and returns nil, so tracing is disabled and the app serves normally. Row 2 above is that path executing.
• Trace IDs survive disabled tracing. Case 2 still emits trace_id on /hello, so the NeverSample correlation behaviour from #4206 is intact under this failure mode. That is the scar the old otel.go comment was about, and it still holds.
• RedactMessage covers the escaped form. redact.go replaces both endpoint and its strconv.Quote rendering, which is why your control-character case is caught — the SDK emits the quoted form, and a naive strings.ReplaceAll on the raw value alone would have missed it.
• schemeOf is right on everything except leading whitespace. I probed 26 values: HTTPS://, HtTp://, grpc://, dns:///, unix://, a+b-c.d:// all reject; 1http://, //host, ://host, host:443/v1?x=a://b all pass through, which is what the doc comment says it intends and I agree with — a stray :// should not be reported as a scheme nobody wrote.
• Lint 0 issues, go vet clean, socket test 8/8, full suite -race 3/3, CI 16 success / 1 skipped / 0 failures.
• The exporter itself costs nothing per request, which I measured rather than assumed. 3000 requests to /hello after 200 warmup, same listener, back to back: otlp mean 48µs / p50 35µs / p99 122µs, gcp mean 41µs / p50 34µs / p99 111µs — identical within laptop noise, and correct by design since the exporter sits behind the BatchSpanProcessor and the request path only enqueues. Binary cost of the blank import is +94,016 bytes (+0.2%), smaller than it looks because a plain GoFr app already links 12 Google modules via cloud.google.com/go/pubsub; the import adds only otlptracegrpc and the two detector modules. For anyone not importing it, +0 bytes.
• The example's own config is still right — .env and README use schemeless forms throughout and explain why TRACER_URL is left unset.

---

Evidence: example built as a binary and run six times against a connection-counting listener · strings.TrimSpace patch applied, rebuilt, both bypass cases re-run and fixed · 5 mutants, 5 kills · 34/34 modules go mod tidy -diff exit 0 · golangci-lint 0 issues · coverage measured 96.9% across six invocations · per-request latency and binary size measured against an otlp control · startup measured against absent, wedged and control metadata servers · worktree restored afterwards.

Two blockers. The first is one line and two test cases. The second is a deadline on one context — the parent fix across both modules is not this PR's job, only not shipping the hang to the traces path is. Everything below those is a note.

[akshat-kumar-singhal]

Both blockers are fixed at 9f44f0b5, rebased onto 2411b1804 (0 behind / 6 ahead).

Row 4 was the right one to lead with. It is not a hole in the scheme check — the endpoint was never normalised, and the scheme check is just the most visible thing falling through that gap. strings.TrimSpace once, before both branches, so the empty-fallback, the scheme check and the returned target all see the same value. Your gcp.go:145 pointer is the argument: the file already trimmed the other operator-supplied value and this one was simply missed.

Tests: four table cases (padded scheme, tab/newline-padded scheme, padded schemeless, whitespace-only) plus a padded subtest on the socket test. The trailing-space case is specifically one a log assertion cannot catch, for the same reason the vacuous endpoint test could not be fixed with a log line — it reads the variable the dial reads.

On the second blocker: your proposed fix would have been inert, and the obvious variant is worse

You asked for "a deadline on the context the detector is given, or on the detector itself". I went to add the first and it does nothing:

// contrib/detectors/gcp@v1.46.0 detector.go:35
func (d *detector) Detect(context.Context) (*resource.Resource, error) {
	if !metadata.OnGCE() {   // the context-free variant, i.e. context.Background()

The parameter is unnamed — the context is discarded outright. A context.WithTimeout around that call bounds nothing, and would have shipped as a fix that passes review, passes a test written against a well-behaved stub, and still hangs in production.

The variant that looks safer is worse than inert. oauth2/google stores the context it is handed inside the Credentials it returns and reuses it for every later token refresh (google/google.go:171,223), so a WithTimeout on the ADC lookup resolves credentials fine at boot and then expires export roughly an hour in — a failure with nothing at all pointing back at startup. On GCE the token source is computeTokenSource (default.go:269) and does not capture it, so it would have been clean on exactly the platform this exporter targets and broken off it.

So the deadline is on the wait, not on the context: awaitWithin runs the call on a goroutine with an uncancellable context and gives up after metadataTimeout. It covers both calls on the startup path — the detector and the ADC lookup, since a wedged metadata server hangs both. 5s is the metadata client's own notion of "too slow" (compute/metadata@v0.9.0 metadata.go:74-83), so this borrows a budget rather than inventing one; worst case a wedged server can add to boot is 2x that, against unbounded today.

The abandoned goroutine is the cost of a library that cannot be cancelled. One goroutine, buffered channel so it never blocks on a receiver that has gone, ends when the server answers or the socket dies.

Degradation is deliberately asymmetric, and I checked both against provider.go rather than assuming symmetry: a detector timeout hits logger.Warnf("traces: resource detection was incomplete") and tracing continues on a partial resource (the project still resolves from GOOGLE_CLOUD_PROJECT); a credentials timeout hits spanExporter's tracing is disabled, since there is no token. Both bind the port, which is your point.

The parent context.Background() is untouched, as you asked — otel.go:74 and container/container.go:141 are shared with the metrics path and belong in their own PR off development.

The stub ignores its context on purpose

hangingDetector.Detect(context.Context) discards its context, mirroring the real one. A stub that honored its context would make the test pass against a fix that does nothing in production, which is the same class of vacuity you caught in the endpoint table last round.

Three mutants run:

mutant	result
drop TrimSpace	killed — 5 tests, incl. dialsTheResolvedEndpoint/padded
call the detector unbounded	killed — test hangs, suite times out
call ADC unbounded	killed — adc.get blocked for 31.17s

That last one is worth reading as a measurement rather than a test result: against a listener that accepts and never replies, the unbounded lookup sat for 31.2s. Your 11.6s/19.6s numbers were the detector; the credentials path is its own, longer hang.

Notes

Taken: the TRACER_INSECURE doc row now says gcp rejects rather than ignores a scheme; warnMissingProject moved after resolveEndpoint, so a rejected endpoint no longer gets a warning first about a project it will not use; the example README has a "Startup cost" section documenting the two metadata calls and the asymmetric degradation.

Declined, with reasoning rather than silence: errSchemeInEndpoint stays unexported. An exported symbol cannot be taken back, the registry degrades to tracing is disabled either way, and I would rather export it when something actually needs to errors.Is it.

Deferred as cross-module, as you framed it: the x-goog-user-project warning being case-sensitive, which metrics/exporters/gcp/gcp.go:169 shares verbatim.

Coverage

98.0% on this push — identical across repeats, under -race, and under GOWORK=off.

On your wider point: you are right that a number offered as evidence costs more later than the fraction of a point is worth, and twice is a habit. I cannot diagnose 97.3% after the fact — the code it was measured against no longer exists, and re-deriving it now would be reconstruction, not verification. What I can change is the practice: coverage figures from here are quoted from go test -cover output in the same session as the claim, never carried across one.

Also: 34/34 modules go mod tidy -diff exit 0, golangci-lint 0 issues, -race green.

[aryanmehrotra]

Approving at 9f44f0b54. Both blockers are fixed, and I checked them the way they were found — mutants and a running app, not a reread of the diff.

You were right and I was wrong about the second fix, and the reason matters more than the outcome. I asked for "a deadline on the context the detector is given." I verified your three claims at the pins and all three hold:

• contrib/detectors/gcp@v1.46.0 detector.go:35 takes context.Context unnamed and discards it, and compute/metadata@v0.9.0 metadata.go:125-127 has OnGCE() call OnGCEWithContext(context.Background()) — so the context is thrown away twice over. A WithTimeout there bounds nothing.
• oauth2@v0.37.0 google/google.go:171,223 captures the context in cfg.TokenSource(ctx) for every later refresh, while default.go:269's computeTokenSource does not — so the "safer" variant would have been clean on GCE and expired an hour into every run off it.
• compute/metadata@v0.9.0 metadata.go:74-83 sets c.Timeout = 5 * time.Second, so metadataTimeout borrows a budget rather than inventing one.

What I asked for would have shipped a fix that passes review, passes a test written against a well-behaved stub, and still hangs in production. Putting the deadline on the wait rather than the context is the right shape, and hangingDetector discarding its own context on purpose is what stops the test from certifying a fix that does nothing. That is the same vacuity class as the endpoint table last round, caught by you this time.

---

Blocker 1 — the whitespace bypass

Mutant: drop strings.TrimSpace from resolveEndpoint → killed, by Test_resolveEndpoint/a_padded_scheme-bearing_value_is_still_rejected among others.

Live, examples/using-gcp-traces unmodified, three values an operator could actually set:

TRACER_URL	Result
" https://telemetry.googleapis.com:443 "	rejected at startup: TRACER_URL must be a schemeless host:port, not "https://…": … carries a "https" scheme and Google's OTLP ingest is always TLS on 443; tracing is disabled
" "	trims to empty → default endpoint path, no bypass
" telemetry.us-central1.rep.googleapis.com:443 "	accepted, padding removed

The ordering change shows up too: the padded-scheme run does not log the missing-project warning, because resolveEndpoint now fails before warnMissingProject runs. That was the note, and it behaves as described.

Blocker 2 — the startup hang

Mutants, each against the test written for it:

Mutant	Result
detector call unbounded	killed — panic: test timed out after 45s, Test_cachingDetector_Detect_isBoundedByTheStartupDeadline
ADC call unbounded	killed — adc.get blocked for 31.06s; the startup deadline did not bound it
drop TrimSpace	killed — see above

Your 31.17s reproduces at 31.06s. Live worst case — metadata server wedged (accepts, never answers) and no key file, so both the detector and ADC must reach it:

run 1   16:46:30.607 -> 16:46:40.611   10.004 s
run 2   16:46:53.645 -> 16:47:03.652   10.007 s
run 3   16:47:04.333 -> 16:47:14.337   10.004 s

Exactly 2 × metadataTimeout, reproducible to 3 ms, and the port binds. Degradation is asymmetric as you describe: the detector timeout logs traces: resource detection was incomplete and tracing continues; the ADC timeout logs failed to initialize … tracing is disabled. Both serve traffic.

For scale, I measured the unfixed twin this week while reviewing #4267: metrics/exporters/gcp under the same wedge binds its port after 94.67 s, split 62.66 s in the detector and 32.01 s in ADC. That is what this PR is 10 s instead of. I have filed it, with the measurement, as #4293 — your awaitWithin shape transfers there nearly verbatim once this lands.

Gates

go vet ./...                          clean
golangci-lint run ./...               0 issues
go mod tidy -diff, all 34 modules     0 untidy
CI at 9f44f0b54                       16 pass, 1 skipping, 0 fail
go test -count=1 -race ./...          ok

---

The coverage number — I can diagnose this one, and the answer is useful

You wrote that you could not diagnose 97.3% after the fact because the code no longer existed. This one still exists, so I looked properly rather than reporting a mismatch again.

pkg/gofr/traces/exporters/gcp has 81 coverable statements at 9f44f0b54. Coverage is a ratio of integers, so go test -cover can only ever print k/81:

79/81 = 97.5%     <- what eight invocations give me
80/81 = 98.8%
81/81 = 100.0%

98.0% is not on that lattice. No test selection, no -race, no -covermode, no GOWORK=off, no flake produces it — the number cannot come out of go test -cover on this package at this commit at all. Eight invocations (plain, -race, atomic, count, set, GOWORK=off, -coverpkg, and from the repo root by package path) all print 97.5%, and go tool cover -func agrees. Uncovered: withProjectID 85.7%, buildExporter 94.1% — two statements.

The same test on #4267, where the claim was 90.0% → 95.3%:

commit	statements	reachable near the claim	claimed
272478ab9	108	94.4% / 95.4% / 96.3%	95.3%
7129f0d54	113	94.7% / 95.6% / 96.5%	95.3%

Unreachable there too. So this is not a measurement that drifts — it is a number arriving from somewhere other than the command it is attributed to. That is a much more fixable problem than "be careful", and it is the last open thread between us: paste the literal go test -cover line next time and the whole category disappears. Nothing about this review turns on it.

Notes, closed

Taken, and each is better for it: the TRACER_INSECURE row now says gcp rejects rather than ignores a scheme; warnMissingProject moved after resolveEndpoint; the example README carries a "Startup cost" section that documents the two metadata calls and the asymmetric degradation — which is exactly the thing an operator needs and cannot infer.

Declined with an argument, which is the right way to decline. errSchemeInEndpoint staying unexported is correct: an exported symbol cannot be withdrawn, the registry degrades to tracing is disabled either way, and there is no caller wanting to errors.Is it. Export it the day one exists.

Deferred correctly. The case-sensitive x-goog-user-project check is shared verbatim with metrics/exporters/gcp/gcp.go:169; moving both together in one PR is the right shape, and I would rather see it there than half-done here.

---

Merge-ready from my side. It is 2 commits behind development (e95c11759, a313baf7f) — worth a rebase before merging, and #4267 landing underneath it means the two gcp modules are finally converging rather than diverging.

[Umang01-hash]

Reviewed deeply and verified everything at head c122ee2 — built, vet, gofmt, -race, repo golangci-lint, 97.5% coverage, and ran the example E2E (boots + degrades cleanly with no creds; warnings, redacted endpoint log, correlation IDs, and export auth errors all surface correctly). Confirmed against Google's docs that telemetry.googleapis.com keyless OTLP ingest + roles/telemetry.tracesWriter are accurate.

Solid, self-contained, additive — plugs into the existing exporter registry (mirrors the metrics/gcp sibling), no core changes needed, no breaking changes. LGTM. Two non-blocking nits below.

[aryanmehrotra]

Re-reviewed at 960e3c74a, which is now even with development (609b381b9).

Both blockers from my last round are closed, and I confirmed each one the same way I raised it — by running the example, not by reading the diff.

Blocker 1 — TRACER_URL padding: closed, live

Same four rows as last time, same built binary, same fake ADC:

TRACER_URL	before (9c796a54b)	now (960e3c74a)
127.0.0.1:34317	INFO exporting …	INFO exporting … at 127.0.0.1:34317
https://127.0.0.1:34318	ERROR … tracing is disabled	ERROR … tracing is disabled
␣https://127.0.0.1:34319	INFO exporting …, 0 dials	ERROR … must be a schemeless host:port … tracing is disabled
127.0.0.1:34320␣	INFO exporting …, 0 dials	INFO exporting … at 127.0.0.1:34320 (trimmed)

Rows 3 and 4 are the two silent-drop cases, and both now behave. Reverting the strings.TrimSpace takes 6 subtests across 2 tests with it, including the socket-level Test_buildExporter_dialsTheResolvedEndpoint/padded — so this is guarded, not merely fixed.

Blocker 2 — the startup hang: closed, and the improvement is measured

Example binary, a listener that accepts and never answers as GCE_METADATA_HOST, timed from exec to the first 200 on /hello:

scenario	time to first 200
metadata absent (connection refused)	45 ms
wedged metadata server — deadline removed (control)	64,689 ms
wedged metadata server — at this head	5,030 / 5,031 ms
wedged, tracing off (control)	24 / 46 ms

5.03s is exactly one metadataTimeout: with GOOGLE_APPLICATION_CREDENTIALS set, FindDefaultCredentials resolves from the file and never reaches metadata, so only the detector's bound is spent. The 2*metadataTimeout worst case in the comment is the right ceiling for the case where it does.

Your citations check out at the pinned versions, which matters because the design rests on them:

claim	verified
detector discards its context	contrib/detectors/gcp@v1.46.0 detector.go:35 — func (d *detector) Detect(context.Context), unnamed parameter, calls the context-free metadata.OnGCE()
5s is the metadata client's own budget	compute/metadata@v0.9.0 metadata.go:83 c.Timeout = 5 * time.Second over :74 Dialer{Timeout: 2s}, and :66 builds the default client with enableTimeouts=true — so it is the real default, not a branch nothing takes
oauth2 keeps the context for refresh	oauth2@v0.37.0 google/google.go:171 and :223 both return cfg.TokenSource(ctx) — the context is retained by the returned source

---

Blocking — one of the two deadline tests does not guard its deadline

I mutated both bounds. One mutant dies; the other walks through CI.

mutant	full suite	in isolation
drop awaitWithin around the platform detector	FAIL (suite hangs, panic: test timed out after 2m0s)	FAIL
drop awaitWithin around the ADC lookup	ok … 4.774s ← survives	FAIL — adc.get blocked for 31.05s

Run under -v, Test_adc_get_isBoundedByTheStartupDeadline reports --- PASS … (0.00s) in the full suite. A test whose whole subject is a 5-second bound cannot be exercising it in zero milliseconds: FindDefaultCredentials is returning before it ever contacts the wedged listener, and the surviving err != nil assertion is satisfied by "no credentials found" rather than by the timeout.

The cause is Test_cachingDetector_Detect_addsProjectID (gcp_test.go:508). It is the one test that builds a cachingDetector with no platform, so platformDetector() hands back the real gcpdetect.NewDetector(), whose Detect calls metadata.OnGCE() — and that answer is memoized for the life of the process (compute/metadata@v0.9.0 metadata.go:118 onGCEOnce sync.Once, :134; the doc comment says "memoized for better performance"). It runs with no metadata host set, caches false, and every later FindDefaultCredentials in that process short-circuits before the metadata step.

I isolated it rather than guessed — every other test calls writeADC(t) and resolves from the file, so none of them reach OnGCE:

go test -run 'Test_buildExporter_endpoint|Test_adc_…'                  -> FAIL, 31.05s
go test -run 'Test_buildExporter_redactsEndpointInLog|Test_adc_…'      -> FAIL, 31.79s
go test -run 'Test_buildExporter_warnsOnIgnoreListed…|Test_adc_…'      -> FAIL, 31.77s
go test -run 'Test_warnMissingProject|Test_adc_…'                      -> FAIL, 30.89s
go test -run 'Test_cachingDetector_Detect_addsProjectID|Test_adc_…'    -> PASS, 0.00s   <-- this one

The fix is one line, and I verified it rather than proposing it. Give that test the same injected seam the rest of the file uses:

d := &cachingDetector{adc: resolvedADC(""), platform: offGCEDetector{}}

// offGCEDetector reproduces what contrib/detectors/gcp does off Google Cloud at
// the pinned version: metadata.OnGCE() is false, so Detect returns (nil, nil)
// without touching the network (detector.go:36-37).
type offGCEDetector struct{}

func (offGCEDetector) Detect(context.Context) (*resource.Resource, error) { return nil, nil }

With that applied:

unmutated, full suite          -> ok    8.939s
ADC mutant, FULL SUITE         -> FAIL  adc.get blocked for 31.53s   <-- the kill is restored

Why I am blocking on a test. The production code is correct — I measured it at 5.03s against a 64.7s control. But these two tests exist so that the bound cannot be removed later without CI noticing, and today one of them would not notice. That is the same reason you rewrote Test_buildExporter_dialsTheResolvedEndpoint last round rather than asserting on the log line, and the reasoning you gave for it applies here unchanged.

It also removes a real network probe from a unit test as a side effect: on a runner with no metadata server, addsProjectID currently does a live OnGCE() DNS-and-HTTP attempt.

---

Notes, not blocking

The "detector fails off Google Cloud" comment is wrong at the pin. gcp.go:196-199 and the comment at gcp_test.go:510 both say the platform detector fails off Google Cloud and that the error is returned so the SDK can report a partial resource. At contrib/detectors/gcp@v1.46.0 detector.go:36-37 it returns (nil, nil) — no error at all. The ErrPartialResource path at :152-153 is the on-GCE partial case. The code does the right thing either way (withProjectID already tolerates a nil resource, and addsProjectID discards the error with _), so this is a comment correction, not a behaviour one.

Test_buildExporter_warnsOnIgnoreListedQuotaHeader kept the old name. The message it asserts now says the header is forwarded and is not honored, which is the wording Umang's thread settled on. The test name still says "IgnoreListed".

The quota-header warning is still case-sensitive — cfg.Headers[quotaProjectHeader] against a map parseHeaders fills verbatim (pkg/gofr/otel.go:141-169, no lowercasing), so TRACER_HEADERS=X-Goog-User-Project=p warns nothing while gRPC still forwards it. Unchanged from my last review and still inherited — metrics/exporters/gcp/gcp.go:169 is identical — so it belongs with the cross-module follow-ups.

errSchemeInEndpoint is still unexported, so an external caller cannot errors.Is it. Low impact; the registry degrades to tracing is disabled regardless.

The production-tracing row I flagged is fixed — page.md:43 now reads "except for gcp, which rejects a scheme at startup rather than ignoring it". Good.

---

Everything else I ran, all clean

gate	result
go build / go vet	clean
go test -count=1	pass, 3/3 repeats
go test -race	pass
coverage	97.5%
golangci-lint (repo config, GOWORK=off)	0 issues
gofmt -l over the module + example	empty
dependabot module count	git ls-files '*go.mod' | wc -l = 34, matches the comment
go.work	new module present at line 37
CI at 960e3c74a	15 success, 1 skipped, 0 failures

Evidence: example built as a binary and run in 8 scenarios · boot timed against absent / wedged / wedged-with-tracing-off metadata servers, 2 runs each, plus a deadline-removed control · 4 TRACER_URL cases re-run live · 3 mutants, 2 kills, 1 survivor isolated to its cause across 5 ordering probes · proposed fix applied and the survivor re-killed in the full suite · three dependency claims read at the pinned versions in GOMODCACHE · worktree restored, git status clean.

One line and a six-line helper. Everything above it is closed.