From 4b9f809ff38108eeea334b6ce751296d0ed68229 Mon Sep 17 00:00:00 2001 From: Lev Brouk Date: Tue, 30 Jun 2026 11:46:42 -0700 Subject: [PATCH] fix(metrics) fix runaway blob file_type attribute values --- packages/shared/pkg/storage/paths.go | 37 +++++++++++++++++------ packages/shared/pkg/storage/paths_test.go | 37 +++++++++++++++++++++++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/packages/shared/pkg/storage/paths.go b/packages/shared/pkg/storage/paths.go index 8f790caa39..d3086e0eb0 100644 --- a/packages/shared/pkg/storage/paths.go +++ b/packages/shared/pkg/storage/paths.go @@ -125,18 +125,35 @@ func seekableObjectType(path string) (SeekableObjectType, CompressionType) { } } -// blobType derives the metric file_type from a blob's last path segment, -// stripping .header so read.blob shares read.read's file_type vocabulary. +// Blob file_type values. The read.blob* metrics cover whole-object (WriteTo) +// reads only — a small, fixed set. The seekable data files (memfile, +// rootfs.ext4) are NOT blobs: they are range-read and recorded under read.read +// with their own vocabulary, so they never appear here. Everything that isn't a +// known type collapses to "other": content-addressed cache blobs are keyed by +// hash, and letting those (or any per-build path) into the label is what blew +// file_type up to ~10^5 distinct values. +const ( + blobTypeHeader = "header" // memfile/rootfs header sidecar (*.header) + blobTypeSnapfile = "snapfile" // VM snapshot file + blobTypeMetadata = "metadata" // metadata.json + blobTypeOther = "other" // anything else — never a raw hash/ID +) + +// blobType classifies a whole-object blob read into the fixed file_type set +// above; anything unrecognized collapses to "other" to keep the label bounded. func blobType(path string) string { - name := path - if i := strings.LastIndex(name, "/"); i >= 0 { - name = name[i+1:] - } - if base, ok := strings.CutSuffix(name, HeaderSuffix); ok { - return base - } + name := StripCompression(path[strings.LastIndex(path, "/")+1:]) - return name + switch { + case strings.HasSuffix(name, HeaderSuffix): + return blobTypeHeader + case name == SnapfileName: + return blobTypeSnapfile + case name == MetadataName: + return blobTypeMetadata + default: + return blobTypeOther + } } func compressionType(name string) CompressionType { diff --git a/packages/shared/pkg/storage/paths_test.go b/packages/shared/pkg/storage/paths_test.go index a7c8ecb0da..4210dce1f5 100644 --- a/packages/shared/pkg/storage/paths_test.go +++ b/packages/shared/pkg/storage/paths_test.go @@ -36,3 +36,40 @@ func TestSeekableKindFromPath(t *testing.T) { }) } } + +// TestBlobType pins blobType to its fixed vocabulary: the known whole-object +// blobs (header, snapfile, metadata) and "other" for everything else. The +// seekable data files memfile/rootfs.ext4 are NOT blobs and must never be +// returned, and no per-hash/per-build path may leak — the cardinality blowup +// from #3063. +func TestBlobType(t *testing.T) { + t.Parallel() + + p := Paths{BuildID: "11111111-1111-1111-1111-111111111111"} + const hash = "deadbeefcafef00ddeadbeefcafef00ddeadbeefcafef00ddeadbeefcafef00d" + + cases := []struct { + name string + path string + want string + }{ + {"memfile header", p.MemfileHeader(), blobTypeHeader}, + {"rootfs header", p.RootfsHeader(), blobTypeHeader}, + {"snapfile", p.Snapfile(), blobTypeSnapfile}, + {"metadata", p.Metadata(), blobTypeMetadata}, + // Not known blobs — bounded "other", never the raw name or hash. + {"memfile data file is not a blob", p.Memfile(), blobTypeOther}, + {"rootfs data file is not a blob", p.RootfsCompressed(CompressionZstd), blobTypeOther}, + {"layer files keyed by hash", "scope-abc/files/" + hash + ".tar", blobTypeOther}, + {"unknown collapses to other", p.BuildID + "/something-else", blobTypeOther}, + {"bare hash never leaks", hash, blobTypeOther}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, c.want, blobType(c.path)) + }) + } +}