From dc7cf35f3f285111098d16484582f97b7b9b6df1 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Thu, 22 Jul 2021 12:42:44 -0700 Subject: [PATCH] Update to use the ballast memory instead of the flag: * 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 --- .../config/collector/agent_config.yaml | 12 +- .../config/collector/full_config_linux.yaml | 18 +-- .../config/collector/gateway_config.yaml | 12 +- .../config/collector/otlp_config_linux.yaml | 12 +- .../collector/upstream_agent_config.yaml | 8 +- cmd/otelcol/main.go | 104 +++++++--------- cmd/otelcol/main_test.go | 115 +++++++++--------- internal/components/components.go | 2 + internal/components/components_test.go | 1 + 9 files changed, 144 insertions(+), 140 deletions(-) diff --git a/cmd/otelcol/config/collector/agent_config.yaml b/cmd/otelcol/config/collector/agent_config.yaml index 49c0beb75e..58f7ebc666 100644 --- 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 index f06da79044..0e175d4f41 100644 --- 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 index 7a55ff1197..8601c586f8 100644 --- 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 index ab124d5265..33d735c9a7 100644 --- 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 index 4fe5047896..d18394850e 100644 --- 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 index a9ca83dd6d..785bfc8d1a 100644 --- 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 +} diff --git a/cmd/otelcol/main_test.go b/cmd/otelcol/main_test.go index c2cc9c63c6..8217742430 100644 --- a/cmd/otelcol/main_test.go +++ b/cmd/otelcol/main_test.go @@ -63,43 +63,80 @@ func TestGetKeyValue(t *testing.T) { } } -func TestCheckRuntimeParams(*testing.T) { +func TestCheckRuntimeParams_Default(t *testing.T) { oldArgs := os.Args - os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig)) - checkConfig() + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) checkRuntimeParams() + assert.Equal(t, "168", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "460", os.Getenv(memLimitMiBEnvVarName)) os.Args = oldArgs - os.Setenv(memTotalEnvVarName, "1000") + os.Clearenv() +} + +func TestCheckRuntimeParams_MemTotalEnv(t *testing.T) { + oldArgs := os.Args + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) + assert.NoError(t, os.Setenv(memTotalEnvVarName, "1000")) checkRuntimeParams() + assert.Equal(t, "330", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "900", os.Getenv(memLimitMiBEnvVarName)) os.Args = oldArgs - os.Setenv(ballastEnvVarName, "50") - setMemoryBallast(100) - os.Unsetenv(ballastEnvVarName) + os.Clearenv() +} + +func TestCheckRuntimeParams_MemTotalAndBallastEnvs(t *testing.T) { + oldArgs := os.Args + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) + assert.NoError(t, os.Setenv(memTotalEnvVarName, "200")) + assert.NoError(t, os.Setenv(ballastEnvVarName, "90")) checkRuntimeParams() + assert.Equal(t, "90", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "180", os.Getenv(memLimitMiBEnvVarName)) os.Args = oldArgs os.Clearenv() } -func HelperTestSetMemoryBallast(val string, t *testing.T) { - args := os.Args[1:] - _, c := getKeyValue(args, "--mem-ballast-size-mib") - if c != val { - t.Errorf("Expected memory ballast CLI param %v got %v", val, c) - } - b := os.Getenv(ballastEnvVarName) - if b != val { - t.Errorf("Expected memory ballast %v got %v", val, b) - } +func TestCheckRuntimeParams_LimitAndBallastEnvs(t *testing.T) { + oldArgs := os.Args + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) + assert.NoError(t, os.Setenv(memLimitMiBEnvVarName, "250")) + assert.NoError(t, os.Setenv(ballastEnvVarName, "120")) + checkRuntimeParams() + assert.Equal(t, "120", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "250", os.Getenv(memLimitMiBEnvVarName)) + + os.Args = oldArgs + os.Clearenv() } -func HelperTestSetMemoryLimit(val string, t *testing.T) { - b := os.Getenv(memLimitMiBEnvVarName) - if b != val { - t.Errorf("Expected memory limit %v got %v", val, b) - } +func TestCheckRuntimeParams_MemTotalLimitAndBallastEnvs(t *testing.T) { + oldArgs := os.Args + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) + assert.NoError(t, os.Setenv(memTotalEnvVarName, "200")) + assert.NoError(t, os.Setenv(memLimitMiBEnvVarName, "150")) + assert.NoError(t, os.Setenv(ballastEnvVarName, "50")) + checkRuntimeParams() + assert.Equal(t, "50", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "150", os.Getenv(memLimitMiBEnvVarName)) + + os.Args = oldArgs + os.Clearenv() +} + +func TestCheckRuntimeParams_MemTotalEnvAndBallastFlag(t *testing.T) { + oldArgs := os.Args + assert.NoError(t, os.Setenv(configEnvVarName, path.Join("../../", defaultLocalSAPMConfig))) + assert.NoError(t, os.Setenv(memTotalEnvVarName, "200")) + os.Args = append(os.Args, "--mem-ballast-size-mib=90") + checkRuntimeParams() + assert.Equal(t, "90", os.Getenv(ballastEnvVarName)) + assert.Equal(t, "180", os.Getenv(memLimitMiBEnvVarName)) + + os.Args = oldArgs + os.Clearenv() } func TestUseConfigFromEnvVar(t *testing.T) { @@ -223,37 +260,3 @@ service: }) } } - -func TestSetMemoryBallast(t *testing.T) { - oldArgs := os.Args - setMemoryBallast(100) - - HelperTestSetMemoryBallast("33", t) - - os.Args = oldArgs - os.Setenv(ballastEnvVarName, "50") - defer os.Unsetenv(ballastEnvVarName) - setMemoryBallast(100) - - HelperTestSetMemoryBallast("50", t) - os.Args = oldArgs -} - -func TestSetMemoryLimit(t *testing.T) { - oldArgs := os.Args - setMemoryLimit(100) - - HelperTestSetMemoryLimit("90", t) - - os.Args = oldArgs - os.Unsetenv(memLimitMiBEnvVarName) - setMemoryLimit(100000) - - HelperTestSetMemoryLimit("90000", t) - - os.Args = oldArgs - os.Setenv(memLimitMiBEnvVarName, "200") - setMemoryLimit(100) - - HelperTestSetMemoryLimit("200", t) -} diff --git a/internal/components/components.go b/internal/components/components.go index e38b47c43b..d769c2ed7a 100644 --- a/internal/components/components.go +++ b/internal/components/components.go @@ -46,6 +46,7 @@ import ( "go.opentelemetry.io/collector/exporter/loggingexporter" "go.opentelemetry.io/collector/exporter/otlpexporter" "go.opentelemetry.io/collector/exporter/otlphttpexporter" + "go.opentelemetry.io/collector/extension/ballastextension" "go.opentelemetry.io/collector/extension/healthcheckextension" "go.opentelemetry.io/collector/extension/pprofextension" "go.opentelemetry.io/collector/extension/zpagesextension" @@ -77,6 +78,7 @@ func Get() (component.Factories, error) { pprofextension.NewFactory(), smartagentextension.NewFactory(), zpagesextension.NewFactory(), + ballastextension.NewFactory(), ) if err != nil { errs = append(errs, err) diff --git a/internal/components/components_test.go b/internal/components/components_test.go index 5d960ac0d6..02f60196e3 100644 --- a/internal/components/components_test.go +++ b/internal/components/components_test.go @@ -32,6 +32,7 @@ func TestDefaultComponents(t *testing.T) { "pprof", "smartagent", "zpages", + "memory_ballast", } expectedReceivers := []config.Type{ "carbon",