From c17188efdca3dbb43561b21f7cf91d243c21092f Mon Sep 17 00:00:00 2001 From: PrashantBtkl Date: Fri, 9 Oct 2026 13:22:04 +0530 Subject: [PATCH] fix(metrics): keep VMs cloned from one image apart in the instance label instance = md5(machine-id + system-uuid) collapses clones that share /etc/machine-id when the hypervisor reports no system UUID or the same one, merging their series in the store. Mix in the cloud instance id and the permanent hardware NIC addresses (skipping virtual, bridge and boot-random ones). With no extras the value is unchanged. --legacy-instance-id keeps the previous derivation. Upgrading changes instance once on hosts that have a cloud instance id or a hardware NIC. Refs nudgebee/nudgebee-enterprise#40582 Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 8 +++ flags/flags.go | 1 + main.go | 7 ++- prom/instance.go | 91 +++++++++++++++++++++++++++ prom/instance_test.go | 139 ++++++++++++++++++++++++++++++++++++++++++ prom/remote_writer.go | 16 +++-- 6 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 prom/instance.go create mode 100644 prom/instance_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ca30290..c1c8c9be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- The metrics `instance` label now also mixes in the cloud instance id and the + permanent hardware NIC addresses (virtual, bridge and boot-random addresses + are ignored). VMs cloned from one image share `/etc/machine-id`, and when the + hypervisor gives them the same or no system UUID they previously collapsed + into one `instance` and overwrote each other's series. **Upgrading changes + `instance` once on every host that has a cloud instance id or a hardware NIC.** + Start with `--legacy-instance-id` (`LEGACY_INSTANCE_ID=true`) to keep the + previous value. - Prometheus `job` label and outbound `User-Agent` are now `nudgebee-node-agent`. Update any dashboards or alerts that filter on `job="coroot-node-agent"`. diff --git a/flags/flags.go b/flags/flags.go index 1d04212c..f223d4f3 100644 --- a/flags/flags.go +++ b/flags/flags.go @@ -71,6 +71,7 @@ var ( ScrapeInterval = kingpin.Flag("scrape-interval", "How often to gather metrics from the agent").Default("15s").Envar("SCRAPE_INTERVAL").Duration() WalDir = kingpin.Flag("wal-dir", "Path to where the agent stores data (e.g. the metrics Write-Ahead Log)").Default("/tmp/nudgebee-node-agent").Envar("WAL_DIR").String() + LegacyInstanceID = kingpin.Flag("legacy-instance-id", "Derive the metrics `instance` label from machine-id and system UUID only, as releases before cloned-VM support did. Keeps existing series after an upgrade, but VMs cloned from one image whose system UUID is missing or identical share an instance").Default("false").Envar("LEGACY_INSTANCE_ID").Bool() MaxSpoolSize = kingpin.Flag("max-spool-size", "Maximum size of the on-disk spool used to buffer data when it cannot be sent to collector. Supports size suffixes like KB, MB, or GB.").Default("500MB").Envar("MAX_SPOOL_SIZE").Bytes() ResolveDns = kingpin.Flag("resolve-dns", "should resolve DNS").Default("false").Envar("RESOLVE_DNS").Bool() IgnoreControlPlane = kingpin.Flag("ignore-control-plane", "ignore control plane like loki").Default("karpenter,loki,prometheus,grafana,kubelet,etcd,apiserver,victoria,nudgebee-agent,kube-system").Envar("IGNORE_CONTROL_PLANE").String() diff --git a/main.go b/main.go index e5f04149..e8067b2e 100644 --- a/main.go +++ b/main.go @@ -261,7 +261,12 @@ func main() { profiling.Start() defer profiling.Stop() - if err := prom.StartAgent(registry, machineId, systemUuid); err != nil { + cloudInstanceID := "" + if md := nodeCollector.Metadata(); md != nil { + cloudInstanceID = md.InstanceId + } + identityExtras := prom.HostIdentityExtras(cloudInstanceID, proc.HostPath("/sys/class/net")) + if err := prom.StartAgent(registry, machineId, systemUuid, identityExtras); err != nil { klog.Exitln(err) } diff --git a/prom/instance.go b/prom/instance.go new file mode 100644 index 00000000..45c22a4b --- /dev/null +++ b/prom/instance.go @@ -0,0 +1,91 @@ +package prom + +import ( + "crypto/md5" + "encoding/hex" + "os" + "path/filepath" + "sort" + "strings" +) + +// InstanceID derives the `instance` label that tells one machine's series from +// another's in the metrics store. +// +// machine-id alone is not enough: VMs cloned from one image share it. The system +// UUID (SMBIOS product_uuid) usually differs per clone, so it is folded in. But +// when the hypervisor reports no UUID, or the same one for a copied disk, two +// clones collapse into one instance and their series overwrite each other. The +// extras (cloud instance id, hardware NIC addresses) are per-machine inputs that +// still differ in those cases. +// +// With no extras the result is byte-for-byte what earlier releases produced, so a +// host that has nothing extra to add keeps its existing series. +func InstanceID(machineID, systemUUID string, extras ...string) string { + uuid := strings.ReplaceAll(systemUUID, "-", "") + var parts []string + if uuid != "" && uuid != machineID { + parts = append(parts, uuid) + } + for _, e := range extras { + if e != "" { + // A separator keeps ("ab","c") and ("a","bc") from hashing alike. + parts = append(parts, "\x00"+e) + } + } + if len(parts) == 0 { + return machineID + } + h := md5.New() + h.Write([]byte(machineID)) + for _, p := range parts { + h.Write([]byte(p)) + } + return hex.EncodeToString(h.Sum(nil)) +} + +// HostIdentityExtras collects the per-machine inputs InstanceID mixes in beyond +// machine-id and system UUID: the cloud instance id when running in a cloud, and +// the permanent hardware addresses of the host's NICs. +func HostIdentityExtras(cloudInstanceID, sysClassNet string) []string { + var extras []string + if cloudInstanceID != "" { + extras = append(extras, "cloud:"+cloudInstanceID) + } + for _, mac := range hardwareMACs(sysClassNet) { + extras = append(extras, "mac:"+mac) + } + return extras +} + +// hardwareMACs lists the sorted, permanent MAC addresses of real NICs under +// /sys/class/net. Interfaces without a backing device (bridges, veth, docker0, +// tunnels) are skipped, as are addresses the kernel generated at boot +// (addr_assign_type 1, random): those would change identity on every restart. +func hardwareMACs(sysClassNet string) []string { + entries, err := os.ReadDir(sysClassNet) + if err != nil { + return nil + } + var macs []string + for _, e := range entries { + dir := filepath.Join(sysClassNet, e.Name()) + if _, err := os.Stat(filepath.Join(dir, "device")); err != nil { + continue + } + if t, err := os.ReadFile(filepath.Join(dir, "addr_assign_type")); err == nil && strings.TrimSpace(string(t)) == "1" { + continue + } + b, err := os.ReadFile(filepath.Join(dir, "address")) + if err != nil { + continue + } + mac := strings.ToLower(strings.TrimSpace(string(b))) + if mac == "" || mac == "00:00:00:00:00:00" { + continue + } + macs = append(macs, mac) + } + sort.Strings(macs) + return macs +} diff --git a/prom/instance_test.go b/prom/instance_test.go new file mode 100644 index 00000000..57d24020 --- /dev/null +++ b/prom/instance_test.go @@ -0,0 +1,139 @@ +package prom + +import ( + "crypto/md5" + "encoding/hex" + "os" + "path/filepath" + "testing" +) + +// legacy is the derivation earlier releases used; InstanceID must reproduce it +// when there is nothing extra to add, so hosts keep their existing series. +func legacy(machineID, systemUUID string) string { + instance := machineID + if s := systemUUIDNoDashes(systemUUID); s != "" && s != machineID { + h := md5.New() + h.Write([]byte(machineID)) + h.Write([]byte(s)) + instance = hex.EncodeToString(h.Sum(nil)) + } + return instance +} + +func systemUUIDNoDashes(s string) string { + out := "" + for _, r := range s { + if r != '-' { + out += string(r) + } + } + return out +} + +func TestInstanceID_MatchesLegacyWithoutExtras(t *testing.T) { + for _, c := range []struct{ machine, uuid string }{ + {"aaaa", "11111111-2222-3333-4444-555555555555"}, + {"aaaa", ""}, + {"aaaa", "aaaa"}, + {"", ""}, + } { + if got, want := InstanceID(c.machine, c.uuid), legacy(c.machine, c.uuid); got != want { + t.Errorf("InstanceID(%q,%q) = %q, legacy %q", c.machine, c.uuid, got, want) + } + } +} + +// The failure this exists for: clones share machine-id and the hypervisor gives +// them the same (or no) system UUID, so the legacy derivation cannot tell them apart. +func TestInstanceID_ClonesWithSameMachineAndUUIDDiffer(t *testing.T) { + const machine, uuid = "aaaa", "11111111-2222-3333-4444-555555555555" + if legacy(machine, uuid) != legacy(machine, uuid) { + t.Fatal("legacy must be deterministic") + } + a := InstanceID(machine, uuid, "mac:52:54:00:00:00:01") + b := InstanceID(machine, uuid, "mac:52:54:00:00:00:02") + if a == b { + t.Fatal("clones with different NICs must get different instances") + } + c := InstanceID(machine, "", "cloud:i-0abc") + d := InstanceID(machine, "", "cloud:i-0def") + if c == d { + t.Fatal("clones with different cloud instance ids must get different instances") + } +} + +func TestInstanceID_Stable(t *testing.T) { + a := InstanceID("m", "u", "cloud:i-1", "mac:aa", "mac:bb") + if b := InstanceID("m", "u", "cloud:i-1", "mac:aa", "mac:bb"); a != b { + t.Fatalf("not stable: %q vs %q", a, b) + } +} + +func TestInstanceID_ExtrasAreDelimited(t *testing.T) { + if InstanceID("m", "", "ab", "c") == InstanceID("m", "", "a", "bc") { + t.Fatal("extras must not run together") + } +} + +func TestInstanceID_EmptyExtrasIgnored(t *testing.T) { + if InstanceID("m", "u", "", "") != InstanceID("m", "u") { + t.Fatal("empty extras must not change the instance") + } +} + +func writeNIC(t *testing.T, root, name string, device bool, address, assign string) { + t.Helper() + dir := filepath.Join(root, name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if device { + if err := os.Mkdir(filepath.Join(dir, "device"), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(dir, "address"), []byte(address+"\n"), 0o644); err != nil { + t.Fatal(err) + } + if assign != "" { + if err := os.WriteFile(filepath.Join(dir, "addr_assign_type"), []byte(assign+"\n"), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func TestHardwareMACs(t *testing.T) { + root := t.TempDir() + writeNIC(t, root, "ens3", true, "52:54:00:AB:CD:02", "0") // real NIC, upper case + writeNIC(t, root, "ens4", true, "52:54:00:ab:cd:01", "0") + writeNIC(t, root, "docker0", false, "02:42:ac:11:00:01", "") // no backing device + writeNIC(t, root, "veth1", false, "de:ad:be:ef:00:01", "1") // virtual, random + writeNIC(t, root, "ens5", true, "7a:00:00:00:00:09", "1") // random at boot: unstable + writeNIC(t, root, "ens6", true, "00:00:00:00:00:00", "0") // unset + writeNIC(t, root, "lo", false, "00:00:00:00:00:00", "") + + got := hardwareMACs(root) + want := []string{"52:54:00:ab:cd:01", "52:54:00:ab:cd:02"} + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestHardwareMACs_MissingDir(t *testing.T) { + if got := hardwareMACs(filepath.Join(t.TempDir(), "nope")); got != nil { + t.Fatalf("got %v", got) + } +} + +func TestHostIdentityExtras(t *testing.T) { + root := t.TempDir() + writeNIC(t, root, "ens3", true, "52:54:00:00:00:01", "0") + got := HostIdentityExtras("i-0abc", root) + if len(got) != 2 || got[0] != "cloud:i-0abc" || got[1] != "mac:52:54:00:00:00:01" { + t.Fatalf("got %v", got) + } + if got := HostIdentityExtras("", filepath.Join(root, "missing")); len(got) != 0 { + t.Fatalf("got %v", got) + } +} diff --git a/prom/remote_writer.go b/prom/remote_writer.go index 127363ca..e5d52a76 100644 --- a/prom/remote_writer.go +++ b/prom/remote_writer.go @@ -2,8 +2,6 @@ package prom import ( "bytes" - "crypto/md5" - "encoding/hex" "errors" "fmt" "net/http" @@ -39,7 +37,9 @@ type Agent struct { maxSpoolSize int64 } -func StartAgent(reg *prometheus.Registry, machineId, systemUuid string) error { +// StartAgent starts remote-writing reg. identityExtras are per-machine inputs +// (see HostIdentityExtras) that keep clones of one image apart in the store. +func StartAgent(reg *prometheus.Registry, machineId, systemUuid string, identityExtras []string) error { if *flags.MetricsEndpoint == nil { return nil } @@ -49,13 +49,11 @@ func StartAgent(reg *prometheus.Registry, machineId, systemUuid string) error { up.Set(1) reg.MustRegister(up) - instance := machineId - if s := strings.ReplaceAll(systemUuid, "-", ""); s != "" && s != machineId { - hash := md5.New() - hash.Write([]byte(machineId)) - hash.Write([]byte(s)) - instance = hex.EncodeToString(hash.Sum(nil)) + if *flags.LegacyInstanceID { + identityExtras = nil } + instance := InstanceID(machineId, systemUuid, identityExtras...) + klog.Infoln("metrics instance:", instance) a := &Agent{ reg: reg,