metrics: add backup_exported_diff_size_bytes by skoya76 · Pull Request #301 · cybozu-go/mantle

skoya76 · 2026-06-26T08:42:06Z

This PR adds a metric named mantle_backup_exported_diff_size_bytes. It is a
Gauge that represents, for a given MantleBackup, the total size in bytes of the
diff parts uploaded to object storage so far. It is exported as soon as the diff
size is known, without waiting for the backup to complete.

So that the value can still be tracked until the backup completes even if the
controller restarts, it is restored from the part sizes in object storage on the
reconcile after a restart. We use a Gauge rather than a Counter because this
restoration can be done with a single Set(). The metric is deleted when the
corresponding MantleBackup is deleted.

The commits are split for ease of review; they will be squashed into one when merged. See the individual commit messages for details.

Copilot

Pull request overview

Adds a new Prometheus gauge to report the total size (bytes) of uploaded diff parts for a MantleBackup, and wires it into the controller lifecycle so it is set during upload progress and removed on finalization.

Changes:

Introduces mantle_backup_exported_diff_size_bytes (BackupExportedDiffSizeBytes) and registers it with the controller-runtime metrics registry.
Computes and sets the gauge by summing object sizes of uploaded parts via a new objectstorage.Bucket.GetSize() method (S3 HeadObject), and deletes the labeled series on backup finalization.
Extends unit/e2e test coverage to assert the metric is exposed, set, and deleted as expected.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/e2e/multik8s/misc_test.go	Adds an e2e assertion that the new metric name is exposed on `/metrics`.
internal/controller/metrics/metrics.go	Defines and registers the new `backup_exported_diff_size_bytes` gauge vec.
internal/controller/mantlebackup_controller.go	Sets the gauge by querying object sizes; deletes the series on finalize; refactors completed-job listing helper.
internal/controller/mantlebackup_controller_test.go	Adds helpers/tests to verify the gauge value and that the series is deleted; stubs object storage `GetSize`.
internal/controller/internal/objectstorage/s3.go	Implements `GetSize` via `HeadObject` and returns `ContentLength`.
internal/controller/internal/objectstorage/objectstorage.go	Extends `Bucket` interface with `GetSize`.
internal/controller/internal/objectstorage/objectstorage_mock.go	Updates gomock to include the new `GetSize` method.

Files not reviewed (1)

internal/controller/internal/objectstorage/objectstorage_mock.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

satoru-takeuchi · 2026-07-02T09:10:45Z

