Skip to content

Commit

Permalink
addressing previous PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Samiur Arif committed Mar 25, 2024
1 parent 9cfc872 commit e87a1a7
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 29 deletions.
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,11 @@ as a reference.
Remove `ballast_size_mib` parameter from `memory_limiter` and make sure that it's added to
`memory_ballast` extension as `size_mib` parameter instead:

- `memory_ballast` extension has been removed in favor of `GOMEMLIMIT` env var. If `GOMEMLIMIT` is not set, collector will enforce a soft limit automatically.

```yaml
extensions:
memory_ballast:
size_mib: ${SPLUNK_BALLAST_SIZE_MIB}
```

### Using Upstream OpenTelemetry Collector

Expand Down
5 changes: 3 additions & 2 deletions deployments/salt/templates/agent_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ receivers:
static_configs:
- targets: ['127.0.0.1:8888']

extensions:[zpages]
extensions:
zpages:

processors:
memory_limiter:
Expand All @@ -29,7 +30,7 @@ exporters:
verbosity: normal

service:
extensions:[zpages]
extensions: [zpages]
pipelines:
metrics:
receivers: [otlp, prometheus]
Expand Down
27 changes: 14 additions & 13 deletions internal/configconverter/remove_memory_ballast_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import (
// extension config if it exists.
type RemoveMemoryBallastKey struct{}

func removeMemoryBallast(strList []interface{}) []interface{} {
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 ret
return strList
}

func (RemoveMemoryBallastKey) Convert(_ context.Context, cfgMap *confmap.Conf) error {
Expand All @@ -52,21 +52,22 @@ func (RemoveMemoryBallastKey) Convert(_ context.Context, cfgMap *confmap.Conf) e
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.")
} else {
continue
} else if secondRegExp.MatchString(k) {
out[k] = cfgMap.Get(k)
if secondRegExp.MatchString(k) {
temp := out[k]
switch temp := temp.(type) {
default:
continue
case []interface{}:
ret := removeMemoryBallast(temp)
if len(ret) > 0 {
out[k] = ret
}
temp := out[k]
switch temp := temp.(type) {
default:
continue
case []interface{}:
ret := removeMemoryBallastStrElementFromSlice(temp)
if len(ret) > 0 {
out[k] = ret
}
}
continue
}
out[k] = cfgMap.Get(k)
}
*cfgMap = *confmap.NewFromStringMap(out)
return nil
Expand Down
29 changes: 26 additions & 3 deletions internal/configconverter/remove_memory_ballast_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,34 @@ func TestRemoveMemoryBallastConverter_Empty(t *testing.T) {
assert.Equal(t, map[string]interface{}{"foo": "bar"}, conf.ToStringMap())
}

func TestRemoveMemoryBallastConverter(t *testing.T) {
cfgMap, err := confmaptest.LoadConf("testdata/memory_ballast.yaml")
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))
assert.Equal(t, map[string]interface{}{"service": map[string]interface{}{"extensions": []interface{}{"health_check", "http_forwarder", "zpages", "smartagent"}}}, cfgMap.ToStringMap())
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)

}
5 changes: 0 additions & 5 deletions internal/configconverter/testdata/memory_ballast.yaml

This file was deleted.

15 changes: 15 additions & 0 deletions internal/configconverter/testdata/with_memory_ballast.yaml
Original file line number Diff line number Diff line change
@@ -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]
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
receivers:
otlp:
protocols:
grpc:
http:

service:
extensions: [health_check, http_forwarder, zpages, smartagent]
pipelines:
metrics:
receivers: [otlp]
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
extensions:
health_check:
endpoint: "${SPLUNK_LISTEN_INTERFACE}:13133"
http_forwarder:
ingress:
endpoint: "${SPLUNK_LISTEN_INTERFACE}:6060"
egress:
endpoint: "${SPLUNK_API_URL}"
smartagent:
bundleDir: "${SPLUNK_BUNDLE_DIR}"
collectd:
configDir: "${SPLUNK_COLLECTD_DIR}"
zpages:


service:
telemetry:
metrics:
address: "${SPLUNK_LISTEN_INTERFACE}:8888"
extensions: [health_check, http_forwarder, zpages, smartagent]
8 changes: 4 additions & 4 deletions internal/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const (
DefaultGatewayConfig = "/etc/otel/collector/gateway_config.yaml"
DefaultOTLPLinuxConfig = "/etc/otel/collector/otlp_config_linux.yaml"
DefaultConfigDir = "/etc/otel/collector/config.d"
DefaultMemoryBallastPercentage = 10
DefaultMemoryBallastPercentage = 33
DefaultMemoryLimitPercentage = 90
DefaultMemoryTotalMiB = 512
DefaultListenInterface = "0.0.0.0"
Expand Down Expand Up @@ -524,14 +524,14 @@ func envVarAsInt(env string) int {
}

// Validate and set the memory ballast.
// Note this will eventually be removed and here only for backwardcompatibility. Softlimit/GOMemlimit sets memory limit.
// Note this will eventually be removed and here only for backward compatibility. Softlimit/GOMemlimit sets memory limit.
func setMemoryBallast(memTotalSizeMiB int) int {
ballastSize := memTotalSizeMiB * DefaultMemoryBallastPercentage / 100
// Check if the memory ballast is specified via the env var, if so, validate and set properly.
if os.Getenv(BallastEnvVar) != "" {
ballastSize = envVarAsInt(BallastEnvVar)
if 10 > ballastSize {
log.Fatalf("Expected a number greater than 10 for %s env variable but got %d", BallastEnvVar, ballastSize)
if 33 > ballastSize {
log.Fatalf("Expected a number greater than 33 for %s env variable but got %d", BallastEnvVar, ballastSize)
}
}
_ = os.Setenv(BallastEnvVar, strconv.Itoa(ballastSize))
Expand Down

0 comments on commit e87a1a7

Please sign in to comment.