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,