Removed memory_ballast and making sure soft memory is set (#4404)

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+
+### 💡 Enhancements 💡
+
+- (Splunk) 'memory_ballast` has been removed. If GOMEMLIMIT env var is not set, then 90% of the total available memory limit is set by default. 
+
 ### 🛑 Breaking changes 🛑
 
 - (Splunk) `spanmetricsprocessor`: Remove `spanmetricsprocessor`. Please use `spanmetrics` connector instead.

cmd/otelcol/config/collector/agent_config.yaml

@@ -28,11 +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:
   fluentforward:
@@ -170,7 +165,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]

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]

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]

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

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]

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:

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]

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:

deployments/salt/templates/agent_config.yaml

@@ -16,8 +16,7 @@ receivers:
             - targets: ['127.0.0.1:8888']
 
 extensions:
-  memory_ballast:
-    size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
+  zpages:
 
 processors:
   memory_limiter:
@@ -31,7 +30,7 @@ exporters:
     verbosity: normal
 
 service:
-  extensions: [memory_ballast]
+  extensions: [zpages]
   pipelines:
     metrics:
       receivers: [otlp, prometheus]

internal/configconverter/remove_memory_ballast_key.go

@@ -0,0 +1,69 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//       http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package configconverter
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"regexp"
+
+	"go.opentelemetry.io/collector/confmap"
+)
+
+// RemoveMemoryBallastKey is a MapConverter that removes a memory_ballast on a
+// extension config if it exists.
+type RemoveMemoryBallastKey struct{}
+
+func removeMemoryBallastStrElementFromSlice(strList []interface{}) []interface{} {
+	ret := make([]interface{}, 0)
+	for i, v := range strList {
+		if v == "memory_ballast" {
+			ret = append(ret, strList[:i]...)
+			return append(ret, strList[i+1:]...)
+		}
+	}
+	return strList
+}
+
+func (RemoveMemoryBallastKey) Convert(_ context.Context, cfgMap *confmap.Conf) error {
+	if cfgMap == nil {
+		return fmt.Errorf("cannot RemoveMemoryBallastKey on nil *confmap.Conf")
+	}
+
+	const firstExp = "extensions::memory_ballast.*"
+	firstRegExp := regexp.MustCompile(firstExp)
+	const secondExp = "service::extensions"
+	secondRegExp := regexp.MustCompile(secondExp)
+
+	out := map[string]any{}
+	for _, k := range cfgMap.AllKeys() {
+		if firstRegExp.MatchString(k) {
+			log.Println("[WARNING]  `memory_ballast` parameter in extensions is deprecated. Please remove it from your configuration.")
+			continue
+		}
+		if secondRegExp.MatchString(k) {
+			out[k] = cfgMap.Get(k)
+			if extSlice, ok := out[k].([]any); ok {
+				ret := removeMemoryBallastStrElementFromSlice(extSlice)
+				out[k] = ret
+			}
+			continue
+		}
+		out[k] = cfgMap.Get(k)
+	}
+	*cfgMap = *confmap.NewFromStringMap(out)
+	return nil
+}

internal/configconverter/remove_memory_ballast_key_test.go

@@ -0,0 +1,76 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Taken from https://github.com/open-telemetry/opentelemetry-collector/blob/v0.66.0/confmap/converter/overwritepropertiesconverter/properties_test.go
+// to prevent breaking changes.
+package configconverter
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
+)
+
+func TestRemoveMemoryBallastConverter_Empty(t *testing.T) {
+	pmp := RemoveMemoryBallastKey{}
+	conf := confmap.NewFromStringMap(map[string]interface{}{"foo": "bar"})
+	assert.NoError(t, pmp.Convert(context.Background(), conf))
+	assert.Equal(t, map[string]interface{}{"foo": "bar"}, conf.ToStringMap())
+}
+
+func TestRemoveMemoryBallastConverter_With_Memory_Ballast(t *testing.T) {
+	cfgMap, err := confmaptest.LoadConf("testdata/with_memory_ballast.yaml")
+	require.NoError(t, err)
+	require.NotNil(t, cfgMap)
+	pmp := RemoveMemoryBallastKey{}
+	assert.NoError(t, pmp.Convert(context.Background(), cfgMap))
+	cfgMapExpected, err := confmaptest.LoadConf("testdata/with_memory_ballast_config_expected.yaml")
+	require.NoError(t, err)
+	assert.Equal(t, cfgMapExpected.ToStringMap(), cfgMap.ToStringMap())
+}
+
+func TestMemoryBallastConverter_Without_Memory_Ballast(t *testing.T) {
+	cfgMap, err := confmaptest.LoadConf("testdata/without_memory_ballast_config.yaml")
+	require.NoError(t, err)
+	require.NotNil(t, cfgMap)
+	pmp := RemoveMemoryBallastKey{}
+	assert.NoError(t, pmp.Convert(context.Background(), cfgMap))
+	assert.Equal(t, cfgMap.ToStringMap(), cfgMap.ToStringMap())
+}
+
+func TestRemoveMemoryBallastStrElementFromSlice(t *testing.T) {
+	originalSlice := []interface{}{"foo", "bar", "memory_ballast", "item2"}
+	actual := removeMemoryBallastStrElementFromSlice(originalSlice)
+	expected := []interface{}{"foo", "bar", "item2"}
+	assert.Equal(t, actual, expected)
+
+	originalSlice1 := []interface{}{"foo", "bar", "foobar", "foobar1"}
+	actual = removeMemoryBallastStrElementFromSlice(originalSlice1)
+	assert.Equal(t, actual, originalSlice1)
+}
+
+func TestRemoveMemoryBallastConverter_With_Only_MemoryBallast_Value(t *testing.T) {
+	cfgMap, err := confmaptest.LoadConf("testdata/with_memory_ballast_only.yaml")
+	require.NoError(t, err)
+	require.NotNil(t, cfgMap)
+	pmp := RemoveMemoryBallastKey{}
+	assert.NoError(t, pmp.Convert(context.Background(), cfgMap))
+	cfgMapExpected, err := confmaptest.LoadConf("testdata/with_memory_ballast_only_expected.yaml")
+	require.NoError(t, err)
+	assert.Equal(t, cfgMapExpected.ToStringMap(), cfgMap.ToStringMap())
+}

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

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

internal/configconverter/testdata/with_memory_ballast.yaml

@@ -0,0 +1,15 @@
+receivers:
+  otlp:
+    protocols:
+      grpc:
+      http:
+
+extensions:
+  memory_ballast:
+    size_mib: 120
+
+service:
+  extensions: [health_check, http_forwarder, zpages, memory_ballast, smartagent]
+  pipelines:
+    metrics:
+      receivers: [otlp]

internal/configconverter/testdata/with_memory_ballast_config_expected.yaml

@@ -0,0 +1,11 @@
+receivers:
+  otlp:
+    protocols:
+      grpc:
+      http:
+
+service:
+  extensions: [health_check, http_forwarder, zpages, smartagent]
+  pipelines:
+    metrics:
+      receivers: [otlp]