Update to use the ballast memory instead of the flag: (#567)

* The memory_ballast extension is the only way to configure ballast starting with v0.31.0 of the collector; * The memory_limiter processor accesses the ballast directly from the config, no need to set ballast_memory_size anymore; Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
signalfx · Jul 23, 2021 · 5f8dec7 · 5f8dec7
1 parent 491b2f0
commit 5f8dec7
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 140 deletions.
diff --git a/cmd/otelcol/config/collector/agent_config.yaml b/cmd/otelcol/config/collector/agent_config.yaml
@@ -27,6 +27,11 @@ extensions:
       configDir: "${SPLUNK_COLLECTD_DIR}"
   zpages:
     #endpoint: 0.0.0.0:55679
+  memory_ballast:
+    # In general, the ballast should be set to 1/3 of the collector's memory, the limit
+    # should be 90% of the collector's memory.
+    # The simplest way to specify the ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable.
+    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
 
 receivers:
   fluentforward:
@@ -88,14 +93,9 @@ processors:
   batch:
   # Enabling the memory_limiter is strongly recommended for every pipeline.
   # Configuration is based on the amount of memory allocated to the collector.
-  # In general, the ballast should be set to 1/3 of the collector's memory, the limit
-  # should be 90% of the collector's memory. The simplest way to specify the
-  # ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable. Alternatively, the
-  # --mem-ballast-size-mib command line flag can be passed and take priority.
   # For more information about memory limiter, see
   # https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiter/README.md
   memory_limiter:
-    ballast_size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
     check_interval: 2s
     limit_mib: ${SPLUNK_MEMORY_LIMIT_MIB}
   # detect if the collector is running on a cloud system
@@ -148,7 +148,7 @@ exporters:
     loglevel: debug
 
 service:
-  extensions: [health_check, http_forwarder, zpages]
+  extensions: [health_check, http_forwarder, zpages, memory_ballast]
   pipelines:
     traces:
       receivers: [jaeger, otlp, smartagent/signalfx-forwarder, zipkin]

diff --git a/cmd/otelcol/config/collector/full_config_linux.yaml b/cmd/otelcol/config/collector/full_config_linux.yaml
@@ -217,14 +217,8 @@ processors:
 
   # Enables the memory limiter processor with default settings
   # Full configuration here: https://github.com/open-telemetry/opentelemetry-collector/tree/main/processor/memorylimiter
-  # Enabling the memory_limiter is strongly recommended for every pipeline.
-  # Configuration is based on the amount of memory allocated to the collector.
-  # The configuration below assumes 2GB of memory. In general, the ballast
-  # should be set to 1/3 of the collector's memory, the limit should be 90% of
-  # the collector's memory.
   # NOTE: These settings need to be change when using this processor
   memory_limiter:
-    ballast_size_mib: 650
     check_interval: 2s
     limit_mib: 1800
 
@@ -535,6 +529,16 @@ extensions:
   zpages:
     #endpoint: 0.0.0.0:55679
 
+  # Enables the memory_ballast extension
+  # Full configuration here: https://github.com/open-telemetry/opentelemetry-collector/tree/main/extension/ballastextension
+  memory_ballast:
+    # Enabling the memory_limiter is strongly recommended for every pipeline.
+    # Configuration is based on the amount of memory allocated to the collector.
+    # The configuration below assumes 2GB of memory for the collector.
+    # In general, the ballast should be set to 1/3 of the collector's memory,
+    # the limit should be 90% of the collector's memory.
+    size_mib: 650
+
 ###############################################################################
 # Service
 # In order to enable a configuration it must be defined in this section
@@ -544,7 +548,7 @@ extensions:
 service:
 
   # Which extensions you want to enable
-  extensions: [health_check, http_forwarder, zpages]
+  extensions: [health_check, http_forwarder, zpages, memory_ballast]
 
   # Pipelines are data source specific today
   # Every data source is made up of at least one receiver and one exporter

diff --git a/cmd/otelcol/config/collector/gateway_config.yaml b/cmd/otelcol/config/collector/gateway_config.yaml
@@ -11,6 +11,11 @@ extensions:
       endpoint: "https://api.${SPLUNK_REALM}.signalfx.com"
   zpages:
     endpoint: 0.0.0.0:55679
+  memory_ballast:
+    # In general, the ballast should be set to 1/3 of the collector's memory, the limit
+    # should be 90% of the collector's memory.
+    # The simplest way to specify the ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable.
+    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
 
 receivers:
   jaeger:
@@ -53,14 +58,9 @@ processors:
   batch:
   # Enabling the memory_limiter is strongly recommended for every pipeline.
   # Configuration is based on the amount of memory allocated to the collector.
-  # In general, the ballast should be set to 1/3 of the collector's memory, the limit
-  # should be 90% of the collector's memory. The simplest way to specify the
-  # ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable. Alternatively, the
-  # --mem-ballast-size-mib command line flag can be passed and take priority.
   # For more information about memory limiter, see
   # https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiter/README.md
   memory_limiter:
-    ballast_size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
     check_interval: 2s
     limit_mib: ${SPLUNK_MEMORY_LIMIT_MIB}
 
@@ -97,7 +97,7 @@ exporters:
     #loglevel: debug
 
 service:
-  extensions: [health_check, http_forwarder, zpages]
+  extensions: [health_check, http_forwarder, zpages, memory_ballast]
   pipelines:
     traces:
       receivers: [jaeger, otlp, sapm, zipkin]

diff --git a/cmd/otelcol/config/collector/otlp_config_linux.yaml b/cmd/otelcol/config/collector/otlp_config_linux.yaml
@@ -45,14 +45,9 @@ processors:
   batch:
   # Enabling the memory_limiter is strongly recommended for every pipeline.
   # Configuration is based on the amount of memory allocated to the collector.
-  # In general, the ballast should be set to 1/3 of the collector's memory, the limit
-  # should be 90% of the collector's memory. The simplest way to specify the
-  # ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable. Alternatively, the
-  # --mem-ballast-size-mib command line flag can be passed and take priority.
   # For more information about memory limiter, see
   # https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiter/README.md
   memory_limiter:
-    ballast_size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
     check_interval: 2s
     limit_mib: ${SPLUNK_MEMORY_LIMIT_MIB}
 
@@ -96,9 +91,14 @@ extensions:
       endpoint: "https://api.${SPLUNK_REALM}.signalfx.com"
   zpages:
     endpoint: 0.0.0.0:55679
+  memory_ballast:
+    # In general, the ballast should be set to 1/3 of the collector's memory, the limit
+    # should be 90% of the collector's memory.
+    # The simplest way to specify the ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable.
+    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
 
 service:
-  extensions: [health_check, http_forwarder, zpages]
+  extensions: [health_check, http_forwarder, zpages, memory_ballast]
   pipelines:
     traces:
       receivers: [jaeger, otlp, smartagent/signalfx-forwarder, zipkin]

diff --git a/cmd/otelcol/config/collector/upstream_agent_config.yaml b/cmd/otelcol/config/collector/upstream_agent_config.yaml
@@ -24,6 +24,11 @@ extensions:
       #endpoint: "${SPLUNK_GATEWAY_URL}"
   zpages:
     #endpoint: 0.0.0.0:55679
+  memory_ballast:
+    # In general, the ballast should be set to 1/3 of the collector's memory, the limit
+    # should be 90% of the collector's memory.
+    # The simplest way to specify the ballast size is set the value of SPLUNK_BALLAST_SIZE_MIB env variable.
+    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
 
 receivers:
   fluentforward:
@@ -89,7 +94,6 @@ processors:
   # For more information about memory limiter, see
   # https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiter/README.md
   memory_limiter:
-    ballast_size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
     check_interval: 2s
     limit_mib: ${SPLUNK_MEMORY_LIMIT_MIB}
   # detect if the collector is running on a cloud system
@@ -141,7 +145,7 @@ exporters:
     loglevel: debug
 
 service:
-  extensions: [health_check, http_forwarder, zpages]
+  extensions: [health_check, http_forwarder, zpages, memory_ballast]
   pipelines:
     # Required for Splunk APM
     traces:

diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go
@@ -36,6 +36,7 @@ import (
 	"github.com/signalfx/splunk-otel-collector/internal/version"
 )
 
+// The list of environment variables must be the same as what is used in the yaml configs.
 const (
 	ballastEnvVarName         = "SPLUNK_BALLAST_SIZE_MIB"
 	configEnvVarName          = "SPLUNK_CONFIG"
@@ -136,35 +137,25 @@ func checkRuntimeParams() {
 	checkConfig()
 
 	// Set default total memory
-	memTotalSizeMiB := defaultMemoryTotalMiB
+	memTotalSize := defaultMemoryTotalMiB
 	// Check if the total memory is specified via the env var
-	memTotalEnvVarVal := os.Getenv(memTotalEnvVarName)
 	// If so, validate and change total memory
-	if memTotalEnvVarVal != "" {
+	if os.Getenv(memTotalEnvVarName) != "" {
 		// Check if it is a numeric value.
-		val, err := strconv.Atoi(memTotalEnvVarVal)
-		if err != nil {
-			log.Fatalf("Expected a number in %s env variable but got %s", memTotalEnvVarName, memTotalEnvVarVal)
-		}
+		memTotalSize = envVarAsInt(memTotalEnvVarName)
 		// Ensure number is above some threshold
-		if 99 > val {
-			log.Fatalf("Expected a number greater than 99 for %s env variable but got %s", memTotalEnvVarName, memTotalEnvVarVal)
+		if 99 > memTotalSize {
+			log.Fatalf("Expected a number greater than 99 for %s env variable but got %d", memTotalEnvVarName, memTotalSize)
 		}
-		memTotalSizeMiB = val
 	}
 
-	// Check if memory ballast flag was passed
-	// If so, ensure memory ballast env var is not set
-	// Then set memory ballast and limit properly
-	_, ballastSize := getKeyValue(os.Args[1:], "--mem-ballast-size-mib")
-	if ballastSize != "" {
-		if os.Getenv(ballastEnvVarName) != "" {
-			log.Fatalf("Both %v and '--mem-ballast-size-mib' were specified, but only one is allowed", ballastEnvVarName)
-		}
-		os.Setenv(ballastEnvVarName, ballastSize)
+	ballastSize := setMemoryBallast(memTotalSize)
+	memLimit := setMemoryLimit(memTotalSize)
+
+	// Validate memoryLimit and memoryBallast are sane
+	if 2*ballastSize > memLimit {
+		log.Fatalf("Memory limit (%d) is less than 2x ballast (%d). Increase memory limit or decrease ballast size.", memLimit, ballastSize)
 	}
-	setMemoryBallast(memTotalSizeMiB)
-	setMemoryLimit(memTotalSizeMiB)
 }
 
 // Sets flag '--config' to specified env var SPLUNK_CONFIG, if the flag not specified.
@@ -256,54 +247,43 @@ func checkRequiredEnvVars(path string) {
 }
 
 // Validate and set the memory ballast
-func setMemoryBallast(memTotalSizeMiB int) {
-	// Check if the memory ballast is specified via the env var
-	ballastSize := os.Getenv(ballastEnvVarName)
-	// If so, validate and set properly
-	if ballastSize != "" {
-		// Check if it is a numeric value.
-		val, err := strconv.Atoi(ballastSize)
-		if err != nil {
-			log.Fatalf("Expected a number in %s env variable but got %s", ballastEnvVarName, ballastSize)
-		}
-		if 33 > val {
-			log.Fatalf("Expected a number greater than 33 for %s env variable but got %s", ballastEnvVarName, ballastSize)
+func setMemoryBallast(memTotalSizeMiB int) int {
+	// Check if deprecated memory ballast flag was passed, if so, ensure the env variable for memory ballast is set.
+	// Then set memory ballast and limit properly
+	_, ballastSizeFlag := getKeyValue(os.Args[1:], "--mem-ballast-size-mib")
+	if ballastSizeFlag != "" {
+		if os.Getenv(ballastEnvVarName) != "" {
+			log.Fatalf("Both %v and '--mem-ballast-size-mib' were specified, but only one is allowed", ballastEnvVarName)
 		}
-	} else {
-		ballastSize = strconv.Itoa(memTotalSizeMiB * defaultMemoryBallastPercentage / 100)
-		os.Setenv(ballastEnvVarName, ballastSize)
+		os.Setenv(ballastEnvVarName, ballastSizeFlag)
 	}
 
-	args := os.Args[1:]
-	if !contains(args, "--mem-ballast-size-mib") {
-		// Inject the command line flag that controls the ballast size.
-		os.Args = append(os.Args, "--mem-ballast-size-mib="+ballastSize)
+	ballastSize := memTotalSizeMiB * defaultMemoryBallastPercentage / 100
+	// Check if the memory ballast is specified via the env var, if so, validate and set properly.
+	if os.Getenv(ballastEnvVarName) != "" {
+		ballastSize = envVarAsInt(ballastEnvVarName)
+		if 33 > ballastSize {
+			log.Fatalf("Expected a number greater than 33 for %s env variable but got %d", ballastEnvVarName, ballastSize)
+		}
 	}
-	log.Printf("Set ballast to %s MiB", ballastSize)
+
+	os.Setenv(ballastEnvVarName, strconv.Itoa(ballastSize))
+	log.Printf("Set ballast to %d MiB", ballastSize)
+	return ballastSize
 }
 
 // Validate and set the memory limit
-func setMemoryLimit(memTotalSizeMiB int) {
-	memLimit := 0
-	// Check if the memory limit is specified via the env var
-	memoryLimit := os.Getenv(memLimitMiBEnvVarName)
-	// If not, calculate it from memTotalSizeMiB
-	if memoryLimit == "" {
-		memLimit = memTotalSizeMiB * defaultMemoryLimitPercentage / 100
-	} else {
-		memLimit, _ = strconv.Atoi(memoryLimit)
-	}
+func setMemoryLimit(memTotalSizeMiB int) int {
+	memLimit := memTotalSizeMiB * defaultMemoryLimitPercentage / 100
 
-	// Validate memoryLimit is sane
-	args := os.Args[1:]
-	_, b := getKeyValue(args, "--mem-ballast-size-mib")
-	ballastSize, _ := strconv.Atoi(b)
-	if (ballastSize * 2) > memLimit {
-		log.Fatalf("Memory limit (%v) is less than 2x ballast (%v). Increase memory limit or decrease ballast size.", memLimit, ballastSize)
+	// Check if the memory limit is specified via the env var, if so, validate and set properly.
+	if os.Getenv(memLimitMiBEnvVarName) != "" {
+		memLimit = envVarAsInt(memLimitMiBEnvVarName)
 	}
 
 	os.Setenv(memLimitMiBEnvVarName, strconv.Itoa(memLimit))
 	log.Printf("Set memory limit to %d MiB", memLimit)
+	return memLimit
 }
 
 // Returns a ParserProvider that reads configuration YAML from an environment variable when applicable.
@@ -332,3 +312,13 @@ func runInteractive(params service.CollectorSettings) error {
 
 	return nil
 }
+
+func envVarAsInt(env string) int {
+	envVal := os.Getenv(env)
+	// Check if it is a numeric value.
+	val, err := strconv.Atoi(envVal)
+	if err != nil {
+		log.Fatalf("Expected a number in %s env variable but got %s", env, envVal)
+	}
+	return val
+}