+		prometheus.GaugeOpts{
+			Namespace: namespace,
+			Name:      "backup_exported_diff_size_bytes",
+			Help:      "Total bytes of the uploaded diff parts.",


Suggested change

Help: "Total bytes of the uploaded diff parts.",

Help: "The size of the uploaded backup diff data",

Please clarify this metrics is about one backup data. I consider total is ambiguous in tihs case.

satoru-takeuchi · 2026-07-02T09:22:52Z

+	}
+
+	var totalSize int64
+	for partNum := 0; partNum <= largestCompletedUploadPartNum; partNum++ {


I worry about the count of HEAD requests on big PVCs. For instance, let assume the PVC size is 10TiB and the part size is 128GiB. Then the number of part is 80. In this case, the total number of HEAD request will be 80*(1+80)/2. Can we cache the last value in memory? The new code would be like this:

if cache[uid] size = cache[uid] + r.objectStorageClient.GetSize(cxt, uid) else size = <get the total size of all diff parts>

skoya76 · 2026-07-03T04:09:12Z

Thanks for the review. I addressed both comments with fixup commits:

satoru-takeuchi

LGTM. Please resolve conflicts and squash commits.

Add a backup_exported_diff_size_bytes Gauge, labeled by persistentvolumeclaim, resource_namespace, and mantlebackup, so there is one series per MantleBackup. A Gauge is used rather than a Counter because it lets us restore the correct value with a single Set() after the controller restarts. Add GetSize to the Bucket interface (the S3 implementation returns the ContentLength of a HeadObject response) and use it in a new setExportedDiffSizeMetric, which sums the sizes of the diff parts uploaded so far and Set()s the gauge to that total. It is called from startUpload (the normal update path) and from the beginning of reconcileAsPrimary (to restore the value after a controller restart, since the replicate path may return early before reaching startUpload). Failures while collecting this metric are only logged, not returned, because it is only for observability: a transient object storage error must not block backup verification, replication, cleanup, or the creation of new upload jobs. The metric series is deleted when the MantleBackup is deleted (in finalizeStandalone), rather than right after the backup completes, since the latter risks the metric disappearing before Prometheus scrapes it. Summing from scratch on every reconcile means the number of HeadObject calls over a backup's lifetime grows quadratically with its number of parts. setExportedDiffSizeMetric now caches, per backup UID, the largest part number already summed and the resulting total, so each call only has to HeadObject the parts completed since the last call. The cache entry is dropped when the MantleBackup is deleted, alongside the metric. Also extract the shared "list the Jobs of a component, keep only the completed ones, and find the largest part number" logic (previously duplicated between getLargestCompletedUploadPartNum and handleCompletedJobsOfComponent) into listCompletedJobsOfComponent, and add an e2e check that backup_exported_diff_size_bytes is exposed. Signed-off-by: Kohya Shiozaki <kouyan120706@gmail.com>

skoya76 · 2026-07-03T06:42:22Z

Resolved the conflicts against main and squashed everything into a single commit. Here's the diff scoped to this PR from when you reviewed it to now. You can confirm from this link that there's no actual diff.

diff: https://github.com/cybozu-go/mantle/pull/301/files/427a6b613817903327b96f215455ee101239e8e4..865065bef4afe73d13055acdb2fbeef384caf853

Thanks for the review.

skoya76 force-pushed the add-backup-exported-diff-size-metric branch 9 times, most recently from fb5f189 to 684c85a Compare July 1, 2026 04:53

skoya76 requested a review from Copilot July 1, 2026 04:55

Copilot started reviewing on behalf of skoya76 July 1, 2026 04:56 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread internal/controller/mantlebackup_controller.go Outdated

Comment thread internal/controller/mantlebackup_controller.go

Comment thread internal/controller/mantlebackup_controller.go

skoya76 force-pushed the add-backup-exported-diff-size-metric branch 4 times, most recently from 82e2350 to c3c2f76 Compare July 2, 2026 00:38

skoya76 marked this pull request as ready for review July 2, 2026 04:11

skoya76 requested a review from a team as a code owner July 2, 2026 04:11

skoya76 requested review from satoru-takeuchi and removed request for a team July 2, 2026 04:11

satoru-takeuchi requested changes Jul 2, 2026

View reviewed changes

skoya76 force-pushed the add-backup-exported-diff-size-metric branch 2 times, most recently from f039f80 to 427a6b6 Compare July 3, 2026 03:03

skoya76 requested a review from satoru-takeuchi July 3, 2026 04:09

satoru-takeuchi previously approved these changes Jul 3, 2026

View reviewed changes

skoya76 dismissed satoru-takeuchi’s stale review via d9183b6 July 3, 2026 05:46

skoya76 force-pushed the add-backup-exported-diff-size-metric branch from 427a6b6 to d9183b6 Compare July 3, 2026 05:46

skoya76 force-pushed the add-backup-exported-diff-size-metric branch from d9183b6 to 865065b Compare July 3, 2026 05:53

skoya76 requested a review from satoru-takeuchi July 3, 2026 06:42

satoru-takeuchi approved these changes Jul 3, 2026

View reviewed changes

satoru-takeuchi merged commit 24555b3 into main Jul 3, 2026
15 checks passed

satoru-takeuchi deleted the add-backup-exported-diff-size-metric branch July 3, 2026 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

metrics: add backup_exported_diff_size_bytes#301

metrics: add backup_exported_diff_size_bytes#301
satoru-takeuchi merged 1 commit into
mainfrom
add-backup-exported-diff-size-metric

skoya76 commented Jun 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satoru-takeuchi Jul 2, 2026

Uh oh!

satoru-takeuchi Jul 2, 2026

Uh oh!

skoya76 commented Jul 3, 2026

Uh oh!

satoru-takeuchi left a comment

Uh oh!

skoya76 commented Jul 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	Help: "Total bytes of the uploaded diff parts.",
	Help: "The size of the uploaded backup diff data",

Uh oh!

Conversation

skoya76 commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satoru-takeuchi Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

satoru-takeuchi Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

skoya76 commented Jul 3, 2026

Uh oh!

satoru-takeuchi left a comment

Choose a reason for hiding this comment

Uh oh!

skoya76 commented Jul 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skoya76 commented Jun 26, 2026 •

edited

Loading