From af2a359e45e642891aba20202c6fa942b410dc85 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 9 May 2026 20:02:26 -0700 Subject: [PATCH] feat(s3/lifecycle): metadata_only_total Prometheus counter (#9399) Operator-visible signal for the metadata-only delete path landed in PR 9390. Increment seaweedfs_s3_lifecycle_metadata_only_total{bucket, rule_hash} after each successful unversioned or noncurrent / expired- marker delete that took the skip-chunk path. Suspended-versioning null delete is intentionally not counted: that path's nil err can mean "deleted" or "NotFound", so a count there would over-report. rule_hash is hex-encoded for label safety; nil bytes collapse to "". DeleteBucketMetrics tears the new series down alongside the existing lifecycle counters when a bucket is removed. --- weed/s3api/s3api_internal_lifecycle.go | 15 +++++++ weed/s3api/s3api_internal_lifecycle_test.go | 50 +++++++++++++++++++++ weed/stats/metrics.go | 17 +++++++ 3 files changed, 82 insertions(+) diff --git a/weed/s3api/s3api_internal_lifecycle.go b/weed/s3api/s3api_internal_lifecycle.go index cec63f112..6199bf7b0 100644 --- a/weed/s3api/s3api_internal_lifecycle.go +++ b/weed/s3api/s3api_internal_lifecycle.go @@ -3,6 +3,7 @@ package s3api import ( "bytes" "context" + "encoding/hex" "errors" "path" "strings" @@ -12,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/s3_lifecycle_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle" + stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" ) // LifecycleDelete executes one (rule, action) verdict: re-fetch, identity @@ -99,6 +101,7 @@ func (s3a *S3ApiServer) lifecycleDispatch(ctx context.Context, req *s3_lifecycle } return retryLater("TRANSPORT_ERROR: deleteUnversioned: " + err.Error()), nil } + recordMetadataOnlyIf(metadataOnly, req) return done(), nil } @@ -141,6 +144,7 @@ func (s3a *S3ApiServer) lifecycleDispatch(ctx context.Context, req *s3_lifecycle } return retryLater("TRANSPORT_ERROR: deleteSpecificVersion: " + err.Error()), nil } + recordMetadataOnlyIf(metadataOnly, req) return done(), nil case s3_lifecycle_pb.ActionKind_ABORT_MPU: @@ -343,6 +347,17 @@ func identityMatches(live, want *s3_lifecycle_pb.EntryIdentity) bool { return bytes.Equal(live.ExtendedHash, want.ExtendedHash) } +// recordMetadataOnlyIf bumps the metadata-only counter when on=true. +// Skipped when off so callers don't need a guard at every call site. +// rule_hash is hex-encoded so operators can group by rule when +// debugging; nil rule_hash collapses to the empty string. +func recordMetadataOnlyIf(on bool, req *s3_lifecycle_pb.LifecycleDeleteRequest) { + if !on || req == nil { + return + } + stats_collect.S3LifecycleMetadataOnlyCounter.WithLabelValues(req.Bucket, hex.EncodeToString(req.RuleHash)).Inc() +} + func done() *s3_lifecycle_pb.LifecycleDeleteResponse { return &s3_lifecycle_pb.LifecycleDeleteResponse{Outcome: s3_lifecycle_pb.LifecycleDeleteOutcome_DONE} } diff --git a/weed/s3api/s3api_internal_lifecycle_test.go b/weed/s3api/s3api_internal_lifecycle_test.go index 9d2d8b156..85482d53a 100644 --- a/weed/s3api/s3api_internal_lifecycle_test.go +++ b/weed/s3api/s3api_internal_lifecycle_test.go @@ -4,9 +4,11 @@ import ( "bytes" "testing" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/pb/s3_lifecycle_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle" + stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" ) func TestComputeEntryIdentity_BasicFields(t *testing.T) { @@ -147,3 +149,51 @@ func TestLifecycleAbortMPU_RejectsTraversalUploadIDs(t *testing.T) { }) } } + +func TestRecordMetadataOnlyIf_OnlyFiresWhenOn(t *testing.T) { + // Counter must increment exactly once per (bucket, hex(rule_hash)) + // when on=true, and not at all when on=false. Other lifecycle paths + // in the same suite share the global counter — use distinct bucket + // names per test so series don't bleed. + c := stats_collect.S3LifecycleMetadataOnlyCounter + bucket := "bk-counter-on" + hash := []byte{0xde, 0xad, 0xbe, 0xef, 0x01, 0x02, 0x03, 0x04} + hexHash := "deadbeef01020304" + + before := testutil.ToFloat64(c.WithLabelValues(bucket, hexHash)) + recordMetadataOnlyIf(true, &s3_lifecycle_pb.LifecycleDeleteRequest{ + Bucket: bucket, + RuleHash: hash, + }) + if got := testutil.ToFloat64(c.WithLabelValues(bucket, hexHash)); got != before+1 { + t.Fatalf("on=true should bump by 1; before=%v after=%v", before, got) + } + + beforeOff := testutil.ToFloat64(c.WithLabelValues("bk-counter-off", hexHash)) + recordMetadataOnlyIf(false, &s3_lifecycle_pb.LifecycleDeleteRequest{ + Bucket: "bk-counter-off", + RuleHash: hash, + }) + if got := testutil.ToFloat64(c.WithLabelValues("bk-counter-off", hexHash)); got != beforeOff { + t.Fatalf("on=false should not bump; before=%v after=%v", beforeOff, got) + } +} + +func TestRecordMetadataOnlyIf_NilRequestSafe(t *testing.T) { + // A nil req is a defensive no-op; never panic on the prometheus + // label call which would otherwise dereference req.Bucket. + recordMetadataOnlyIf(true, nil) +} + +func TestRecordMetadataOnlyIf_EmptyRuleHashCollapsesToEmptyLabel(t *testing.T) { + // Bootstrap or test paths may not stamp a rule hash; the label + // must end up as an empty string rather than panicking on + // hex.EncodeToString(nil). + c := stats_collect.S3LifecycleMetadataOnlyCounter + bucket := "bk-counter-emptyhash" + before := testutil.ToFloat64(c.WithLabelValues(bucket, "")) + recordMetadataOnlyIf(true, &s3_lifecycle_pb.LifecycleDeleteRequest{Bucket: bucket}) + if got := testutil.ToFloat64(c.WithLabelValues(bucket, "")); got != before+1 { + t.Fatalf("nil rule_hash should produce empty-label series; before=%v after=%v", before, got) + } +} diff --git a/weed/stats/metrics.go b/weed/stats/metrics.go index b5e7965c4..2aeb843ba 100644 --- a/weed/stats/metrics.go +++ b/weed/stats/metrics.go @@ -577,6 +577,21 @@ var ( Name: "bootstrap_dispatch_total", Help: "Counter of bootstrap-walk Delete dispatches by bucket and action kind.", }, []string{"bucket", "kind"}) + + // S3LifecycleMetadataOnlyCounter counts successful LifecycleDelete + // dispatches that took the metadata-only path — entry was removed + // without per-chunk DeleteFile RPCs because the entry's Attributes + // .TtlSec > 0 and the volume's natural TTL will reclaim chunks. Per- + // rule cardinality (rule_hash hex-encoded) lets operators identify + // which specific rule is exercising the optimization; in clusters + // with many rules this can be reduced via Prometheus relabeling. + S3LifecycleMetadataOnlyCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: Namespace, + Subsystem: "s3_lifecycle", + Name: "metadata_only_total", + Help: "Counter of LifecycleDelete completions that skipped per-chunk delete (volume TTL reclaim).", + }, []string{"bucket", "rule_hash"}) ) func init() { @@ -652,6 +667,7 @@ func init() { Gather.MustRegister(S3LifecycleCursorMinTsNs) Gather.MustRegister(S3LifecycleEventCounter) Gather.MustRegister(S3LifecycleBootstrapDispatchCounter) + Gather.MustRegister(S3LifecycleMetadataOnlyCounter) Gather.MustRegister(UploadErrorCounter) @@ -727,6 +743,7 @@ func DeleteBucketMetrics(bucket string) { c += S3BucketObjectCountGauge.DeletePartialMatch(labels) c += S3LifecycleDispatchCounter.DeletePartialMatch(labels) c += S3LifecycleBootstrapDispatchCounter.DeletePartialMatch(labels) + c += S3LifecycleMetadataOnlyCounter.DeletePartialMatch(labels) glog.V(0).Infof("delete bucket metrics, %s: %d", bucket, c) }