addressing previous PR comments

signalfx · Mar 12, 2024 · d10b2d9 · d10b2d9
1 parent f6411dc
commit d10b2d9
Show file tree

Hide file tree

Showing 15 changed files with 17 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+
+
+### 💡 Enhancements 💡
+
+- (Splunk) 'memory_ballast` has been removed. If GOMEMLIMIT env var is not used a 90% of total memory limit is set by default. 
+
 ### 🛑 Breaking changes 🛑
 
 - (Splunk) `spanmetricsprocessor`: Remove `spanmetricsprocessor`. Please use `spanmetrics` connector instead.

diff --git a/cmd/otelcol/config/collector/agent_config.yaml b/cmd/otelcol/config/collector/agent_config.yaml
@@ -28,10 +28,6 @@ extensions:
       configDir: "${SPLUNK_COLLECTD_DIR}"
   zpages:
     #endpoint: "${SPLUNK_LISTEN_INTERFACE}: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:
@@ -170,7 +166,7 @@ service:
   telemetry:
     metrics:
       address: "${SPLUNK_LISTEN_INTERFACE}:8888"
-  extensions: [health_check, http_forwarder, zpages, memory_ballast, smartagent]
+  extensions: [health_check, http_forwarder, zpages, smartagent]
   pipelines:
     traces:
       receivers: [jaeger, otlp, smartagent/signalfx-forwarder, zipkin]

diff --git a/cmd/otelcol/config/collector/ecs_ec2_config.yaml b/cmd/otelcol/config/collector/ecs_ec2_config.yaml
@@ -18,11 +18,6 @@ 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:
   # The fluentforward receiver can be used to forward logs from the Docker fluentd logging driver.
@@ -149,7 +144,7 @@ exporters:
     log_data_enabled: false
 
 service:
-  extensions: [health_check, http_forwarder, zpages, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
   pipelines:
     traces:
       receivers: [jaeger, otlp, zipkin, smartagent/signalfx-forwarder]

diff --git a/cmd/otelcol/config/collector/fargate_config.yaml b/cmd/otelcol/config/collector/fargate_config.yaml
@@ -16,11 +16,6 @@ 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:
@@ -123,7 +118,7 @@ exporters:
     log_data_enabled: false
 
 service:
-  extensions: [health_check, http_forwarder, zpages, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
   pipelines:
     traces:
       receivers: [jaeger, otlp, zipkin, smartagent/signalfx-forwarder]

diff --git a/cmd/otelcol/config/collector/full_config_linux.yaml b/cmd/otelcol/config/collector/full_config_linux.yaml
@@ -753,16 +753,6 @@ 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
@@ -772,7 +762,7 @@ extensions:
 service:
 
   # Which extensions you want to enable
-  extensions: [health_check, http_forwarder, zpages, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
 
   # 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,11 +11,6 @@ extensions:
       endpoint: "https://api.${SPLUNK_REALM}.signalfx.com"
   zpages:
     endpoint: "${SPLUNK_LISTEN_INTERFACE}: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:
@@ -133,7 +128,7 @@ service:
   telemetry:
     metrics:
       address: "${SPLUNK_LISTEN_INTERFACE}:8888"
-  extensions: [health_check, http_forwarder, zpages, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
   pipelines:
     traces:
       receivers: [jaeger, otlp, sapm, zipkin]

diff --git a/cmd/otelcol/config/collector/logs_config_linux.yaml b/cmd/otelcol/config/collector/logs_config_linux.yaml
@@ -687,10 +687,6 @@ extensions:
   health_check:
     endpoint: "${SPLUNK_LISTEN_INTERFACE}:13133"
 
-  memory_ballast:
-    # In general, the ballast should be set to 1/3 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}
 
   # Storage extension for storing filelog checkpoints.
   # Checkpoints allow the receiver to pick up where it left off in the case of a
@@ -705,7 +701,7 @@ extensions:
 
 
 service:
-  extensions: [health_check, memory_ballast, file_storage/filelogs]
+  extensions: [health_check, file_storage/filelogs]
   pipelines:
     logs:
       receivers:

diff --git a/cmd/otelcol/config/collector/otlp_config_linux.yaml b/cmd/otelcol/config/collector/otlp_config_linux.yaml
@@ -104,14 +104,9 @@ 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, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
   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
@@ -27,10 +27,6 @@ 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 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:
@@ -158,7 +154,7 @@ exporters:
     verbosity: detailed
 
 service:
-  extensions: [health_check, http_forwarder, zpages, memory_ballast]
+  extensions: [health_check, http_forwarder, zpages]
   pipelines:
     # Required for Splunk APM
     traces:

diff --git a/deployments/salt/templates/agent_config.yaml b/deployments/salt/templates/agent_config.yaml
@@ -16,8 +16,6 @@ receivers:
             - targets: ['127.0.0.1:8888']
 
 extensions:
-  memory_ballast:
-    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
 
 processors:
   memory_limiter:
@@ -31,7 +29,7 @@ exporters:
     verbosity: normal
 
 service:
-  extensions: [memory_ballast]
+  extensions:
   pipelines:
     metrics:
       receivers: [otlp, prometheus]

diff --git a/internal/configconverter/testdata/ballast_mem_limiter.yaml b/internal/configconverter/testdata/ballast_mem_limiter.yaml
@@ -1,6 +1,4 @@
 extensions:
-  memory_ballast:
-    size_mib: 64
 receivers:
   hostmetrics:
     collection_interval: 1s
@@ -27,4 +25,3 @@ service:
       exporters:
         - logging
   extensions:
-    - memory_ballast
diff --git a/internal/configconverter/testdata/ballast_mem_limiter_custom.yaml b/internal/configconverter/testdata/ballast_mem_limiter_custom.yaml
@@ -1,6 +1,4 @@
 extensions:
-  memory_ballast:
-    size_mib: 64
 receivers:
   hostmetrics:
     collection_interval: 1s
@@ -26,5 +24,3 @@ service:
         - memory_limiter/foo
       exporters:
         - logging
-  extensions:
-    - memory_ballast
diff --git a/internal/configconverter/testdata/disable_excessive_internal_metrics/new_input.yaml b/internal/configconverter/testdata/disable_excessive_internal_metrics/new_input.yaml
@@ -13,8 +13,6 @@ extensions:
   k8s_observer:
     auth_type: serviceAccount
     node: ${K8S_NODE_NAME}
-  memory_ballast:
-    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
   zpages: null
 processors:
   batch: null
@@ -169,7 +167,6 @@ service:
   extensions:
     - health_check
     - k8s_observer
-    - memory_ballast
     - zpages
   pipelines:
     metrics:

diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go
@@ -357,7 +357,7 @@ func TestSetSoftMemLimitWithoutGoMemLimitEnvVar(t *testing.T) {
 	settings, err = New([]string{})
 	require.NoError(t, err)
 	require.NotNil(t, settings)
-	require.Equal(t, int64(482344960), debug.SetMemoryLimit(100))
+	require.Equal(t, int64(482344960), debug.SetMemoryLimit(-1))
 
 }
 

diff --git a/tests/general/default_config_test.go b/tests/general/default_config_test.go
@@ -369,7 +369,7 @@ func TestDefaultAgentConfig(t *testing.T) {
 					"zipkin": map[string]any{"endpoint": fmt.Sprintf("%s:9411", ip)}},
 				"service": map[string]any{
 					"telemetry":  map[string]any{"metrics": map[string]any{"address": fmt.Sprintf("%s:8888", ip)}},
-					"extensions": []any{"health_check", "http_forwarder", "zpages", "memory_ballast", "smartagent"},
+					"extensions": []any{"health_check", "http_forwarder", "zpages", "smartagent"},
 					"pipelines": map[string]any{
 						"logs": map[string]any{
 							"exporters":  []any{"splunk_hec", "splunk_hec/profiling"},