feat(rest): add scan-planning retries, telemetry, and documentation

[Revanth14]

Summary

Retry transient REST scan-planning POST failures up to three times with jittered backoff and Retry-After handling. Each logical call reuses its UUIDv7 idempotency key; cancellation, terminal HTTP errors, transport failures, and malformed successful responses stop retries.

Add OpenTelemetry spans and metrics for planning requests, duration, retries, plan expiration, and automatic local fallback. Expiration metrics are recorded before their operation span ends, and attributes contain bounded operation, outcome, and fallback reason values.

Document scan-planning modes, capabilities, retries, credential lifetime, telemetry, and usage. Automatic remote planning requires submission, polling, and task retrieval; cancellation is best-effort. Document that the server's scan-planning-mode is not enforced and that deployments requiring server planning must explicitly choose remote mode.

Add coverage for malformed successful responses, expiration span lifetime and exemplars, and preservation of already-partial remote tasks. Link the parity fixture's snapshots and enforce serial telemetry tests with deterministic sampler configuration. A regression test also confirms that external reference wrappers are rejected during transform binding, before serialization.

The Phase 6 commit has been removed by rebasing onto main after #2019 merged. The changes include Phase 7 implementation and review follow-ups.

Validation

• GOTOOLCHAIN=go1.25.9 go test ./...
• GOTOOLCHAIN=go1.25.9 go test -race . ./catalog/rest ./internal/scanmetrics ./table (run in two invocations)
• GOTOOLCHAIN=go1.25.9 golangci-lint run --timeout=10m (v2.12.2)

Part of #1178 (Phase 7).

[laskoviymishka]

The parity test is the strongest part of this. Comparing local vs remote by binding residuals semantically instead of round-tripping the expected value through the codec under test, and using independent JSON maps on both ends, is exactly how I'd want a parity harness built. The retry wrapper is careful too: never replaying an ambiguous transport failure or a malformed 200 is the right call.

I'd hold this before merging though. One process note first: I reviewed this as Phase 7 (retries, telemetry, docs) and treated the Phase 6 files carried in the diff as out of scope, since they'll disappear once #2019 lands and you rebase, so nothing below re-litigates that work.

The thing I'd most want resolved is the scan-planning-mode interop gap. A REST server can send scan-planning-mode: server to force clients through the planning endpoint (Polaris and Nessie lean on this to scope storage credentials), and today a Go client silently ignores it and plans however the scan option says. That can send us down a local-read path the server never intended, potentially against the wrong identity. The README already notes the key isn't resolved yet, which is honest, but I'd spell out the hazard rather than leaving it as a neutral "not yet resolved", or fail fast when a server requires server mode and we can't honor it.

A few things I'd want settled before merge:

• Rebase off #2019 so the diff is only Phase 7 and can be re-reviewed cleanly.
• Fix the README claim that SupportsFullRemoteScanPlanning checks cancel and that auto needs all four endpoints; it checks three.
• Spell out the scan-planning-mode interop hazard, or decide to fail fast on server mode.
• Record the expiration metric before finish ends the span, and drop the two dead sentinel branches.

Once those are addressed, happy to take another pass and approve.

[laskoviymishka]

This says SupportsFullRemoteScanPlanning checks the cancel endpoint, but it only checks plan, fetch-result, and fetch-tasks. Cancel is deliberately best-effort and not required for auto mode, which the code comment gets right.

The auto-mode row just above has the same slip ("all four endpoints are advertised"). I'd reword both to "submission, polling, and task retrieval" so an operator who withholds cancel doesn't think it drops them out of auto mode.

[laskoviymishka]

Two things stacked in this defer. finish(err) ends the span before scanmetrics.Expired runs, so the expiration counter records after the span stops recording and its data point loses the trace exemplar (the requests counter inside finish still gets one). I'd record Expired first and let finish run last.

While reordering: this branch can only ever see ErrPlanExpired (fetchPlanningResultErrorTypes maps nothing to ErrNoSuchPlanTask), so that half is dead. FetchScanTasks at line 736 is the mirror, dead on ErrPlanExpired instead. I'd narrow each to its own sentinel:

defer func() {
    if errors.Is(err, ErrPlanExpired) {
        scanmetrics.Expired(ctx, "fetch-result")
    }
    finish(err)
}()

[laskoviymishka]

The second snapshot (id 20) is added without a ParentSnapshotID, so the fixture's snapshot chain is technically invalid per spec. Planning doesn't walk the chain so the test still passes, but a parity fixture is exactly the kind of thing people copy from. I'd set ParentSnapshotID to 10 on the i==1 iteration. wdyt?

[laskoviymishka]

The name says terminal HTTP responses, but the 200 case returns invalid JSON and bails on the parse error, which is the non-HTTP-error short-circuit, not the terminal-status path. So the 200 row is really exercising a different branch than 400/401/403/404/501.

I'd pull 200 into its own test (something like DoesNotRetryNonHTTPErrors) and keep this one to the 4xx/5xx codes, so each test pins one reason. Minor, but it makes a regression easier to localize.

[laskoviymishka]

This test swaps the global OTel providers, so the "keep it serial" comment is load-bearing, but it's only a comment. If someone adds t.Parallel() here later it'll race the global swap against every other test in the package, and the failure will be baffling.

I'd make the constraint machine-checkable, either a //nolint:paralleltest on the function or a package-level mutex around the provider swap. wdyt?

[laskoviymishka]

This pulls the qualified name through an anonymous assertion on the unexported referenceName(), so only the in-package *boundRef satisfies it. Any other BoundReference silently falls back to the leaf Field().Name and drops the path prefix, which is the exact nested-field bug this PR fixes, reintroduced for external implementations.

Since the leaf-only path is the bug we're closing, I'd make it enforceable: either add ReferenceName() string to BoundReference (defaulting to Field().Name) or a boundReferenceName(BoundReference) helper. wdyt?

[laskoviymishka]

Small one, and fine to leave: the meter and instruments are looked up on every finish and every count() rather than once. The SDK dedups by name, but each lookup takes a registry lock. It's nothing at one-per-query, but if this ever gets reused for per-task or frequent-replan instrumentation it'd start to matter, and it reads as slightly non-idiomatic.

If you want to preempt it, a sync.Once-guarded package init (deferring otel.Meter to first use so the app's provider is set) caches them. Genuinely optional.

[laskoviymishka]

The parity server forces a large target size so tasks always arrive whole, which means the pass-through path here (and the guard in splitParquetScanTask that keeps already-partial server tasks from being re-split) never gets exercised. That guard is load-bearing for any planner that returns pre-ranged tasks.

I'd add a small unit test that hands splitRemoteScanTasks a couple of already-partial tasks and asserts they come back unchanged. wdyt?