tanmayrauth opened a new issue, #1236:
URL: https://github.com/apache/iceberg-go/issues/1236
### Feature Request / Improvement
## Summary
Today Java ships Iceberg's **Metrics Reporting API**, PyIceberg has none of
it, and neither does Go. This proposal builds the **full framework** in
iceberg-go, modeled on Java's long-standing implementation: a pluggable
`Reporter` that receives a `ScanReport` after scan planning and a
`CommitReport` after a commit, the `Counter`/`Timer`/`Context` primitives that
feed them, the scan- and commit-path instrumentation that populates them, and
the built-in `Logging`, `InMemory`, and `REST` reporters.
**OpenTelemetry is the final, optional piece** — one opt-in adapter layered
on top, not the core abstraction. The framework is the spec-compliant contract;
any backend (including OTel) plugs in behind it.
## Upstream alignment
**Framework (PRs 1–6) — modeled on Java's existing implementation.** Java
has shipped the full Metrics Reporting framework for years: `MetricsReporter`,
`ScanReport`/`CommitReport`, `ScanMetrics`/`CommitMetrics`,
`Counter`/`Timer`/`MetricsContext`, the `Logging`/`InMemory`/`REST` reporters,
and the scan- and commit-path instrumentation that drives them. The bulk of
this proposal ports that proven design to Go, adapting mechanics to Go idioms
(e.g. a registry instead of reflection for reporter selection). There is no
recent "discussion" to cite for this part — it predates the current dev
process, so the reference is the Java codebase itself.
**OTel adapter (PR 7) — informed by the recent Java dev-list thread.** The
one piece Java is adding *now* is an OpenTelemetry reporter ([issue
#16169](https://github.com/apache/iceberg/issues/16169), [PR
#16250](https://github.com/apache/iceberg/pull/16250), [dev-list
thread](https://lists.apache.org/thread/vn4gglocg2g40p69mfrrh86qzkn1rr4b)).
Since OTel is likewise the last/optional piece for us, we mirror the
conclusions that thread settled *for the OTel layer specifically*: the host
owns the SDK (the reporter just looks up a meter), cardinality controls (no
snapshot ID as a metric attribute; configurable attribute allowlist), the
curated OTel metric-name subset, and deferring span-based reporting. The one
mechanism we *don't* copy is Java's `compileOnly` dependency: iceberg-go
**already** pulls `go.opentelemetry.io` in transitively (via its GCP
integrations), so the OTel reporter is a plain in-module `metrics/otel` package
rather than a separate module — non-users pay no run
time cost because Go compiles only the packages a build actually imports.
## Motivation
Scan- and commit-planning metrics (files/manifests considered, scanned,
skipped; bytes read; commit attempts and durations) are otherwise invisible
from outside the client. A standard reporting interface lets operators
aggregate these across many clients, and a standard *naming* scheme lets
dashboards line up across Iceberg implementations. Today only Java ships this;
Go has nothing.
## Design decisions
1. **Spec-compliant interface as the core abstraction.** The catalog spec
defines a metrics endpoint that bundles metrics into a
`ScanReport`/`CommitReport`. We implement that contract so REST catalogs
interoperate, rather than emitting ad-hoc metrics straight from the library.
2. **OpenTelemetry is an opt-in adapter, not the core.** The OTel reporter
is an isolated `metrics/otel` package that the host wires its own SDK into (the
reporter only looks up a meter). It is opt-in: a build that never imports it
compiles none of its code. Unlike Java — which used a `compileOnly` dependency
to keep OTel off the runtime classpath — iceberg-go **already** depends on
`go.opentelemetry.io` transitively (via its GCP integrations), so a separate
module buys nothing and would force the repo to become multi-module. A plain
in-module package is simpler and achieves the same opt-in goal.
3. **Two distinct layers for metric names.** The **report / REST wire**
carries the *full* set of metrics (kebab-case, from
`ScanMetricsResult`/`CommitMetricsResult`) for spec interop. The **OTel
surface** exposes a deliberately *curated subset* under `iceberg.scan.*` /
`iceberg.commit.*` names, matching Java's `OtelMetricsReporter` (PR #16250)
verbatim so dashboards line up across implementations. One source of truth (the
reports), two presentations.
4. **Cardinality controls from day one.** Never attach snapshot ID as a
metric attribute (it is unbounded). The OTel adapter takes a configurable
attribute allowlist (`iceberg.otel.metrics.attributes`); the default bounded
set is **scan = `table-name` + `schema-id`, commit = `table-name` +
`operation`** — matching the Java PR after its mid-review cardinality fix.
5. **Zero overhead when unconfigured.** With no reporter set, the metrics
context hands out no-op counters/timers so instrumented code paths cost nothing.
6. **Reporting must never block scans or commits.** `Report()` is invoked
inline at the scan/commit completion point, so reporters must return promptly.
Network-backed reporters (the REST reporter, and OTel push exporters) dispatch
the actual send on a background worker; a slow or failing sink must never stall
or fail a scan/commit. Reporter errors are logged and swallowed, never
propagated to the operation.
7. **`filter` serialization is deferred for v1.** The REST spec types
`ScanReport.filter` as a structured `Expression` object (Java uses
`ExpressionParser.toJson`). iceberg-go has **no** expression-JSON serializer
today — `BooleanExpression` (`exprs.go`) implements only `fmt.Stringer`.
Building structured Expression (de)serialization is a separate, non-trivial
effort (also needed for REST predicate pushdown in general), so **v1 emits
`AlwaysTrue` (`{"type":"true"}`) for `filter`** and leaves full serialization
to a follow-up; every other report field is fully populated. (PyIceberg has no
expression-JSON serializer either, so this is not a Go-specific gap.)
8. **Opt-in by default; no behavior change for existing users.** Unlike
engine-embedded Java (which defaults to `LoggingMetricsReporter`), iceberg-go
is a library. The default reporter is **no-op** — no logging, no network
traffic — until a user explicitly configures one via `metrics-reporter-impl` or
`WithReporter`. The REST reporter is likewise opt-in, never auto-enabled, so
existing REST-catalog users see no new `/metrics` POSTs unless they turn it on.
9. **Scan counters are concurrency-safe.** Scan planning reads manifests
concurrently — `collectManifestEntries` uses `errgroup` with a bounded worker
pool (`g.SetLimit`) and mutex-guarded shared state today. Scan metric counters
are therefore incremented from multiple goroutines and must be safe under
`-race`: implemented with `atomic.Int64`, or via per-worker accumulation merged
at the `PlanFiles` completion point (matching the scanner's existing
`manifestEntries` pattern). The `Counter` interface documents this contract.
## Scope
**In scope**
- `metrics/` package: `Reporter`, `Report` marker,
`Counter`/`Timer`/`Context` primitives, report + result types, JSON wire format.
- Default reporters: `LoggingReporter`, `InMemoryReporter`, `Combine`
(composite).
- Reporter configuration (`metrics-reporter-impl` property + registry, plus
programmatic option) and threading through catalog → table → scan.
- Scan-path and commit-path instrumentation.
- `RESTMetricsReporter` implementing the catalog metrics endpoint.
- OpenTelemetry adapter as an isolated in-module `metrics/otel` package.
**Out of scope (deferred)**
- Span/trace-based reporting — belongs in a dedicated scan/commit
instrumentation hook; Java deferred it too.
- Histograms (unused by scan/commit reports).
- Server-side metrics aggregation — the catalog is not a metrics platform.
## Current state of iceberg-go
- **No metrics framework exists** — no `Reporter`, `ScanReport`,
`CommitReport`, `Context`, `Counter`, or `Timer`.
- **Commit data is already half-computed**: `updateMetrics`
(`table/snapshots.go`) tracks 16 `int64` counters (added/removed data files,
delete files, records, sizes, positional/equality deletes) for the snapshot
summary.
- **The REST metrics endpoint is a stub**: `endpointReportMetrics` (`POST
.../tables/{table}/metrics`, `catalog/rest/endpoints.go`) is declared but has
no handler or client call.
- **No direct OpenTelemetry usage** in source.
So commit metrics are mostly a matter of *surfacing* existing data; scan
metrics and the framework are net-new.
## Proposed architecture
```
metrics/ // core package (no new deps)
reporter.go // Reporter, Report marker, Combine, Logging,
InMemory
context.go // Context, Counter, Timer (+ no-op variants)
scan_report.go // ScanReport, ScanMetrics (live), ScanMetricsResult
commit_report.go // CommitReport, CommitMetrics (live),
CommitMetricsResult
result.go // CounterResult, TimerResult
request.go // ReportMetricsRequest envelope (REST)
metrics/otel/ // in-module package, opt-in (OTel already a
transitive dep)
reporter.go // OtelReporter, attribute allowlist
```
### Core interfaces
```go
package metrics
// Reporter is a pluggable sink for metrics reports.
type Reporter interface {
Report(ctx context.Context, report Report)
}
// Report is implemented by ScanReport and CommitReport.
type Report interface{ isMetricsReport() }
type Unit string
const (
UnitCount Unit = "count"
UnitBytes Unit = "bytes"
)
// Counter and Timer are the live metric primitives. The no-op
// implementations returned by a nil/absent reporter make instrumentation
free.
type Counter interface {
Incr()
Add(n int64)
Value() int64
Unit() Unit
}
type Timer interface{ Start() (stop func()) }
// Context produces named counters/timers for one scan or commit.
type Context interface {
Counter(name string, unit Unit) Counter
Timer(name string) Timer
}
```
### Report types (JSON = spec wire format)
Verified against `apache/iceberg`'s `open-api/rest-catalog-open-api.yaml`:
`ReportMetricsRequest` is `anyOf[ScanReport, CommitReport]` plus a required
`report-type` discriminator; `metrics` is an open map of `string →
(CounterResult | TimerResult)` (so readers must tolerate unknown metric keys).
```go
type ScanReport struct {
TableName string `json:"table-name"`
SnapshotID int64 `json:"snapshot-id"`
SchemaID int `json:"schema-id"`
ProjectedFieldIDs []int `json:"projected-field-ids"`
ProjectedFieldNames []string `json:"projected-field-names"`
// Spec type is a structured Expression object, not a string. iceberg-go
has no
// Expression JSON serializer yet (BooleanExpression only implements
Stringer),
// so v1 emits AlwaysTrue ({"type":"true"}). See design decision #7.
Filter json.RawMessage `json:"filter"`
Metrics ScanMetricsResult `json:"metrics"`
Metadata map[string]string `json:"metadata,omitempty"`
}
func (ScanReport) isMetricsReport() {}
type CommitReport struct {
TableName string `json:"table-name"`
SnapshotID int64 `json:"snapshot-id"`
SequenceNumber int64 `json:"sequence-number"`
Operation string `json:"operation"`
Metrics CommitMetricsResult `json:"metrics"`
Metadata map[string]string `json:"metadata,omitempty"`
}
func (CommitReport) isMetricsReport() {}
type CounterResult struct {
Unit Unit `json:"unit"`
Value int64 `json:"value"`
}
type TimerResult struct {
Count int64 `json:"count"`
TimeUnit string `json:"time-unit"`
TotalDuration int64 `json:"total-duration"`
}
type ScanMetricsResult struct {
TotalPlanningDuration *TimerResult
`json:"total-planning-duration,omitempty"`
ResultDataFiles *CounterResult `json:"result-data-files,omitempty"`
ResultDeleteFiles *CounterResult
`json:"result-delete-files,omitempty"`
// ... remaining scan metrics (table below)
}
```
### Configuration
```go
// String-configured (works for REST catalog config too):
cat, _ := catalog.Load(ctx, "prod", iceberg.Properties{
"metrics-reporter-impl": "logging", // or a registered name
})
// Programmatic (the idiomatic Go path):
cat, _ := rest.NewCatalog(ctx, "prod", uri,
rest.WithMetricsReporter(myReporter))
// Per-scan override:
scan := tbl.Scan(table.WithReporter(inmem))
```
Java selects a reporter by reflecting on a fully-qualified class name
(`metrics-reporter-impl=org.apache...`). Go has no equivalent runtime class
loading, so we use a small registry that maps a short name (`"logging"`,
`"rest"`) to a constructor, plus the programmatic `WithReporter` option as the
primary idiom.
## Metric names
There are two layers, and they intentionally differ:
1. **Report / REST wire** — the *full* set of metrics, kebab-case, taken
verbatim from Java's `ScanMetricsResult` / `CommitMetricsResult`. These are the
cross-implementation contract for the spec-compliant reports.
2. **OTel surface** — a deliberately *curated subset* (~12 series), named to
match Java's `OtelMetricsReporter` (PR #16250). The OTel reporter does **not**
export every report field; it maps the high-value ones so backends like
Prometheus/CloudWatch stay manageable.
### Report fields — scan (17, kebab-case wire names)
`total-planning-duration` (timer), `result-data-files`,
`result-delete-files`, `total-data-manifests`, `total-delete-manifests`,
`scanned-data-manifests`, `scanned-delete-manifests`, `skipped-data-manifests`,
`skipped-delete-manifests`, `skipped-data-files`, `skipped-delete-files`,
`total-file-size-in-bytes`, `total-delete-file-size-in-bytes`,
`indexed-delete-files`, `equality-delete-files`, `positional-delete-files`,
`dvs`.
### Report fields — commit (26, kebab-case wire names)
`total-duration` (timer), `attempts`, plus the summary-derived counters:
`added/removed/total-data-files`, `added/removed/total-delete-files`,
`added/removed/total-records`, `added/removed/total-files-size-bytes`,
`added/removed/total-positional-deletes`,
`added/removed/total-equality-deletes`, equality/positional delete-file
variants, `added/removed-dvs`, `manifests-created/replaced/kept`,
`manifest-entries-processed`.
### OTel surface (curated subset, matching Java PR #16250)
These are the names Java's `OtelMetricsReporter` actually emits — we mirror
them exactly:
| OTel metric | Kind / unit | Source report field |
|---|---|---|
| `iceberg.scan.planning.duration` | histogram, ms |
`total-planning-duration` |
| `iceberg.scan.result.data_files` | sum | `result-data-files` |
| `iceberg.scan.result.delete_files` | sum | `result-delete-files` |
| `iceberg.scan.data_manifests.scanned` | sum | `scanned-data-manifests` |
| `iceberg.scan.data_manifests.skipped` | sum | `skipped-data-manifests` |
| `iceberg.scan.file_size.bytes` | sum, By | `total-file-size-in-bytes` |
| `iceberg.commit.duration` | histogram, ms | `total-duration` |
| `iceberg.commit.attempts` | sum | `attempts` |
| `iceberg.commit.records.added` | sum | `added-records` |
| `iceberg.commit.data_files.added` | sum | `added-data-files` |
| `iceberg.commit.data_files.removed` | sum | `removed-data-files` |
| `iceberg.commit.file_size.added_bytes` | sum, By |
`added-files-size-bytes` |
**OTel attributes:** configurable via the `iceberg.otel.metrics.attributes`
allowlist. Default bounded set — scan: `iceberg.table.name` +
`iceberg.schema.id`; commit: `iceberg.table.name` + `iceberg.operation`.
`iceberg.snapshot.id` is deliberately **never** a metric attribute (unbounded
cardinality); per-snapshot detail stays available through the full reports and
snapshot history.
> **Naming reconciliation note:** iceberg-go's existing *snapshot-summary*
keys differ slightly from Java's *commit-report* names (e.g. `added-files-size`
vs `added-files-size-bytes`, `added-position-deletes` vs
`added-positional-deletes`, `deleted-data-files` vs `removed-data-files`). The
summary keys are spec-defined and stay as-is; the **report** uses Java's metric
names. The commit instrumentation computes report values from the existing
`updateMetrics` counters and emits them under the Java names.
>
> Feature-gated metrics (deletion vectors, equality/positional deletes) are
populated only where iceberg-go implements the underlying feature; otherwise
they are omitted rather than fabricated.
## Implementation plan (incremental, independently reviewable PRs)
| # | PR | Scope |
|---|---|---|
| 1 | **Metrics package foundation** | `Unit`, `Counter`, `Timer`, `Context`
(+ no-ops), `Reporter`, `Report` marker, `LoggingReporter`, `InMemoryReporter`,
`Combine`. New `metrics/` only. |
| 2 | **Report + wire types** | `ScanReport`/`CommitReport`,
`*MetricsResult`, `CounterResult`/`TimerResult`, `ReportMetricsRequest`,
kebab-case JSON + interop tests against Java-produced JSON. |
| 3 | **Plumbing** | `metrics-reporter-impl` + registry + programmatic
option; thread reporter through `Table` and the scan builder; default =
**no-op** (metrics are strictly opt-in — a library must not emit logs or
network calls unless asked). |
| 4 | **Scan instrumentation** | populate scan counters/timer in scan
planning, emit `ScanReport`. |
| 5 | **Commit instrumentation** | `total-duration` timer + `attempts`;
snapshot `updateMetrics` into a `CommitReport`; emit it. |
| 6 | **REST reporter** | implement the `endpointReportMetrics` stub:
**async** (non-blocking) POST of the spec-verified `ReportMetricsRequest` +
combine into the REST catalog. |
| 7 | **OpenTelemetry adapter + docs** | isolated in-module `metrics/otel`
package, meter mapping, configurable attribute allowlist, cardinality docs,
metrics-reporting page. |
**Critical path:** 1 → 2 → 3 → 4. PRs 5, 6, 7 fan out from PR 3 and can be
built in parallel.
PRs 1–6 are the framework — a Go port of machinery Java has had for years
(no live upstream PR to track). PR 7 is the only part with a current Java
analog (PR #16250), and it's deliberately the smallest and most isolated: an
opt-in `metrics/otel` package.
## Open questions
## Open questions
1. **Reporter selection** — registry + `metrics-reporter-impl` (for
spec/REST parity) *plus* a programmatic option? (proposed: both)
2. **OTel placement** — in-module `metrics/otel` package (proposed:
`go.opentelemetry.io` is already a transitive dependency via the GCP
integrations, so isolating it in a separate module buys nothing and would make
iceberg-go multi-module) vs. a separate Go module. Java kept its reporter in
`iceberg-core` via `compileOnly`; Go has no compile-only mechanism but doesn't
need one here because the dependency is already present.
3. **OTel metric naming** — Java's `iceberg.scan.*` / `iceberg.commit.*`
names (PR #16250) are the only concrete precedent, but Java itself flagged them
as **provisional** (issue #16169: no OTel semantic convention exists yet, would
realign if one emerges) and that PR is still unmerged. Mirror Java's current
names verbatim for parity (proposed), adopt the dotted
`iceberg.<operation>.<resource>.<count-type>` scheme, or wait for an OTel
semantic convention? And should the Go subset match Java's ~12 series exactly?
4. **Metric accumulation shape** — the proposal currently sketches Java's
exported `Counter`/`Timer`/`MetricsContext` primitives. The alternative is to
keep accumulation as a plain internal `atomic.Int64` struct — mirroring the
existing `table.updateMetrics` — and export only the `Reporter` + report/result
types, which is smaller and arguably more idiomatic Go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
