From 24329e278e244714ac9a86d7fdb21665e685b9b3 Mon Sep 17 00:00:00 2001 From: Prototype0645 Date: Mon, 31 Aug 2026 12:06:40 +0200 Subject: [PATCH] feat(wan): label WAN metrics with their site and source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every unpoller_wan_* series shipped with site_name="" and source="". The exporter said so itself: cfg.WANLoadBalanceType, "", // site_name - will be set by caller if available "", // source - will be set by caller if available The caller had nothing to set them from: WANEnrichedConfiguration carried no identity. unifi/v6.0.3 fixes that upstream — GetWANEnrichedConfiguration now stamps SiteName and SourceName from the site it fetched, the same way GetSiteDPI does. This bumps to v6.0.3 and fills the labels in. Two slices needed it, not one: the base label set and the provider label set built further down for the isp_name/isp_city descriptors. The test caught the second, which I had missed. Why it matters: an instance polling several controllers emitted WAN metrics that were indistinguishable from one another, since wan_id is the only other distinguishing label. Attributing them downstream meant hardcoding a mapping in the scrape config and hoping no second controller ever gained a gateway — when one does, its metrics are silently filed under the wrong customer. No error, no missing series, just wrong data. Tests use the fakeReport already present in the package. They assert every emitted metric carries both labels, and that a nil configuration still produces nothing rather than panicking a poll. TestExportWANIsAttributed fails on master and passes with this change. --- go.mod | 2 +- go.sum | 4 +-- pkg/promunifi/wan.go | 8 ++--- pkg/promunifi/wan_test.go | 69 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 pkg/promunifi/wan_test.go diff --git a/go.mod b/go.mod index f31c04b8..2fb92ae3 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/prometheus/common v0.70.1 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.12.1 - github.com/unpoller/unifi/v6 v6.0.2 + github.com/unpoller/unifi/v6 v6.0.3 go.opentelemetry.io/otel v1.46.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.46.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.46.0 diff --git a/go.sum b/go.sum index 17e75164..8291259e 100644 --- a/go.sum +++ b/go.sum @@ -121,8 +121,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= -github.com/unpoller/unifi/v6 v6.0.2 h1:Mzcn0zSTnFxMZuFIoVqhSkECT1sX9S7RJ8e/ZxikOcw= -github.com/unpoller/unifi/v6 v6.0.2/go.mod h1:d7dz1cBxVbbFZobNRcdUHHYtTdicVyZ05J5OmgoINa8= +github.com/unpoller/unifi/v6 v6.0.3 h1:NoxbSA5HLMErs/1yVkmuwm+lcyy9SxSfOLDW58PKqrQ= +github.com/unpoller/unifi/v6 v6.0.3/go.mod h1:d7dz1cBxVbbFZobNRcdUHHYtTdicVyZ05J5OmgoINa8= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= diff --git a/pkg/promunifi/wan.go b/pkg/promunifi/wan.go index e40a12b2..4ffd15db 100644 --- a/pkg/promunifi/wan.go +++ b/pkg/promunifi/wan.go @@ -87,8 +87,8 @@ func (u *promUnifi) exportWAN(r report, w *unifi.WANEnrichedConfiguration) { cfg.WANNetworkgroup, cfg.WANType, cfg.WANLoadBalanceType, - "", // site_name - will be set by caller if available - "", // source - will be set by caller if available + w.SiteName, + w.SourceName, } // Convert boolean FlexBool values to float64 @@ -130,8 +130,8 @@ func (u *promUnifi) exportWAN(r report, w *unifi.WANEnrichedConfiguration) { cfg.WANNetworkgroup, details.ServiceProvider.Name, details.ServiceProvider.City, - "", // site_name - "", // source + w.SiteName, + w.SourceName, } metrics = append(metrics, &metric{u.WAN.ServiceProviderASN, gauge, details.ServiceProvider.ASN.Val, providerLabels}) diff --git a/pkg/promunifi/wan_test.go b/pkg/promunifi/wan_test.go new file mode 100644 index 00000000..acf67747 --- /dev/null +++ b/pkg/promunifi/wan_test.go @@ -0,0 +1,69 @@ +//nolint:testpackage // white-box: exercises the unexported descriptors and export path. +package promunifi + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/unpoller/unifi/v6" +) + +func testWANConfig() *unifi.WANEnrichedConfiguration { + return &unifi.WANEnrichedConfiguration{ + SiteName: "Default (default)", + SourceName: "https://udr.example", + Configuration: unifi.WANConfiguration{ + ID: "a1", + Name: "Internet 1", + WANNetworkgroup: "WAN", + WANType: "dhcp", + WANLoadBalanceType: "weighted", + }, + Details: unifi.WANDetails{ + ServiceProvider: unifi.WANServiceProvider{ + Name: "SpaceX Starlink", + City: "Brussels", + }, + }, + } +} + +// TestExportWANIsAttributed is the point of this change. Every unpoller_wan_* +// series used to ship with empty site_name and source, which makes the metrics +// of two controllers polled by the same instance indistinguishable. The only +// way to attribute them downstream was to hardcode a mapping in the scrape +// config and hope no second controller ever gained a gateway. +func TestExportWANIsAttributed(t *testing.T) { + t.Parallel() + + r := &fakeReport{} + u := &promUnifi{WAN: descWAN("unifi_")} + u.exportWAN(r, testWANConfig()) + + require.NotEmpty(t, r.sent, "a WAN configuration must produce metrics") + + a := assert.New(t) + + for _, m := range r.sent { + require.GreaterOrEqual(t, len(m.Labels), 7, "every WAN metric carries the base label set") + // site_name and source are the last two of the base label set; the + // provider descriptors substitute isp_name/isp_city earlier but keep + // the same trailing pair. + a.Equal("Default (default)", m.Labels[len(m.Labels)-2], "site_name must be populated") + a.Equal("https://udr.example", m.Labels[len(m.Labels)-1], "source must be populated") + a.NotContains(m.Labels[:len(m.Labels)-2], "", "no other label should be blank in this fixture") + } +} + +// TestExportWANNilIsSafe pins the existing guard: the collector iterates over +// whatever the input plugin produced, and a nil entry must not panic a poll. +func TestExportWANNilIsSafe(t *testing.T) { + t.Parallel() + + r := &fakeReport{} + u := &promUnifi{WAN: descWAN("unifi_")} + u.exportWAN(r, nil) + + assert.Empty(t, r.sent, "a nil WAN configuration produces nothing and does not panic") +}