From 87359d0d75a9236c67de194f0cc91e06db4ae31b Mon Sep 17 00:00:00 2001 From: Chris Grindstaff Date: Wed, 6 Nov 2024 05:08:06 -0500 Subject: [PATCH] feat: doctor should handle embedded exporters (#3258) --- cmd/tools/doctor/doctor.go | 49 ++++++++++++++++++++++-- cmd/tools/doctor/doctor_test.go | 2 +- cmd/tools/doctor/testdata/testConfig.yml | 10 +++++ integration/go.mod | 4 +- integration/go.sum | 8 ++-- pkg/color/color.go | 4 +- pkg/conf/conf.go | 16 ++++---- pkg/tree/tree_test.go | 2 +- 8 files changed, 74 insertions(+), 21 deletions(-) diff --git a/cmd/tools/doctor/doctor.go b/cmd/tools/doctor/doctor.go index 1390a5e09..ff0fdc1ef 100644 --- a/cmd/tools/doctor/doctor.go +++ b/cmd/tools/doctor/doctor.go @@ -422,8 +422,8 @@ func checkExportersExist(config conf.HarvestConfig) validation { return valid } -// checkUniquePromPorts checks that all Prometheus exporters -// which specify a port, do so uniquely +// checkUniquePromPorts checks that all Prometheus exporters, which specify a port, do so uniquely. +// Embedded exporters can conflict with non-embedded exporters, but not with each other. func checkUniquePromPorts(config conf.HarvestConfig) validation { if config.Exporters == nil { return validation{} @@ -431,11 +431,18 @@ func checkUniquePromPorts(config conf.HarvestConfig) validation { // Add all exporters that have a port to a // map of portNum -> list of names seen := make(map[int][]string) + var embeddedExporters []conf.ExporterDef + for name, exporter := range config.Exporters { // ignore configuration with both port and portrange defined. PortRange takes precedence if exporter.Port == nil || exporter.Type != "Prometheus" || exporter.PortRange != nil { continue } + // Ignore embedded exporters + if exporter.IsEmbedded { + embeddedExporters = append(embeddedExporters, conf.ExporterDef{Exporter: exporter, Name: name}) + continue + } previous := seen[*exporter.Port] previous = append(previous, name) seen[*exporter.Port] = previous @@ -462,10 +469,26 @@ func checkUniquePromPorts(config conf.HarvestConfig) validation { continue } valid.isValid = false - valid.invalid = append(valid.invalid, exporterNames...) break } + // Check that embedded exports do not conflict with each other + embeddedPorts := make(map[int][]string) + for _, embeddedExporter := range embeddedExporters { + // Check if the embedded exporter has a port + if embeddedExporter.Port == nil { + continue + } + // Check if the port is unique + previous := embeddedPorts[*embeddedExporter.Port] + previous = append(previous, embeddedExporter.Name) + embeddedPorts[*embeddedExporter.Port] = previous + if len(previous) > 1 { + valid.isValid = false + break + } + } + if !valid.isValid { fmt.Printf("%s: Exporter PromPort conflict\n", color.Colorize("Error", color.Red)) fmt.Println(" Prometheus exporters must specify unique ports. Change the following exporters to use unique ports:") @@ -474,7 +497,25 @@ func checkUniquePromPorts(config conf.HarvestConfig) validation { continue } names := strings.Join(exporterNames, ", ") - fmt.Printf(" port: [%s] duplicateExporters: [%s]\n", color.Colorize(port, color.Red), color.Colorize(names, color.Yellow)) + valid.invalid = append(valid.invalid, exporterNames...) + fmt.Printf(" port: [%s] duplicate exporters: [%s]\n", color.Colorize(port, color.Red), color.Colorize(names, color.Yellow)) + } + for port, exporterNames := range embeddedPorts { + if len(exporterNames) == 1 { + continue + } + pollerNames := make([]string, 0, len(exporterNames)) + for _, exporterName := range exporterNames { + index := strings.LastIndex(exporterName, "-") + if index == -1 { + pollerNames = append(pollerNames, exporterName) + continue + } + pollerNames = append(pollerNames, exporterName[:index]) + } + names := strings.Join(pollerNames, ", ") + valid.invalid = append(valid.invalid, exporterNames...) + fmt.Printf(" port: [%s] duplicate embedded exporters for pollers: [%s]\n", color.Colorize(port, color.Red), color.Colorize(names, color.Yellow)) } fmt.Println() } diff --git a/cmd/tools/doctor/doctor_test.go b/cmd/tools/doctor/doctor_test.go index 232ef8181..b10033628 100644 --- a/cmd/tools/doctor/doctor_test.go +++ b/cmd/tools/doctor/doctor_test.go @@ -118,7 +118,7 @@ func TestUniquePromPorts(t *testing.T) { if valid.isValid { t.Fatal(`expected isValid to be false since there are duplicate prom ports, actual was isValid=true`) } - if len(valid.invalid) != 2 { + if len(valid.invalid) != 4 { t.Fatalf(`expected checkUniquePromPorts to return 2 invalid results, actual was %s`, valid.invalid) } } diff --git a/cmd/tools/doctor/testdata/testConfig.yml b/cmd/tools/doctor/testdata/testConfig.yml index e67dd27e2..d2dc8f5c7 100644 --- a/cmd/tools/doctor/testdata/testConfig.yml +++ b/cmd/tools/doctor/testdata/testConfig.yml @@ -170,4 +170,14 @@ Pollers: username: admin password: '#pass' # you can use single or double quotes to escape # + poller-with-embedded-exporter: + exporters: + - exporter: Prometheus + port: 2000 + + poller-with-embedded-exporter2: + exporters: + - exporter: Prometheus + port: 2000 + ll: grafana_api_token grafana_api_token diff --git a/integration/go.mod b/integration/go.mod index 55d1b3c2c..17837796c 100644 --- a/integration/go.mod +++ b/integration/go.mod @@ -14,7 +14,7 @@ require ( ) require ( - github.com/ebitengine/purego v0.8.0 // indirect + github.com/ebitengine/purego v0.8.1 // indirect github.com/go-ole/go-ole v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect @@ -26,7 +26,7 @@ require ( github.com/mailru/easyjson v0.7.7 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect github.com/rivo/uniseg v0.4.7 // indirect - github.com/shirou/gopsutil/v4 v4.24.9 // indirect + github.com/shirou/gopsutil/v4 v4.24.10 // indirect github.com/spf13/cobra v1.8.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/tidwall/match v1.1.1 // indirect diff --git a/integration/go.sum b/integration/go.sum index 4fd396742..cc1104683 100644 --- a/integration/go.sum +++ b/integration/go.sum @@ -3,8 +3,8 @@ github.com/carlmjohnson/requests v0.24.2/go.mod h1:duYA/jDnyZ6f3xbcF5PpZ9N8clgop github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/ebitengine/purego v0.8.0 h1:JbqvnEzRvPpxhCJzJJ2y0RbiZ8nyjccVUrSM3q+GvvE= -github.com/ebitengine/purego v0.8.0/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= +github.com/ebitengine/purego v0.8.1 h1:sdRKd6plj7KYW33EH5As6YKfe8m9zbN9JMrOjNVF/BE= +github.com/ebitengine/purego v0.8.1/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= @@ -39,8 +39,8 @@ github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUc github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/shirou/gopsutil/v4 v4.24.9 h1:KIV+/HaHD5ka5f570RZq+2SaeFsb/pq+fp2DGNWYoOI= -github.com/shirou/gopsutil/v4 v4.24.9/go.mod h1:3fkaHNeYsUFCGZ8+9vZVWtbyM1k2eRnlL+bWO8Bxa/Q= +github.com/shirou/gopsutil/v4 v4.24.10 h1:7VOzPtfw/5YDU+jLEoBwXwxJbQetULywoSV4RYY7HkM= +github.com/shirou/gopsutil/v4 v4.24.10/go.mod h1:s4D/wg+ag4rG0WO7AiTj2BeYCRhym0vM7DHbZRxnIT8= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= diff --git a/pkg/color/color.go b/pkg/color/color.go index cf8bec432..39db8f7b4 100644 --- a/pkg/color/color.go +++ b/pkg/color/color.go @@ -36,9 +36,9 @@ func DetectConsole(option string) { } } -func Colorize(s interface{}, color string) string { +func Colorize(s any, color string) string { if withColor { return fmt.Sprintf("%s%v\x1b[0m", color, s) } - return fmt.Sprintf("%s", s) + return fmt.Sprintf("%v", s) } diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go index f9450ecb0..cfa6f86b3 100644 --- a/pkg/conf/conf.go +++ b/pkg/conf/conf.go @@ -152,9 +152,10 @@ func fixupExporters() { for _, pollerName := range Config.PollersOrdered { poller := Config.Pollers[pollerName] for i, e := range poller.ExporterDefs { - exporterName := e.name + exporterName := e.Name if exporterName == "" { // This is an embedded exporter, synthesize a name for it + e.Exporter.IsEmbedded = true exporterName = fmt.Sprintf("%s-%d", pollerName, i) Config.Exporters[exporterName] = e.Exporter } @@ -498,8 +499,8 @@ type CertificateScript struct { Timeout string `yaml:"timeout,omitempty"` } -type ExportDef struct { - name string +type ExporterDef struct { + Name string Exporter } @@ -509,7 +510,7 @@ type Recorder struct { KeepLast string `yaml:"keep_last,omitempty"` // number of records to keep before overwriting } -func (e *ExportDef) UnmarshalYAML(n *yaml.Node) error { +func (e *ExporterDef) UnmarshalYAML(n *yaml.Node) error { if n.Kind == yaml.MappingNode { var aExporter *Exporter err := n.Decode(&aExporter) @@ -518,7 +519,7 @@ func (e *ExportDef) UnmarshalYAML(n *yaml.Node) error { } e.Exporter = *aExporter } else if n.Kind == yaml.ScalarNode && n.ShortTag() == "!!str" { - e.name = n.Value + e.Name = n.Value } return nil } @@ -536,7 +537,7 @@ type Poller struct { CredentialsFile string `yaml:"credentials_file,omitempty"` CredentialsScript CredentialsScript `yaml:"credentials_script,omitempty"` Datacenter string `yaml:"datacenter,omitempty"` - ExporterDefs []ExportDef `yaml:"exporters,omitempty"` + ExporterDefs []ExporterDef `yaml:"exporters,omitempty"` Exporters []string `yaml:"-"` IsKfs bool `yaml:"is_kfs,omitempty"` Labels *[]map[string]string `yaml:"labels,omitempty"` @@ -698,7 +699,8 @@ type Exporter struct { ClientTimeout *string `yaml:"client_timeout,omitempty"` Version *string `yaml:"version,omitempty"` - IsTest bool // true when run from unit tests + IsTest bool // true when run from unit tests + IsEmbedded bool // true when the exporter is embedded in a poller } type Pollers struct { diff --git a/pkg/tree/tree_test.go b/pkg/tree/tree_test.go index b00eefa52..b14fe18f6 100644 --- a/pkg/tree/tree_test.go +++ b/pkg/tree/tree_test.go @@ -164,7 +164,7 @@ func TestHarvestConfigImportYaml(t *testing.T) { } } - want = 12 + want = 14 got = 0 if pollers := template.GetChildS("Pollers"); pollers != nil { for range pollers.GetChildren() {