Skip to content

feat(metrics): Instrument commits and emit CommitReport ( #1236) - #1596

Open
tanmayrauth wants to merge 2 commits into
apache:mainfrom
tanmayrauth:metrics-reporting-1236-pr5-commit-instrumentation
Open

feat(metrics): Instrument commits and emit CommitReport ( #1236)#1596
tanmayrauth wants to merge 2 commits into
apache:mainfrom
tanmayrauth:metrics-reporting-1236-pr5-commit-instrumentation

Conversation

@tanmayrauth

Copy link
Copy Markdown
Contributor

What

Emits a spec-compliant CommitReport from the commit path on successful completion:

  • doCommit's retry loop now records total-duration and attempts.
  • The committed snapshot's summary is mapped into a CommitReport under Java's commit-report metric names. iceberg-go's summary keys differ from those names, so the mapping translates them (added-files-size →
    added-files-size-bytes, deleted-data-files → removed-data-files, added-position-deletes → added-positional-deletes, …) so the wire format matches Java and dashboards line up across implementations.
  • Metrics with no iceberg-go summary key (DVs, manifest counts) are omitted rather than reported as zero, mirroring Java's counterFrom.

Behavior notes

  • No behavior change for existing users: emission is guarded by metrics.IsNop, so under the default no-op reporter it's a pure no-op (no report assembled). Report failures can never fail a commit.
  • Metadata-only commits emit nothing: a property/schema-only change produces no snapshot, so reporting the unchanged branch head would misattribute a prior snapshot's metrics. Skipped via
    commitAddedSnapshot.
  • Non-default branches resolve the just-committed head via SnapshotByName(branch) rather than the table's current snapshot.

Deferred

CommitReport.Metadata (Java's engine/commit-context map) is left unset with a TODO — iceberg-go doesn't thread that context into the commit path yet; a follow-up will populate it.

Testing

  • Unit tests for buildCommitReport: name-translated mappings, nil snapshot, absent/unparseable summary values omitted.
  • Integration tests: an append emits a CommitReport with correct operation/attempts/records; a metadata-only commit emits none.
  • go build, go vet, and full table/metrics suites pass.

Realted to #1236

Fifth PR of the metrics reporting framework. Emits a CommitReport from
the commit path on successful completion:

- the retry loop in doCommit now records the total duration and the
  number of attempts;
- the committed snapshot's summary is mapped into a CommitReport, under
  Java's commit-report metric names. iceberg-go's summary keys differ
  from those names (e.g. added-files-size -> added-files-size-bytes,
  deleted-data-files -> removed-data-files, added-position-deletes ->
  added-positional-deletes); the mapping translates them so the wire
  format matches Java. Metrics with no iceberg-go summary key (DVs,
  manifest counts) are omitted rather than reported as zero.

The committed table now also preserves its reporter. With the default
no-op reporter this is a pure no-op for existing users.
Java's CommitReport carries a metadata map (engine/commit context)
that iceberg-go does not thread into the commit path yet. Mark it as
a TODO in buildCommitReport rather than emitting an empty map.
@tanmayrauth
tanmayrauth requested a review from zeroshade as a code owner July 29, 2026 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant