diff --git a/internal/monitoring/broadcast_conversion_test.go b/internal/monitoring/broadcast_conversion_test.go new file mode 100644 index 0000000000..0702662cb2 --- /dev/null +++ b/internal/monitoring/broadcast_conversion_test.go @@ -0,0 +1,130 @@ +package monitoring + +import ( + "bytes" + "encoding/json" + "errors" + "math" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +// These deliberately use named nil collections: a nil slice/map can still +// have a custom wire representation, unlike a nil pointer marshaler. +type projectionNilSlice []string + +func (projectionNilSlice) MarshalJSON() ([]byte, error) { return []byte(`["custom"]`), nil } + +type projectionNilMap map[string]string + +func (projectionNilMap) MarshalJSON() ([]byte, error) { return []byte(`{"custom":true}`), nil } + +type projectionNilTextSlice []string + +func (projectionNilTextSlice) MarshalText() ([]byte, error) { return []byte("custom-text"), nil } + +type projectionNilTextMap map[string]string + +func (projectionNilTextMap) MarshalText() ([]byte, error) { return []byte("custom-text"), nil } + +type projectionInvalidJSON struct{} + +func (projectionInvalidJSON) MarshalJSON() ([]byte, error) { return []byte(`{broken`), nil } + +type projectionFailedJSON struct{} + +func (projectionFailedJSON) MarshalJSON() ([]byte, error) { + return nil, errors.New("synthetic encode failure") +} + +func projectionRawJSONReference(value any) json.RawMessage { + if value == nil { + return nil + } + encoded, err := json.Marshal(value) + if err != nil || string(encoded) == "null" { + return nil + } + return encoded +} + +func TestBroadcastRawJSONPreservesAbsentEmptyAndFailureSemantics(t *testing.T) { + cases := []struct { + name string + value any + }{ + {"untyped-nil", nil}, {"nil-facet", (*unifiedresources.K8sData)(nil)}, + {"nil-slice", []string(nil)}, {"nil-map", map[string]string(nil)}, + {"empty-slice", []string{}}, {"empty-map", map[string]string{}}, + {"named-nil-slice-marshaler", projectionNilSlice(nil)}, + {"named-nil-map-marshaler", projectionNilMap(nil)}, + {"named-nil-slice-text-marshaler", projectionNilTextSlice(nil)}, + {"named-nil-map-text-marshaler", projectionNilTextMap(nil)}, + {"nil-pointer-marshaler", (*projectionNilSlice)(nil)}, + {"invalid-json", projectionInvalidJSON{}}, {"encode-failure", projectionFailedJSON{}}, + {"invalid-float", math.NaN()}, {"raw-null", json.RawMessage(`null`)}, + {"unicode-html", map[string]string{"label": " & café \u2028"}}, + {"facet", &unifiedresources.K8sData{ClusterID: "synthetic", Labels: map[string]string{"env": "test"}}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, want := monitorRawJSON(tc.value), projectionRawJSONReference(tc.value) + if !bytes.Equal(got, want) || (got == nil) != (want == nil) { + t.Fatalf("raw JSON changed: got %q, want %q", got, want) + } + }) + } +} + +func TestBroadcastAbsentFacetsDoNotAllocateNull(t *testing.T) { + for _, value := range []any{(*unifiedresources.K8sData)(nil), (*unifiedresources.StorageMeta)(nil), []unifiedresources.ResourceIncident(nil)} { + allocations := testing.AllocsPerRun(100, func() { + if monitorRawJSON(value) != nil { + panic("absent facet became present") + } + }) + if allocations != 0 { + t.Fatalf("absent %T allocated discarded JSON: %.0f", value, allocations) + } + } +} + +func TestBroadcastConversionBorrowedHelpersKeepOutputOwned(t *testing.T) { + now := time.Date(2026, 10, 5, 10, 0, 0, 0, time.UTC) + temperature := 42.5 + total, used := int64(1000), int64(250) + resource := unifiedresources.Resource{ID: "pod:synthetic", Type: unifiedresources.ResourceTypePod, Name: "synthetic", LastSeen: now, + Sources: []unifiedresources.DataSource{unifiedresources.SourceK8s}, Tags: []string{"public"}, + Identity: unifiedresources.ResourceIdentity{Hostnames: []string{"synthetic"}, IPAddresses: []string{"192.0.2.1"}}, + Metrics: &unifiedresources.ResourceMetrics{CPU: &unifiedresources.MetricValue{Percent: 0.25}, Memory: &unifiedresources.MetricValue{Total: &total, Used: &used}}, + Kubernetes: &unifiedresources.K8sData{ClusterID: "synthetic", PodPhase: "Running", Labels: map[string]string{"env": "synthetic"}, Temperature: &temperature, UptimeSeconds: 60}, + } + before, err := json.Marshal(resource) + if err != nil { + t.Fatal(err) + } + projected := models.ConvertResourceToFrontend(monitorResourceToConvertInput(resource)) + projected.Tags[0] = "caller-change" + projected.Sources[0] = "caller-change" + projected.Labels["env"] = "caller-change" + projected.Identity.IPs[0] = "caller-change" + *projected.Temperature = 100 + *projected.Uptime = 100 + *projected.Memory.Total = 2000 + *projected.Memory.Used = 1000 + for _, raw := range []json.RawMessage{projected.Canonical, projected.Kubernetes, projected.MetricsTarget, projected.DiscoveryTarget, projected.Capabilities} { + if len(raw) > 0 { + raw[0] = 'x' + } + } + after, err := json.Marshal(resource) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) { + t.Fatal("converter or output caller mutated source resource") + } +} diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index f65c96057a..3c398a8cbf 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -2592,7 +2592,7 @@ func TestPollersHonorDisabledInstanceFlag(t *testing.T) { func TestMonitorUptimeFallsBackToCanonicalResourceUptime(t *testing.T) { t.Run("falls through to Resource.Uptime when no source carve-out is set", func(t *testing.T) { resource := unifiedresources.Resource{Uptime: 123456} - got := monitorUptime(resource) + got := monitorUptime(&resource) if got == nil || *got != 123456 { t.Fatalf("monitorUptime() = %v, want 123456 from canonical Resource.Uptime", got) } @@ -2603,21 +2603,21 @@ func TestMonitorUptimeFallsBackToCanonicalResourceUptime(t *testing.T) { Uptime: 999999, Agent: &unifiedresources.AgentData{UptimeSeconds: 100}, } - got := monitorUptime(resource) + got := monitorUptime(&resource) if got == nil || *got != 100 { t.Fatalf("monitorUptime() = %v, want 100 from agent (precedence over canonical Uptime)", got) } resource.Agent = nil resource.Proxmox = &unifiedresources.ProxmoxData{Uptime: 200} - got = monitorUptime(resource) + got = monitorUptime(&resource) if got == nil || *got != 200 { t.Fatalf("monitorUptime() = %v, want 200 from proxmox (precedence over canonical Uptime)", got) } }) t.Run("returns nil when nothing populates", func(t *testing.T) { - if got := monitorUptime(unifiedresources.Resource{}); got != nil { + if got := monitorUptime(&unifiedresources.Resource{}); got != nil { t.Fatalf("monitorUptime() = %v, want nil when all sources empty", got) } }) diff --git a/internal/monitoring/guest_config_conversion_coverage_test.go b/internal/monitoring/guest_config_conversion_coverage_test.go index f334213129..1904bc3bfa 100644 --- a/internal/monitoring/guest_config_conversion_coverage_test.go +++ b/internal/monitoring/guest_config_conversion_coverage_test.go @@ -209,7 +209,7 @@ func TestMonitorFrontendAndMetricHelpers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := monitorFrontendResourceType(tt.resource); got != tt.want { + if got := monitorFrontendResourceType(&tt.resource); got != tt.want { t.Fatalf("monitorFrontendResourceType() = %q, want %q", got, tt.want) } }) @@ -320,7 +320,7 @@ func TestMonitorFrontendAndMetricHelpers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := monitorPlatformType(tt.resource, tt.resourceType); got != tt.want { + if got := monitorPlatformType(&tt.resource, tt.resourceType); got != tt.want { t.Fatalf("monitorPlatformType() = %q, want %q", got, tt.want) } }) @@ -387,7 +387,7 @@ func TestMonitorFrontendAndMetricHelpers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := monitorPlatformID(tt.resource, tt.resourceType); got != tt.want { + if got := monitorPlatformID(&tt.resource, tt.resourceType); got != tt.want { t.Fatalf("monitorPlatformID() = %q, want %q", got, tt.want) } }) @@ -482,7 +482,7 @@ func TestMonitorFrontendAndMetricHelpers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := monitorFrontendStatus(tt.resource, tt.resourceType); got != tt.want { + if got := monitorFrontendStatus(&tt.resource, tt.resourceType); got != tt.want { t.Fatalf("monitorFrontendStatus() = %q, want %q", got, tt.want) } }) @@ -553,7 +553,7 @@ func TestMonitorPlatformData(t *testing.T) { }, } - payload := decodePlatformDataPayload(t, monitorPlatformData(resource, "node", "ignored")) + payload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "node", "ignored")) if payload["instance"] != "pve-a" { t.Fatalf("instance = %#v, want pve-a", payload["instance"]) } @@ -584,7 +584,7 @@ func TestMonitorPlatformData(t *testing.T) { }, } - payload := decodePlatformDataPayload(t, monitorPlatformData(resource, "vm", "ignored")) + payload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "vm", "ignored")) if payload["networkIn"] != float64(11) { t.Fatalf("networkIn = %#v, want 11", payload["networkIn"]) } @@ -614,7 +614,7 @@ func TestMonitorPlatformData(t *testing.T) { }, } - payload := decodePlatformDataPayload(t, monitorPlatformData(resource, "pbs", "ignored")) + payload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "pbs", "ignored")) if payload["memoryUsed"] != float64(20) { t.Fatalf("memoryUsed = %#v, want 20", payload["memoryUsed"]) } @@ -630,7 +630,7 @@ func TestMonitorPlatformData(t *testing.T) { ParentID: &parent, } - storagePayload := decodePlatformDataPayload(t, monitorPlatformData(resource, "storage", "pve-a")) + storagePayload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "storage", "pve-a")) if storagePayload["instance"] != "pve-a" { t.Fatalf("instance = %#v, want pve-a", storagePayload["instance"]) } @@ -641,7 +641,7 @@ func TestMonitorPlatformData(t *testing.T) { t.Fatalf("active = %#v, want true", storagePayload["active"]) } - poolPayload := decodePlatformDataPayload(t, monitorPlatformData(resource, "pool", "pve-a")) + poolPayload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "pool", "pve-a")) if poolPayload["active"] != true { t.Fatalf("pool active = %#v, want true", poolPayload["active"]) } @@ -667,7 +667,7 @@ func TestMonitorPlatformData(t *testing.T) { }, } - payload := decodePlatformDataPayload(t, monitorPlatformData(resource, "storage", "ignored")) + payload := decodePlatformDataPayload(t, monitorPlatformData(&resource, "storage", "ignored")) if payload["platform"] != "unraid" { t.Fatalf("platform = %#v, want unraid", payload["platform"]) } @@ -687,10 +687,10 @@ func TestMonitorPlatformData(t *testing.T) { }) t.Run("nil payload branches", func(t *testing.T) { - if got := monitorPlatformData(unifiedresources.Resource{}, "agent", "id"); got != nil { + if got := monitorPlatformData(&unifiedresources.Resource{}, "agent", "id"); got != nil { t.Fatalf("expected nil payload for agent without resource agent data, got %s", string(got)) } - if got := monitorPlatformData(unifiedresources.Resource{}, "unknown", "id"); got != nil { + if got := monitorPlatformData(&unifiedresources.Resource{}, "unknown", "id"); got != nil { t.Fatalf("expected nil payload for unknown type, got %s", string(got)) } }) diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index c722fc70f0..53182cf222 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -3,6 +3,7 @@ package monitoring import ( "context" "crypto/sha256" + "encoding" "encoding/hex" "encoding/json" stderrors "errors" @@ -6554,17 +6555,17 @@ func validBroadcastMetricsTarget( } func monitorResourceToConvertInput(resource unifiedresources.Resource) models.ResourceConvertInput { - resourceType := monitorFrontendResourceType(resource) + resourceType := monitorFrontendResourceType(&resource) if resource.DiscoveryTarget == nil { - resource.DiscoveryTarget = monitorDiscoveryTarget(resource, resourceType) + resource.DiscoveryTarget = monitorDiscoveryTarget(&resource, resourceType) } if resource.MetricsTarget == nil { - resource.MetricsTarget = monitorMetricsTarget(resource, resourceType) + resource.MetricsTarget = monitorMetricsTarget(&resource, resourceType) } unifiedresources.RefreshCanonicalMetadata(&resource) slimResourceForBroadcast(&resource) - name, displayName := monitorFrontendNames(resource, resourceType) - platformID := monitorPlatformID(resource, resourceType) + name, displayName := monitorFrontendNames(&resource, resourceType) + platformID := monitorPlatformID(&resource, resourceType) input := models.ResourceConvertInput{ ID: resource.ID, @@ -6573,21 +6574,21 @@ func monitorResourceToConvertInput(resource unifiedresources.Resource) models.Re Name: name, DisplayName: displayName, PlatformID: platformID, - PlatformType: monitorPlatformType(resource, resourceType), + PlatformType: monitorPlatformType(&resource, resourceType), SourceType: monitorSourceType(resource.Sources), Sources: monitorSourceKeys(resource.Sources), ParentID: monitorStringValue(resource.ParentID), ParentName: resource.ParentName, ChildCount: resource.ChildCount, - ClusterID: monitorClusterID(resource), - Status: monitorFrontendStatus(resource, resourceType), + ClusterID: monitorClusterID(&resource), + Status: monitorFrontendStatus(&resource, resourceType), CPU: monitorMetricInput(monitorMetricValue(resource.Metrics, func(metrics *unifiedresources.ResourceMetrics) *unifiedresources.MetricValue { return metrics.CPU })), Memory: monitorMetricInput(monitorMetricValue(resource.Metrics, func(metrics *unifiedresources.ResourceMetrics) *unifiedresources.MetricValue { return metrics.Memory })), Disk: monitorMetricInput(monitorMetricValue(resource.Metrics, func(metrics *unifiedresources.ResourceMetrics) *unifiedresources.MetricValue { return metrics.Disk })), - Temperature: monitorTemperature(resource), - Uptime: monitorUptime(resource), + Temperature: monitorTemperature(&resource), + Uptime: monitorUptime(&resource), Tags: append([]string(nil), resource.Tags...), - Labels: monitorLabels(resource), + Labels: monitorLabels(&resource), CustomURL: strings.TrimSpace(resource.CustomURL), LastSeenUnix: monitorLastSeenUnix(resource.LastSeen), Health: monitorRawJSON(resource.Health), @@ -6601,7 +6602,7 @@ func monitorResourceToConvertInput(resource unifiedresources.Resource) models.Re IncidentImpactSummary: resource.IncidentImpactSummary, IncidentUrgency: resource.IncidentUrgency, IncidentAction: resource.IncidentAction, - Identity: monitorIdentity(resource, name), + Identity: monitorIdentity(&resource, name), DiscoveryTarget: monitorRawJSON(resource.DiscoveryTarget), MetricsTarget: monitorRawJSON(resource.MetricsTarget), Canonical: monitorRawJSON(resource.Canonical), @@ -6625,7 +6626,7 @@ func monitorResourceToConvertInput(resource unifiedresources.Resource) models.Re VMware: monitorRawJSON(resource.VMware), Availability: monitorRawJSON(resource.Availability), AvailabilityChecks: monitorRawJSON(resource.AvailabilityChecks), - PlatformData: monitorPlatformData(resource, resourceType, platformID), + PlatformData: monitorPlatformData(&resource, resourceType, platformID), } hasNetwork, rx, tx := monitorNetworkMetricInput(resource.Metrics) @@ -6641,6 +6642,25 @@ func monitorRawJSON(value interface{}) json.RawMessage { if value == nil { return nil } + // Optional facets arrive as typed nil pointers/slices in an interface. + // Marshaling "null" only to discard it costs one encode and allocation + // per absent facet, on every row and frame. Named slice/map marshalers + // still get their normal chance to emit a non-null representation. + v := reflect.ValueOf(value) + switch v.Kind() { + case reflect.Ptr: + if v.IsNil() { + return nil + } + case reflect.Slice, reflect.Map: + if v.IsNil() { + if _, custom := value.(json.Marshaler); !custom { + if _, customText := value.(encoding.TextMarshaler); !customText { + return nil + } + } + } + } encoded, err := json.Marshal(value) if err != nil || string(encoded) == "null" { return nil @@ -6708,7 +6728,7 @@ func capabilityCatalogID(encoded json.RawMessage) string { return hex.EncodeToString(digest[:6]) } -func monitorDiscoveryTarget(resource unifiedresources.Resource, resourceType string) *unifiedresources.DiscoveryTarget { +func monitorDiscoveryTarget(resource *unifiedresources.Resource, resourceType string) *unifiedresources.DiscoveryTarget { switch resourceType { case "agent", "docker-host": agentID := monitorAgentTargetID(resource) @@ -6726,7 +6746,7 @@ func monitorDiscoveryTarget(resource unifiedresources.Resource, resourceType str } } -func monitorMetricsTarget(resource unifiedresources.Resource, resourceType string) *unifiedresources.MetricsTarget { +func monitorMetricsTarget(resource *unifiedresources.Resource, resourceType string) *unifiedresources.MetricsTarget { switch resourceType { case "agent", "docker-host": agentID := monitorAgentTargetID(resource) @@ -6742,7 +6762,7 @@ func monitorMetricsTarget(resource unifiedresources.Resource, resourceType strin } } -func monitorAgentTargetID(resource unifiedresources.Resource) string { +func monitorAgentTargetID(resource *unifiedresources.Resource) string { if resource.Agent != nil { if id := strings.TrimSpace(resource.Agent.AgentID); id != "" { return id @@ -6764,7 +6784,7 @@ func monitorAgentTargetID(resource unifiedresources.Resource) string { return "" } -func monitorTargetHostname(resource unifiedresources.Resource) string { +func monitorTargetHostname(resource *unifiedresources.Resource) string { if resource.Agent != nil { if hostname := strings.TrimSpace(resource.Agent.Hostname); hostname != "" { return hostname @@ -6788,19 +6808,19 @@ func monitorTargetHostname(resource unifiedresources.Resource) string { return strings.TrimSpace(resource.Name) } -func monitorFrontendResourceType(resource unifiedresources.Resource) string { - return string(unifiedresources.ContractResourceType(resource)) +func monitorFrontendResourceType(resource *unifiedresources.Resource) string { + return string(unifiedresources.ContractResourceType(*resource)) } -func monitorFrontendNames(resource unifiedresources.Resource, resourceType string) (string, string) { - name := strings.TrimSpace(unifiedresources.ResourceDisplayName(resource)) +func monitorFrontendNames(resource *unifiedresources.Resource, resourceType string) (string, string) { + name := strings.TrimSpace(unifiedresources.ResourceDisplayName(*resource)) if name == "" { name = resource.ID } return name, name } -func monitorPlatformType(resource unifiedresources.Resource, resourceType string) string { +func monitorPlatformType(resource *unifiedresources.Resource, resourceType string) string { if resource.Proxmox != nil { return "proxmox-pve" } @@ -6901,7 +6921,7 @@ func monitorStoragePlatformType(storage *unifiedresources.StorageMeta, sources [ } } -func monitorPlatformID(resource unifiedresources.Resource, resourceType string) string { +func monitorPlatformID(resource *unifiedresources.Resource, resourceType string) string { switch resourceType { case "node", "vm", "system-container": if resource.Proxmox != nil && strings.TrimSpace(resource.Proxmox.Instance) != "" { @@ -6947,7 +6967,7 @@ func monitorPlatformID(resource unifiedresources.Resource, resourceType string) return resource.ID } -func monitorFrontendStatus(resource unifiedresources.Resource, resourceType string) string { +func monitorFrontendStatus(resource *unifiedresources.Resource, resourceType string) string { switch resourceType { case "app-container": switch resource.Status { @@ -7001,8 +7021,8 @@ func monitorIsWorkloadType(resourceType string) bool { } } -func monitorClusterID(resource unifiedresources.Resource) string { - if clusterID := strings.TrimSpace(unifiedresources.ResourceClusterName(resource)); clusterID != "" { +func monitorClusterID(resource *unifiedresources.Resource) string { + if clusterID := strings.TrimSpace(unifiedresources.ResourceClusterName(*resource)); clusterID != "" { return clusterID } @@ -7079,7 +7099,7 @@ func monitorDiskIOMetricInput(metrics *unifiedresources.ResourceMetrics) (*int64 return read, write } -func monitorTemperature(resource unifiedresources.Resource) *float64 { +func monitorTemperature(resource *unifiedresources.Resource) *float64 { if resource.Agent != nil && resource.Agent.Temperature != nil { value := *resource.Agent.Temperature return &value @@ -7099,7 +7119,7 @@ func monitorTemperature(resource unifiedresources.Resource) *float64 { return nil } -func monitorUptime(resource unifiedresources.Resource) *int64 { +func monitorUptime(resource *unifiedresources.Resource) *int64 { if resource.Agent != nil && resource.Agent.UptimeSeconds > 0 { value := resource.Agent.UptimeSeconds return &value @@ -7141,7 +7161,7 @@ func monitorUptime(resource unifiedresources.Resource) *int64 { return nil } -func monitorLabels(resource unifiedresources.Resource) map[string]string { +func monitorLabels(resource *unifiedresources.Resource) map[string]string { if resource.Kubernetes == nil || len(resource.Kubernetes.Labels) == 0 { return nil } @@ -7152,7 +7172,7 @@ func monitorLabels(resource unifiedresources.Resource) map[string]string { return labels } -func monitorIdentity(resource unifiedresources.Resource, fallbackName string) *models.ResourceIdentityInput { +func monitorIdentity(resource *unifiedresources.Resource, fallbackName string) *models.ResourceIdentityInput { hostname := "" if resource.Agent != nil { hostname = strings.TrimSpace(resource.Agent.Hostname) @@ -7199,7 +7219,7 @@ func monitorIdentity(resource unifiedresources.Resource, fallbackName string) *m } } -func monitorPlatformData(resource unifiedresources.Resource, resourceType string, platformID string) json.RawMessage { +func monitorPlatformData(resource *unifiedresources.Resource, resourceType string, platformID string) json.RawMessage { var payload interface{} switch resourceType { @@ -7416,7 +7436,7 @@ func monitorAttachSourceKeys(payload interface{}, sources []unifiedresources.Dat return payload } -func monitorStoragePlatformData(resource unifiedresources.Resource, platformID string) map[string]interface{} { +func monitorStoragePlatformData(resource *unifiedresources.Resource, platformID string) map[string]interface{} { nodeLabel := resource.ParentName if nodeLabel == "" { nodeLabel = monitorStringValue(resource.ParentID) @@ -7507,7 +7527,7 @@ func convertProxmoxDisks(disks []unifiedresources.DiskInfo) []map[string]interfa return out } -func buildProxmoxVMPayload(resource unifiedresources.Resource) map[string]interface{} { +func buildProxmoxVMPayload(resource *unifiedresources.Resource) map[string]interface{} { if resource.Proxmox == nil { return nil } diff --git a/internal/monitoring/monitor_conversion_helpers_additional_test.go b/internal/monitoring/monitor_conversion_helpers_additional_test.go index a8ec7e61db..495c50e25b 100644 --- a/internal/monitoring/monitor_conversion_helpers_additional_test.go +++ b/internal/monitoring/monitor_conversion_helpers_additional_test.go @@ -11,7 +11,7 @@ func ptrF64Coverage(v float64) *float64 { return &v } func TestMonitorFrontendNamesAdditional(t *testing.T) { t.Run("frontend names use canonical display name", func(t *testing.T) { - name, display := monitorFrontendNames(unifiedresources.Resource{ + name, display := monitorFrontendNames(&unifiedresources.Resource{ ID: "node-1", Name: "Node Friendly", Proxmox: &unifiedresources.ProxmoxData{ @@ -28,7 +28,7 @@ func TestMonitorFrontendNamesAdditional(t *testing.T) { }) t.Run("frontend names ignore source hostnames when a display name exists", func(t *testing.T) { - hostName, hostDisplay := monitorFrontendNames(unifiedresources.Resource{ + hostName, hostDisplay := monitorFrontendNames(&unifiedresources.Resource{ ID: "host-1", Name: "Friendly Host", Agent: &unifiedresources.AgentData{ @@ -39,7 +39,7 @@ func TestMonitorFrontendNamesAdditional(t *testing.T) { t.Fatalf("agent branch mismatch: name=%q display=%q", hostName, hostDisplay) } - dockerName, dockerDisplay := monitorFrontendNames(unifiedresources.Resource{ + dockerName, dockerDisplay := monitorFrontendNames(&unifiedresources.Resource{ ID: "docker-1", Name: "Friendly Docker", Docker: &unifiedresources.DockerData{ @@ -52,7 +52,7 @@ func TestMonitorFrontendNamesAdditional(t *testing.T) { }) t.Run("falls back to id when display name is empty", func(t *testing.T) { - name, display := monitorFrontendNames(unifiedresources.Resource{ + name, display := monitorFrontendNames(&unifiedresources.Resource{ ID: "fallback-id", Name: " ", }, "unknown") @@ -74,31 +74,31 @@ func TestMonitorTemperatureAndUptimeAdditional(t *testing.T) { Kubernetes: &unifiedresources.K8sData{Temperature: ptrF64Coverage(35)}, } - got := monitorTemperature(resource) + got := monitorTemperature(&resource) if got == nil || *got != 50 { t.Fatalf("temperature = %v, want 50 from agent", got) } resource.Agent = nil - got = monitorTemperature(resource) + got = monitorTemperature(&resource) if got == nil || *got != 45 { t.Fatalf("temperature = %v, want 45 from proxmox", got) } resource.Proxmox = nil - got = monitorTemperature(resource) + got = monitorTemperature(&resource) if got == nil || *got != 40 { t.Fatalf("temperature = %v, want 40 from docker", got) } resource.Docker = nil - got = monitorTemperature(resource) + got = monitorTemperature(&resource) if got == nil || *got != 35 { t.Fatalf("temperature = %v, want 35 from kubernetes", got) } resource.Kubernetes = nil - if got := monitorTemperature(resource); got != nil { + if got := monitorTemperature(&resource); got != nil { t.Fatalf("temperature = %v, want nil when no source has temp", got) } }) @@ -113,19 +113,19 @@ func TestMonitorTemperatureAndUptimeAdditional(t *testing.T) { PMG: &unifiedresources.PMGData{UptimeSeconds: 12}, } - got := monitorUptime(resource) + got := monitorUptime(&resource) if got == nil || *got != 11 { t.Fatalf("uptime = %v, want 11 from PBS", got) } resource.PBS.UptimeSeconds = 0 - got = monitorUptime(resource) + got = monitorUptime(&resource) if got == nil || *got != 12 { t.Fatalf("uptime = %v, want 12 from PMG", got) } resource.PMG.UptimeSeconds = 0 - if got := monitorUptime(resource); got != nil { + if got := monitorUptime(&resource); got != nil { t.Fatalf("uptime = %v, want nil when all sources are zero", got) } }) @@ -133,7 +133,7 @@ func TestMonitorTemperatureAndUptimeAdditional(t *testing.T) { func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { t.Run("labels are copied and nil is preserved", func(t *testing.T) { - if got := monitorLabels(unifiedresources.Resource{}); got != nil { + if got := monitorLabels(&unifiedresources.Resource{}); got != nil { t.Fatalf("monitorLabels() = %#v, want nil", got) } @@ -142,7 +142,7 @@ func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { Labels: map[string]string{"env": "prod"}, }, } - labels := monitorLabels(resource) + labels := monitorLabels(&resource) labels["env"] = "dev" if resource.Kubernetes.Labels["env"] != "prod" { t.Fatalf("expected labels to be copied, source mutated to %q", resource.Kubernetes.Labels["env"]) @@ -157,7 +157,7 @@ func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { IPAddresses: []string{" 10.0.0.1 ", "", "10.0.0.2"}, }, } - identity := monitorIdentity(resource, "fallback-name") + identity := monitorIdentity(&resource, "fallback-name") if identity == nil { t.Fatal("expected non-nil identity") } @@ -171,7 +171,7 @@ func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { t.Fatalf("ips = %#v, want trimmed non-empty entries", identity.IPs) } - none := monitorIdentity(unifiedresources.Resource{}, "") + none := monitorIdentity(&unifiedresources.Resource{}, "") if none != nil { t.Fatalf("expected nil identity for empty input, got %#v", none) } @@ -204,7 +204,7 @@ func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { }) t.Run("identity prefers direct source hostnames", func(t *testing.T) { - identity := monitorIdentity(unifiedresources.Resource{ + identity := monitorIdentity(&unifiedresources.Resource{ Agent: &unifiedresources.AgentData{Hostname: "agent-host"}, Docker: &unifiedresources.DockerData{Hostname: "docker-host"}, Proxmox: &unifiedresources.ProxmoxData{NodeName: "proxmox-node"}, diff --git a/internal/unifiedresources/canonical_identity.go b/internal/unifiedresources/canonical_identity.go index bc89842669..838def2176 100644 --- a/internal/unifiedresources/canonical_identity.go +++ b/internal/unifiedresources/canonical_identity.go @@ -387,18 +387,47 @@ func firstTrimmed(values ...string) string { } func uniqueTrimmed(values ...string) []string { - seen := make(map[string]struct{}, len(values)) - aliases := make([]string, 0, len(values)) + // Canonical aliases usually contain only a handful of non-empty unique + // identities among many optional inputs. Do not allocate a map sized for + // every placeholder, or retain that capacity in each row's alias slice. + // A bounded stack scan handles the common case; larger vocabularies use + // the same lowercased-key map semantics without quadratic growth. + var smallKeys [8]string + var seen map[string]struct{} + aliases := make([]string, 0, min(len(values), len(smallKeys))) for _, value := range values { trimmed := strings.TrimSpace(value) if trimmed == "" { continue } key := strings.ToLower(trimmed) - if _, ok := seen[key]; ok { - continue + if seen != nil { + if _, ok := seen[key]; ok { + continue + } + } else { + duplicate := false + for _, previous := range smallKeys[:len(aliases)] { + if previous == key { + duplicate = true + break + } + } + if duplicate { + continue + } + if len(aliases) < len(smallKeys) { + smallKeys[len(aliases)] = key + } else { + seen = make(map[string]struct{}, min(len(values), 64)) + for _, previous := range smallKeys { + seen[previous] = struct{}{} + } + } + } + if seen != nil { + seen[key] = struct{}{} } - seen[key] = struct{}{} aliases = append(aliases, trimmed) } return aliases diff --git a/internal/unifiedresources/canonical_identity_allocation_test.go b/internal/unifiedresources/canonical_identity_allocation_test.go new file mode 100644 index 0000000000..15be6207be --- /dev/null +++ b/internal/unifiedresources/canonical_identity_allocation_test.go @@ -0,0 +1,85 @@ +package unifiedresources + +import ( + "fmt" + "math/rand" + "reflect" + "strings" + "testing" +) + +// Independent original implementation. Lowercasing is intentionally not +// EqualFold: those operations differ for some Unicode identities. +func uniqueTrimmedMapReference(values ...string) []string { + seen := make(map[string]struct{}, len(values)) + aliases := make([]string, 0, len(values)) + for _, value := range values { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + continue + } + key := strings.ToLower(trimmed) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + aliases = append(aliases, trimmed) + } + return aliases +} +func TestCanonicalAliasesBoundedScanMatchesOriginalMap(t *testing.T) { + cases := [][]string{nil, {}, {"", " ", "\t"}, {" Host ", "host", "HOST", "other"}, {"K", "K", "k", "İ", "i", "Σ", "σ", "ς", "ſ", "S", "s"}} + for _, count := range []int{7, 8, 9, 16, 128, 1024} { + var values []string + for i := 0; i < count; i++ { + value := fmt.Sprintf("Alias-%d", i) + values = append(values, " ", value, strings.ToLower(value), " "+value+" ") + } + cases = append(cases, values) + } + rng := rand.New(rand.NewSource(2199)) + alphabet := []string{"", " ", "Host", "host", "HOST", "agent:lab", " vm:100 ", "K", "K", "İ", "i", "Σ", "σ", "ς", "ſ", "S"} + for i := 0; i < 200; i++ { + values := make([]string, rng.Intn(80)) + for j := range values { + values[j] = alphabet[rng.Intn(len(alphabet))] + } + cases = append(cases, values) + } + for i, values := range cases { + got, want := uniqueTrimmed(values...), uniqueTrimmedMapReference(values...) + if !reflect.DeepEqual(got, want) { + t.Fatalf("case %d: got %q, want %q", i, got, want) + } + if len(values) > 0 && len(got) > 0 { + original := values[0] + got[0] = "caller-edit" + if values[0] != original { + t.Fatal("aliases share caller storage") + } + } + } +} +func TestCanonicalAliasesSmallVocabularyAvoidsPlaceholderMap(t *testing.T) { + values := []string{"lab", "", "", "LAB", "agent:lab", "", "", "agent:lab", "lab.example", "", "", "", "", "lab.example", "", ""} + optimized := testing.AllocsPerRun(100, func() { _ = uniqueTrimmed(values...) }) + original := testing.AllocsPerRun(100, func() { _ = uniqueTrimmedMapReference(values...) }) + if optimized >= original { + t.Fatalf("common alias list allocations %.0f, original %.0f", optimized, original) + } + t.Logf("same alias vocabulary: allocations %.0f -> %.0f", original, optimized) +} +func BenchmarkCanonicalAliasesVocabulary(b *testing.B) { + for _, size := range []int{4, 8, 9, 128} { + values := make([]string, 0, size*3) + for i := 0; i < size; i++ { + values = append(values, fmt.Sprintf("alias-%d", i), "", fmt.Sprintf("ALIAS-%d", i)) + } + b.Run(fmt.Sprint(size), func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + _ = uniqueTrimmed(values...) + } + }) + } +} diff --git a/internal/unifiedresources/code_standards_test.go b/internal/unifiedresources/code_standards_test.go index f9c85fa78c..6bf168aa27 100644 --- a/internal/unifiedresources/code_standards_test.go +++ b/internal/unifiedresources/code_standards_test.go @@ -2695,9 +2695,9 @@ func TestBroadcastStateUsesSharedCanonicalResourceContract(t *testing.T) { requiredMonitorSnippets := []string{ "unifiedView := m.currentUnifiedStateView()", - "return string(unifiedresources.ContractResourceType(resource))", - "unifiedresources.ResourceDisplayName(resource)", - "unifiedresources.ResourceClusterName(resource)", + "return string(unifiedresources.ContractResourceType(*resource))", + "unifiedresources.ResourceDisplayName(*resource)", + "unifiedresources.ResourceClusterName(*resource)", } for _, snippet := range requiredMonitorSnippets { if !strings.Contains(string(monitorSource), snippet) {