From 8b67d7d43b67802fbeb61c525ac15e64e5009000 Mon Sep 17 00:00:00 2001 From: VURU Date: Thu, 29 Dec 2022 13:26:12 +0100 Subject: [PATCH 1/5] rebase --- collector/collector.go | 50 +++++++++++++++++++++++++++++++++++++++ exporter/exporter.go | 53 +++++++++++++++++++++--------------------- main.go | 8 +++++++ 3 files changed, 85 insertions(+), 26 deletions(-) diff --git a/collector/collector.go b/collector/collector.go index 4895f30..7c9b279 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -171,6 +171,56 @@ func GetServerIDFromVarz(endpoint string, retryInterval time.Duration) string { return id } +func GetServerNameFromVarz(endpoint string, retryInterval time.Duration) string { + getServerName := func() (string, error) { + resp, err := http.DefaultClient.Get(endpoint + "/varz") + if err != nil { + return "", err + } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + var response map[string]interface{} + err = json.Unmarshal(body, &response) + if err != nil { + return "", err + } + serverName, ok := response["server_name"] + if !ok { + Fatalf("Could not find server name in /varz") + } + id, ok := serverName.(string) + if !ok { + Fatalf("Invalid server_name type in /varz: %+v", serverName) + } + + return id, nil + } + + var id string + var err error + id, err = getServerName() + if err == nil { + return id + } + + // Retry periodically until available, in case it never starts + // then a liveness check against the NATS Server itself should + // detect that an restart the server, in terms of the exporter + // we just wait for it to eventually be available. + for range time.NewTicker(retryInterval).C { + id, err = getServerName() + if err != nil { + Errorf("Could not find server id: %s", err) + continue + } + break + } + return id +} + // Describe the metric to the Prometheus server. func (nc *NATSCollector) Describe(ch chan<- *prometheus.Desc) { nc.Lock() diff --git a/exporter/exporter.go b/exporter/exporter.go index 2a475f0..acb6a0c 100644 --- a/exporter/exporter.go +++ b/exporter/exporter.go @@ -41,31 +41,32 @@ const ( // NATSExporterOptions are options to configure the NATS collector type NATSExporterOptions struct { collector.LoggerOptions - ListenAddress string - ListenPort int - ScrapePath string - GetHealthz bool - GetConnz bool - GetConnzDetailed bool - GetVarz bool - GetSubz bool - GetRoutez bool - GetGatewayz bool - GetLeafz bool - GetReplicatorVarz bool - GetStreamingChannelz bool - GetStreamingServerz bool - GetJszFilter string - RetryInterval time.Duration - CertFile string - KeyFile string - CaFile string - NATSServerURL string - NATSServerTag string - HTTPUser string // User in metrics scrape by prometheus. - HTTPPassword string - Prefix string - UseInternalServerID bool + ListenAddress string + ListenPort int + ScrapePath string + GetHealthz bool + GetConnz bool + GetConnzDetailed bool + GetVarz bool + GetSubz bool + GetRoutez bool + GetGatewayz bool + GetLeafz bool + GetReplicatorVarz bool + GetStreamingChannelz bool + GetStreamingServerz bool + GetJszFilter string + RetryInterval time.Duration + CertFile string + KeyFile string + CaFile string + NATSServerURL string + NATSServerTag string + HTTPUser string // User in metrics scrape by prometheus. + HTTPPassword string + Prefix string + UseInternalServerID bool + UseInternalServerName bool } // NATSExporter collects NATS metrics @@ -175,7 +176,7 @@ func (ne *NATSExporter) InitializeCollectors() error { } getJsz := opts.GetJszFilter != "" - if !opts.GetHealthz && !opts.GetConnz && !opts.GetConnzDetailed && !opts.GetRoutez && + if !opts.GetConnz && !opts.GetRoutez && !opts.GetSubz && !opts.GetVarz && !opts.GetGatewayz && !opts.GetLeafz && !opts.GetStreamingChannelz && !opts.GetStreamingServerz && !opts.GetReplicatorVarz && !getJsz { return fmt.Errorf("no Collectors specfied") diff --git a/main.go b/main.go index a6138c3..4c5254e 100644 --- a/main.go +++ b/main.go @@ -130,6 +130,7 @@ func main() { flag.StringVar(&opts.HTTPPassword, "http_pass", "", "Set the password for HTTP scrapes. NATS bcrypt supported.") flag.StringVar(&opts.Prefix, "prefix", "", "Replace the default prefix for all the metrics.") flag.BoolVar(&opts.UseInternalServerID, "use_internal_server_id", false, "Enables using ServerID from /varz") + flag.BoolVar(&opts.UseInternalServerName, "use_internal_server_name", false, "Enables using ServerName from /varz") flag.Parse() opts.RetryInterval = time.Duration(retryInterval) * time.Second @@ -165,6 +166,13 @@ necessary.`) if err := exp.AddServer(id, url); err != nil { collector.Fatalf("Unable to setup server in exporter: %s, %s: %v", id, url, err) } + } else if len(args) == 1 && opts.UseInternalServerID { + // Pick the server id from the /varz endpoint info. + url := flag.Args()[0] + id := collector.GetServerNameFromVarz(url, opts.RetryInterval) + if err := exp.AddServer(id, url); err != nil { + collector.Fatalf("Unable to setup server in exporter: %s, %s: %v", id, url, err) + } } else { // For each URL specified, add the NATS server with the optional ID. for _, arg := range args { From e78548167cf5d8912b03729648960b5ecaeb06b6 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 6 Mar 2023 20:27:25 +0100 Subject: [PATCH 2/5] fix review comments --- collector/collector.go | 2 +- exporter/exporter.go | 52 +++++++++++++++++++++--------------------- main.go | 4 ++-- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/collector/collector.go b/collector/collector.go index 7c9b279..38bcfdf 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -178,7 +178,7 @@ func GetServerNameFromVarz(endpoint string, retryInterval time.Duration) string return "", err } defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { return "", err } diff --git a/exporter/exporter.go b/exporter/exporter.go index acb6a0c..1f19766 100644 --- a/exporter/exporter.go +++ b/exporter/exporter.go @@ -41,32 +41,32 @@ const ( // NATSExporterOptions are options to configure the NATS collector type NATSExporterOptions struct { collector.LoggerOptions - ListenAddress string - ListenPort int - ScrapePath string - GetHealthz bool - GetConnz bool - GetConnzDetailed bool - GetVarz bool - GetSubz bool - GetRoutez bool - GetGatewayz bool - GetLeafz bool - GetReplicatorVarz bool - GetStreamingChannelz bool - GetStreamingServerz bool - GetJszFilter string - RetryInterval time.Duration - CertFile string - KeyFile string - CaFile string - NATSServerURL string - NATSServerTag string - HTTPUser string // User in metrics scrape by prometheus. - HTTPPassword string - Prefix string - UseInternalServerID bool - UseInternalServerName bool + ListenAddress string + ListenPort int + ScrapePath string + GetHealthz bool + GetConnz bool + GetConnzDetailed bool + GetVarz bool + GetSubz bool + GetRoutez bool + GetGatewayz bool + GetLeafz bool + GetReplicatorVarz bool + GetStreamingChannelz bool + GetStreamingServerz bool + GetJszFilter string + RetryInterval time.Duration + CertFile string + KeyFile string + CaFile string + NATSServerURL string + NATSServerTag string + HTTPUser string // User in metrics scrape by prometheus. + HTTPPassword string + Prefix string + UseInternalServerID bool + UseServerName bool } // NATSExporter collects NATS metrics diff --git a/main.go b/main.go index 4c5254e..8b75b75 100644 --- a/main.go +++ b/main.go @@ -130,7 +130,7 @@ func main() { flag.StringVar(&opts.HTTPPassword, "http_pass", "", "Set the password for HTTP scrapes. NATS bcrypt supported.") flag.StringVar(&opts.Prefix, "prefix", "", "Replace the default prefix for all the metrics.") flag.BoolVar(&opts.UseInternalServerID, "use_internal_server_id", false, "Enables using ServerID from /varz") - flag.BoolVar(&opts.UseInternalServerName, "use_internal_server_name", false, "Enables using ServerName from /varz") + flag.BoolVar(&opts.UseServerName, "use_internal_server_name", false, "Enables using ServerName from /varz") flag.Parse() opts.RetryInterval = time.Duration(retryInterval) * time.Second @@ -166,7 +166,7 @@ necessary.`) if err := exp.AddServer(id, url); err != nil { collector.Fatalf("Unable to setup server in exporter: %s, %s: %v", id, url, err) } - } else if len(args) == 1 && opts.UseInternalServerID { + } else if len(args) == 1 && opts.UseServerName { // Pick the server id from the /varz endpoint info. url := flag.Args()[0] id := collector.GetServerNameFromVarz(url, opts.RetryInterval) From d613b2e163845f2f15703212e0ba039a7e425310 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 6 Mar 2023 20:47:26 +0100 Subject: [PATCH 3/5] fix linter violations --- collector/collector.go | 79 ++++++++++-------------------------------- main.go | 11 +++--- 2 files changed, 26 insertions(+), 64 deletions(-) diff --git a/collector/collector.go b/collector/collector.go index 38bcfdf..c269676 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -122,57 +122,20 @@ func getMetricURL(httpClient *http.Client, url string, response interface{}) err // GetServerIDFromVarz gets the server ID from the server. func GetServerIDFromVarz(endpoint string, retryInterval time.Duration) string { - getServerID := func() (string, error) { - resp, err := http.DefaultClient.Get(endpoint + "/varz") - if err != nil { - return "", err - } - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", err - } - var response map[string]interface{} - err = json.Unmarshal(body, &response) - if err != nil { - return "", err - } - serverID, ok := response["server_id"] - if !ok { - Fatalf("Could not find server id in /varz") - } - id, ok := serverID.(string) - if !ok { - Fatalf("Invalid server_id type in /varz: %+v", serverID) - } - - return id, nil - } + return getServerKeyFromVarz(endpoint, retryInterval, "server_id") +} - var id string - var err error - id, err = getServerID() - if err == nil { - return id - } +// GetServerNameFromVarz gets the server name from the server. +func GetServerNameFromVarz(endpoint string, retryInterval time.Duration) string { + return getServerKeyFromVarz(endpoint, retryInterval, "server_name") +} +func getServerKeyFromVarz(endpoint string, retryInterval time.Duration, key string) string { // Retry periodically until available, in case it never starts // then a liveness check against the NATS Server itself should // detect that an restart the server, in terms of the exporter // we just wait for it to eventually be available. - for range time.NewTicker(retryInterval).C { - id, err = getServerID() - if err != nil { - Errorf("Could not find server id: %s", err) - continue - } - break - } - return id -} - -func GetServerNameFromVarz(endpoint string, retryInterval time.Duration) string { - getServerName := func() (string, error) { + getServerVarzValue := func() (string, error) { resp, err := http.DefaultClient.Get(endpoint + "/varz") if err != nil { return "", err @@ -187,38 +150,34 @@ func GetServerNameFromVarz(endpoint string, retryInterval time.Duration) string if err != nil { return "", err } - serverName, ok := response["server_name"] + serverVarzValue, ok := response[key] if !ok { - Fatalf("Could not find server name in /varz") + Fatalf("Could not find %s in /varz", key) } - id, ok := serverName.(string) + varzValue, ok := serverVarzValue.(string) if !ok { - Fatalf("Invalid server_name type in /varz: %+v", serverName) + Fatalf("Invalid %s type in /varz: %+v", key, serverVarzValue) } - return id, nil + return varzValue, nil } - var id string + var varzValue string var err error - id, err = getServerName() + varzValue, err = getServerVarzValue() if err == nil { - return id + return varzValue } - // Retry periodically until available, in case it never starts - // then a liveness check against the NATS Server itself should - // detect that an restart the server, in terms of the exporter - // we just wait for it to eventually be available. for range time.NewTicker(retryInterval).C { - id, err = getServerName() + varzValue, err = getServerVarzValue() if err != nil { - Errorf("Could not find server id: %s", err) + Errorf("Could not find %s: %s", key, err) continue } break } - return id + return varzValue } // Describe the metric to the Prometheus server. diff --git a/main.go b/main.go index 8b75b75..a48f151 100644 --- a/main.go +++ b/main.go @@ -159,21 +159,24 @@ necessary.`) // Create an instance of the NATS exporter. exp := exporter.NewExporter(opts) - if len(args) == 1 && opts.UseInternalServerID { + switch { + case len(args) == 1 && opts.UseInternalServerID: // Pick the server id from the /varz endpoint info. url := flag.Args()[0] id := collector.GetServerIDFromVarz(url, opts.RetryInterval) if err := exp.AddServer(id, url); err != nil { collector.Fatalf("Unable to setup server in exporter: %s, %s: %v", id, url, err) } - } else if len(args) == 1 && opts.UseServerName { - // Pick the server id from the /varz endpoint info. + + case len(args) == 1 && opts.UseServerName: + // Pick the server name from the /varz endpoint info. url := flag.Args()[0] id := collector.GetServerNameFromVarz(url, opts.RetryInterval) if err := exp.AddServer(id, url); err != nil { collector.Fatalf("Unable to setup server in exporter: %s, %s: %v", id, url, err) } - } else { + + default: // For each URL specified, add the NATS server with the optional ID. for _, arg := range args { // This should make the http request to get the server id From 178588bd9ca8adfc09c8a029c22a2a30dea21dbe Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 6 Mar 2023 21:48:16 +0100 Subject: [PATCH 4/5] add unit test for the new API --- collector/collector_test.go | 12 ++++++++++++ test/test.go | 27 +++++++++++++++++++-------- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/collector/collector_test.go b/collector/collector_test.go index 15a496d..b4af926 100644 --- a/collector/collector_test.go +++ b/collector/collector_test.go @@ -176,6 +176,18 @@ func TestServerIDFromVarz(t *testing.T) { } } +func TestServerNameFromVarz(t *testing.T) { + serverName := "My Awesome Server Name" + s := pet.RunServerWithName(serverName) + defer s.Shutdown() + + url := fmt.Sprintf("http://localhost:%d/", pet.MonitorPort) + result := GetServerNameFromVarz(url, 2*time.Second) + if result != serverName { + t.Fatalf("Unexpected server name: %v", result) + } +} + func TestVarz(t *testing.T) { s := pet.RunServer() defer s.Shutdown() diff --git a/test/test.go b/test/test.go index cc1668a..ac85d99 100644 --- a/test/test.go +++ b/test/test.go @@ -50,6 +50,11 @@ func RunServer() *server.Server { return RunServerWithPorts(ClientPort, MonitorPort) } +// RunServerWithName runs the NATS server in a go routine +func RunServerWithName(name string) *server.Server { + return RunServerWithPortsAndName(ClientPort, MonitorPort, name) +} + // RunStreamingServer runs the STAN server in a go routine. func RunStreamingServer() *nss.StanServer { return RunStreamingServerWithPorts(nss.DefaultClusterID, ClientPort, MonitorPort) @@ -105,8 +110,8 @@ func RunStreamingServerWithPorts(clusterID string, port, monitorPort int) *nss.S return s } -// RunServerWithPorts runs the NATS server with a monitor port in a go routine -func RunServerWithPorts(cport, mport int) *server.Server { +// RunServerWithPortsAndName runs the NATS server with a monitor port and a name in a go routine +func RunServerWithPortsAndName(cport, mport int, serverName string) *server.Server { var enableLogging bool resetPreviousHTTPConnections() @@ -116,12 +121,13 @@ func RunServerWithPorts(cport, mport int) *server.Server { // enableLogging = true opts := &server.Options{ - Host: "127.0.0.1", - Port: cport, - HTTPHost: "127.0.0.1", - HTTPPort: mport, - NoLog: !enableLogging, - NoSigs: true, + ServerName: serverName, + Host: "127.0.0.1", + Port: cport, + HTTPHost: "127.0.0.1", + HTTPPort: mport, + NoLog: !enableLogging, + NoSigs: true, } s := server.New(opts) @@ -167,6 +173,11 @@ func RunServerWithPorts(cport, mport int) *server.Server { panic("Unable to start NATS Server in Go Routine") } +// RunServerWithPorts runs the NATS server with a monitor port in a go routine +func RunServerWithPorts(cport, mport int) *server.Server { + return RunServerWithPortsAndName(cport, mport, "") +} + var tempRoot = filepath.Join(os.TempDir(), "exporter") // RunJetStreamServerWithPorts starts a JetStream server. From adad172c6c3f6dd0e4b6309142ff82ab4c27d456 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 6 Mar 2023 21:55:51 +0100 Subject: [PATCH 5/5] fix: revert bad removal --- exporter/exporter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/exporter.go b/exporter/exporter.go index 1f19766..62f72b5 100644 --- a/exporter/exporter.go +++ b/exporter/exporter.go @@ -176,7 +176,7 @@ func (ne *NATSExporter) InitializeCollectors() error { } getJsz := opts.GetJszFilter != "" - if !opts.GetConnz && !opts.GetRoutez && + if !opts.GetHealthz && !opts.GetConnz && !opts.GetConnzDetailed && !opts.GetRoutez && !opts.GetSubz && !opts.GetVarz && !opts.GetGatewayz && !opts.GetLeafz && !opts.GetStreamingChannelz && !opts.GetStreamingServerz && !opts.GetReplicatorVarz && !getJsz { return fmt.Errorf("no Collectors specfied")