Rename "servers" to "instances" for logstash (#391)

kuskoman · Nov 8, 2024 · a9ef6ac · a9ef6ac
1 parent 846f972
commit a9ef6ac
Show file tree

Hide file tree

Showing 17 changed files with 67 additions and 50 deletions.
diff --git a/.docker/exporter-config.yml b/.docker/exporter-config.yml
@@ -1,5 +1,5 @@
 logstash:
-  servers:
+  instances:
     - url: "http://logstash:9600"
     - url: "http://logstash2:9600"
 server:

diff --git a/README.md b/README.md
@@ -108,9 +108,11 @@ The Helm chart has its own [README](./chart/README.md).
 
 The application is now configured using a YAML file instead of environment variables. An example configuration is as follows:
 
+**Important:** The `servers` section got renamed to `instances`. For now both names are supported, but the `servers` name will be removed in the future.
+
 ```yaml
 logstash:
-  servers:
+  instances:
     - url: "http://logstash:9600" # URL to Logstash API
     - url: "http://logstash2:9600"
 server:

diff --git a/chart/README.md b/chart/README.md
@@ -23,11 +23,11 @@
 
 ### Custom logstash-exporter configuration
 
-| Name                   | Description                 | Value                                                |
-| ---------------------- | --------------------------- | ---------------------------------------------------- |
-| `customConfig.enabled` | Enable custom configuration | `false`                                              |
+| Name                   | Description                 | Value                                                  |
+| ---------------------- | --------------------------- | ------------------------------------------------------ |
+| `customConfig.enabled` | Enable custom configuration | `false`                                                |
 | `customConfig.config`  | Custom configuration        | `logstash:
-  servers:
+  instances:
     - "http://logstash:9600"
 ` |
 

diff --git a/chart/schema.json b/chart/schema.json
@@ -53,7 +53,7 @@
                 "config": {
                     "type": "string",
                     "description": "Custom configuration",
-                    "default": "logstash:\n  servers:\n    - \"http://logstash:9600\"\n"
+                    "default": "logstash:\n  instances:\n    - \"http://logstash:9600\"\n"
                 }
             }
         },

diff --git a/chart/templates/configmap.yaml b/chart/templates/configmap.yaml
@@ -11,7 +11,7 @@ data:
     {{- .Values.customConfig.config | nindent 4 }}
   {{- else }}
     logstash:
-      servers:
+      instances:
         {{ range .Values.logstash.urls -}}
         - url: {{ . | quote }}
         {{ end }}

diff --git a/chart/values.yaml b/chart/values.yaml
@@ -33,7 +33,7 @@ customConfig:
   ##
   config: |
     logstash:
-      servers:
+      instances:
         - "http://logstash:9600"
 
 ## @section Image settings

diff --git a/config.example.yml b/config.example.yml
@@ -1,5 +1,5 @@
 logstash:
-  servers:
+  instances:
     - url: "http://localhost:9600"
   timeout: 3s
 server:

diff --git a/fixtures/valid_config.yml b/fixtures/valid_config.yml
@@ -1,5 +1,5 @@
 logstash:
-  servers:
+  instances:
     - url: "http://localhost:9601"
   httpTimeout: 3s
   httpInsecure: true

diff --git a/internal/server/healthcheck.go b/internal/server/healthcheck.go
@@ -10,9 +10,9 @@ import (
 	"github.com/kuskoman/logstash-exporter/pkg/config"
 )
 
-func convertServersToUrls(servers []*config.LogstashServer) []string {
-	urls := make([]string, len(servers))
-	for i, server := range servers {
+func convertInstancesToUrls(instances []*config.LogstashInstance) []string {
+	urls := make([]string, len(instances))
+	for i, server := range instances {
 		urls[i] = server.Host
 	}
 

diff --git a/internal/server/server.go b/internal/server/server.go
@@ -14,7 +14,7 @@ import (
 // to the server's mux. The prometheus handler is managed under the
 // hood by the prometheus client library.
 func NewAppServer(cfg *config.Config) *http.Server {
-	logstashUrls := convertServersToUrls(cfg.Logstash.Servers)
+	logstashUrls := convertInstancesToUrls(cfg.Logstash.Instances)
 
 	mux := http.NewServeMux()
 	mux.Handle("/metrics", promhttp.Handler())

diff --git a/internal/server/server_test.go b/internal/server/server_test.go
@@ -51,7 +51,7 @@ func TestNewAppServer(t *testing.T) {
 	t.Run("test handling of /healthcheck endpoint", func(t *testing.T) {
 		cfg := &config.Config{
 			Logstash: config.LogstashConfig{
-				Servers: []*config.LogstashServer{
+				Instances: []*config.LogstashInstance{
 					{Host: "http://localhost:1234"},
 				},
 			},

diff --git a/internal/startup_manager/startup_manager.go b/internal/startup_manager/startup_manager.go
@@ -161,7 +161,7 @@ func (sm *StartupManager) shutdownServer(ctx context.Context) error {
 
 func (sm *StartupManager) startPrometheus(cfg *config.Config) {
 	collectorManager := collector_manager.NewCollectorManager(
-		cfg.Logstash.Servers,
+		cfg.Logstash.Instances,
 		cfg.Logstash.HttpTimeout,
 	)
 

diff --git a/pkg/collector_manager/collector_manager.go b/pkg/collector_manager/collector_manager.go
@@ -28,7 +28,7 @@ type CollectorManager struct {
 	httpTimeout     time.Duration
 }
 
-func getClientsForEndpoints(endpoints []*config.LogstashServer) []logstash_client.Client {
+func getClientsForEndpoints(endpoints []*config.LogstashInstance) []logstash_client.Client {
 	clients := make([]logstash_client.Client, len(endpoints))
 
 	for i, endpoint := range endpoints {
@@ -38,9 +38,9 @@ func getClientsForEndpoints(endpoints []*config.LogstashServer) []logstash_clien
 	return clients
 }
 
-// NewCollectorManager creates a new CollectorManager with the provided logstash servers and http timeout
-func NewCollectorManager(servers []*config.LogstashServer, httpTimeout time.Duration) *CollectorManager {
-	clients := getClientsForEndpoints(servers)
+// NewCollectorManager creates a new CollectorManager with the provided logstash instances and http timeout
+func NewCollectorManager(instances []*config.LogstashInstance, httpTimeout time.Duration) *CollectorManager {
+	clients := getClientsForEndpoints(instances)
 
 	collectors := getCollectors(clients)
 

diff --git a/pkg/collector_manager/collector_manager_test.go b/pkg/collector_manager/collector_manager_test.go
@@ -17,15 +17,15 @@ func TestNewCollectorManager(t *testing.T) {
 	t.Parallel()
 
 	t.Run("multiple endpoints", func(t *testing.T) {
-		endpoint1 := &config.LogstashServer{
+		endpoint1 := &config.LogstashInstance{
 			Host: "http://localhost:9600",
 		}
 
-		endpoint2 := &config.LogstashServer{
+		endpoint2 := &config.LogstashInstance{
 			Host: "http://localhost:9601",
 		}
 
-		mockEndpoints := []*config.LogstashServer{endpoint1, endpoint2}
+		mockEndpoints := []*config.LogstashInstance{endpoint1, endpoint2}
 		cm := NewCollectorManager(mockEndpoints, httpTimeout)
 
 		if cm == nil {

diff --git a/pkg/config/exporter_config.go b/pkg/config/exporter_config.go
@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"log/slog"
 	"os"
 	"reflect"
@@ -23,16 +24,19 @@ var (
 	ExporterConfigLocation = getEnvWithDefault("EXPORTER_CONFIG_LOCATION", defaultConfigLocation)
 )
 
-// LogstashServer represents individual Logstash server configuration
-type LogstashServer struct {
+// LogstashInstance represents individual Logstash server configuration
+type LogstashInstance struct {
 	Host         string `yaml:"url"`
 	HttpInsecure bool   `yaml:"httpInsecure"`
 }
 
-// LogstashConfig holds the configuration for all Logstash servers
+// LogstashConfig holds the configuration for all Logstash instances
 type LogstashConfig struct {
-	Servers     []*LogstashServer `yaml:"servers"`
-	HttpTimeout time.Duration     `yaml:"httpTimeout"`
+	// LegacyServers is a deprecated field, use Instances instead
+	LegacyServers []*LogstashInstance `yaml:"servers"`
+
+	Instances   []*LogstashInstance `yaml:"instances"`
+	HttpTimeout time.Duration       `yaml:"httpTimeout"`
 }
 
 // ServerConfig represents the server configuration
@@ -65,6 +69,15 @@ func (config *Config) Equals(other *Config) bool {
 	return reflect.DeepEqual(config, other)
 }
 
+// handleLegacyServersProperty handles the deprecated 'servers' property.
+// This method will log a warning and append the legacy servers to the new 'instances' property.
+func (config *Config) handleLegacyServersProperty() {
+	if len(config.Logstash.LegacyServers) > 0 {
+		slog.Warn("The 'servers' property is deprecated, please use 'instances' instead", "servers", fmt.Sprintf("%v", config.Logstash.LegacyServers))
+		config.Logstash.Instances = append(config.Logstash.Instances, config.Logstash.LegacyServers...)
+	}
+}
+
 // loadConfig loads the configuration from the YAML file.
 func loadConfig(location string) (*Config, error) {
 	data, err := os.ReadFile(location)
@@ -78,6 +91,8 @@ func loadConfig(location string) (*Config, error) {
 		return nil, err
 	}
 
+	config.handleLegacyServersProperty()
+
 	return &config, nil
 }
 
@@ -102,9 +117,9 @@ func mergeWithDefault(config *Config) *Config {
 		config.Logging.Format = defaultLogFormat
 	}
 
-	if len(config.Logstash.Servers) == 0 {
+	if len(config.Logstash.Instances) == 0 {
 		slog.Debug("using default logstash server", "url", defaultLogstashURL)
-		config.Logstash.Servers = append(config.Logstash.Servers, &LogstashServer{
+		config.Logstash.Instances = append(config.Logstash.Instances, &LogstashInstance{
 			Host: defaultLogstashURL,
 		})
 	}

diff --git a/pkg/config/exporter_config_test.go b/pkg/config/exporter_config_test.go
@@ -19,8 +19,8 @@ func TestLoadConfig(t *testing.T) {
 		if config == nil {
 			t.Fatal("expected config to be non-nil")
 		}
-		if config.Logstash.Servers[0].Host != "http://localhost:9601" {
-			t.Errorf("expected URL to be %v, got %v", "http://localhost:9601", config.Logstash.Servers[0].Host)
+		if config.Logstash.Instances[0].Host != "http://localhost:9601" {
+			t.Errorf("expected URL to be %v, got %v", "http://localhost:9601", config.Logstash.Instances[0].Host)
 		}
 	})
 
@@ -67,7 +67,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9602"},
 				},
@@ -83,7 +83,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9602"},
 				},
@@ -105,7 +105,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9603"},
 				},
@@ -121,7 +121,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9602"},
 				},
@@ -143,7 +143,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9602"},
 				},
@@ -165,7 +165,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601"},
 					{Host: "http://localhost:9602"},
 				},
@@ -181,7 +181,7 @@ func TestConfigEquals(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: nil,
+				Instances: nil,
 			},
 		}
 
@@ -209,8 +209,8 @@ func TestMergeWithDefault(t *testing.T) {
 		if mergedConfig.Logging.Format != defaultLogFormat {
 			t.Errorf("expected format to be %v, got %v", defaultLogFormat, mergedConfig.Logging.Format)
 		}
-		if mergedConfig.Logstash.Servers[0].Host != defaultLogstashURL {
-			t.Errorf("expected URL to be %v, got %v", defaultLogstashURL, mergedConfig.Logstash.Servers[0].Host)
+		if mergedConfig.Logstash.Instances[0].Host != defaultLogstashURL {
+			t.Errorf("expected URL to be %v, got %v", defaultLogstashURL, mergedConfig.Logstash.Instances[0].Host)
 		}
 		if mergedConfig.Logstash.HttpTimeout != defaultHttpTimeout {
 			t.Errorf("expected http timeout to be %v, got %v", defaultHttpTimeout, mergedConfig.Logstash.HttpTimeout)
@@ -231,8 +231,8 @@ func TestMergeWithDefault(t *testing.T) {
 		if mergedConfig.Logging.Format != defaultLogFormat {
 			t.Errorf("expected format to be %v, got %v", defaultLogFormat, mergedConfig.Logging.Format)
 		}
-		if mergedConfig.Logstash.Servers[0].Host != defaultLogstashURL {
-			t.Errorf("expected URL to be %v, got %v", defaultLogstashURL, mergedConfig.Logstash.Servers[0].Host)
+		if mergedConfig.Logstash.Instances[0].Host != defaultLogstashURL {
+			t.Errorf("expected URL to be %v, got %v", defaultLogstashURL, mergedConfig.Logstash.Instances[0].Host)
 		}
 		if mergedConfig.Logstash.HttpTimeout != defaultHttpTimeout {
 			t.Errorf("expected http timeout to be %v, got %v", defaultHttpTimeout, mergedConfig.Logstash.HttpTimeout)
@@ -251,7 +251,7 @@ func TestMergeWithDefault(t *testing.T) {
 				Format: "json",
 			},
 			Logstash: LogstashConfig{
-				Servers: []*LogstashServer{
+				Instances: []*LogstashInstance{
 					{Host: "http://localhost:9601", HttpInsecure: true},
 					{Host: "http://localhost:9602"},
 				},
@@ -273,12 +273,12 @@ func TestMergeWithDefault(t *testing.T) {
 			t.Errorf("expected format to be %v, got %v", "json", mergedConfig.Logging.Format)
 		}
 
-		if mergedConfig.Logstash.Servers[0].Host != "http://localhost:9601" {
-			t.Errorf("expected URL to be %v, got %v", "http://localhost:9601", mergedConfig.Logstash.Servers[0].Host)
+		if mergedConfig.Logstash.Instances[0].Host != "http://localhost:9601" {
+			t.Errorf("expected URL to be %v, got %v", "http://localhost:9601", mergedConfig.Logstash.Instances[0].Host)
 		}
 
-		if mergedConfig.Logstash.Servers[1].Host != "http://localhost:9602" {
-			t.Errorf("expected URL to be %v, got %v", "http://localhost:9602", mergedConfig.Logstash.Servers[1].Host)
+		if mergedConfig.Logstash.Instances[1].Host != "http://localhost:9602" {
+			t.Errorf("expected URL to be %v, got %v", "http://localhost:9602", mergedConfig.Logstash.Instances[1].Host)
 		}
 		if mergedConfig.Logstash.HttpTimeout != 3*time.Second {
 			t.Errorf("expected http timeout to be %v, got %v", 3*time.Second, mergedConfig.Logstash.HttpTimeout)

diff --git a/scripts/migrate_v1_to_v2.sh b/scripts/migrate_v1_to_v2.sh
@@ -31,7 +31,7 @@ log_level=$(get_env_with_default "LOG_LEVEL" "info")
 
 cat << EOF
 logstash:
-  servers:
+  instances:
     - url: "$logstash_url"
 server:
   host: "${host:-0.0.0.0}"