diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go index 9cc4612149..177b3866c1 100644 --- a/cmd/otelcol/main.go +++ b/cmd/otelcol/main.go @@ -79,11 +79,10 @@ func main() { Version: version.Version, } - ballastSizeMib, _ := strconv.Atoi(os.Getenv(ballastEnvVarName)) serviceParams := service.CollectorSettings{ BuildInfo: info, Factories: factories, - ParserProvider: ballastParserProvider(info, ballastSizeMib), + ParserProvider: memLimitRemoverParserProvider(info), } if err := run(serviceParams); err != nil { @@ -91,17 +90,17 @@ func main() { } } -func ballastParserProvider(info component.BuildInfo, ballastSizeMib int) parserprovider.ParserProvider { +// memLimitRemoverParserProvider removes a ballast_size_mib key from a memory +// limiter processor, if it exists. Support for this key will be (or has been) +// removed and will cause the Collector to not start. +func memLimitRemoverParserProvider(info component.BuildInfo) parserprovider.ParserProvider { cfgSourcePP := configprovider.NewConfigSourceParserProvider( newBaseParserProvider(), zap.NewNop(), // The service logger is not available yet, setting it to NoP. info, configsources.Get()..., ) - if ballastSizeMib == 0 { - return cfgSourcePP - } - return configprovider.BallastParserProvider(cfgSourcePP, ballastSizeMib) + return configprovider.NewMemLimitRemoverParserProvider(cfgSourcePP) } // Check whether a string exists in an array of CLI arguments diff --git a/internal/configprovider/ballast_parser_provider.go b/internal/configprovider/ballast_parser_provider.go deleted file mode 100644 index 04f54139bd..0000000000 --- a/internal/configprovider/ballast_parser_provider.go +++ /dev/null @@ -1,93 +0,0 @@ -// 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 configprovider - -import ( - "fmt" - "log" - "strings" - - "go.opentelemetry.io/collector/config/configparser" - "go.opentelemetry.io/collector/service/parserprovider" -) - -const ( - serviceExtensionsPath = "service::extensions" - extensionsMemoryBallastPath = "extensions::memory_ballast::size_mib" - memoryBallast = "memory_ballast" -) - -// ballastParserProvider implements ParserProvider and wraps another ParserProvider, -// adding a memory_ballast extension to the wrapped Otel config if one isn't already -// present. -type ballastParserProvider struct { - pp parserprovider.ParserProvider - sizeMib int -} - -var _ parserprovider.ParserProvider = (*ballastParserProvider)(nil) - -func BallastParserProvider(pp parserprovider.ParserProvider, sizeMib int) parserprovider.ParserProvider { - return &ballastParserProvider{ - pp: pp, - sizeMib: sizeMib, - } -} - -func (bpp ballastParserProvider) Get() (*configparser.ConfigMap, error) { - cfgMap, err := bpp.pp.Get() - if err != nil { - return nil, fmt.Errorf("ballastParserProvider.Get(): %w", err) - } - if hasBallastExtension(cfgMap) { - return cfgMap, nil - } - log.Println("Extension `memory_ballast` not found in config. Adding and enabling a `memory_ballast` extension.") - return cfgMapWithBallastExt(cfgMap, bpp.sizeMib), nil -} - -func hasBallastExtension(cfgMap *configparser.ConfigMap) bool { - for _, key := range cfgMap.AllKeys() { - if key == serviceExtensionsPath { - if exts, ok := cfgMap.Get(key).([]interface{}); ok && extensionsContainMemoryBallast(exts) { - return true - } - } - } - return false -} - -func extensionsContainMemoryBallast(extensions []interface{}) bool { - for _, v := range extensions { - if s, ok := v.(string); ok && isMemoryBallastComponent(s) { - return true - } - } - return false -} - -func isMemoryBallastComponent(extName string) bool { - return extName == memoryBallast || strings.HasPrefix(extName, memoryBallast+"/") -} - -func cfgMapWithBallastExt(parser *configparser.ConfigMap, sizeMib int) *configparser.ConfigMap { - out := configparser.NewParser() - for _, k := range parser.AllKeys() { - out.Set(k, parser.Get(k)) - } - out.Set(extensionsMemoryBallastPath, sizeMib) - out.Set(serviceExtensionsPath, []interface{}{memoryBallast}) - return out -} diff --git a/internal/configprovider/ballast_parser_provider_test.go b/internal/configprovider/ballast_parser_provider_test.go deleted file mode 100644 index dd6df869a0..0000000000 --- a/internal/configprovider/ballast_parser_provider_test.go +++ /dev/null @@ -1,69 +0,0 @@ -// 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 configprovider - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestBallastParserProvider(t *testing.T) { - tests := []struct { - name string - fname string - cfgKey string - expectedVal int - }{ - { - name: "percentage", - fname: "testdata/ballast_percentage.yaml", - cfgKey: "extensions::memory_ballast::size_in_percentage", - expectedVal: 20, - }, - { - name: "mib", - fname: "testdata/ballast_mib.yaml", - cfgKey: "extensions::memory_ballast::size_mib", - expectedVal: 64, - }, - { - name: "custom", - fname: "testdata/ballast_custom.yaml", - cfgKey: "extensions::memory_ballast/foo::size_in_percentage", - expectedVal: 20, - }, - { - name: "missing", - fname: "testdata/ballast_missing.yaml", - cfgKey: "extensions::memory_ballast::size_mib", - expectedVal: 128, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - bpp := &ballastParserProvider{ - pp: &fileParserProvider{FileName: test.fname}, - sizeMib: 128, - } - cfgMap, err := bpp.Get() - require.NoError(t, err) - hasExt := hasBallastExtension(cfgMap) - assert.True(t, hasExt) - assert.Equal(t, test.expectedVal, cfgMap.Get(test.cfgKey)) - }) - } -} diff --git a/internal/configprovider/mem_limit_ballast_remover_parser_provider.go b/internal/configprovider/mem_limit_ballast_remover_parser_provider.go new file mode 100644 index 0000000000..5908b98907 --- /dev/null +++ b/internal/configprovider/mem_limit_ballast_remover_parser_provider.go @@ -0,0 +1,65 @@ +// 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 configprovider + +import ( + "fmt" + "log" + "regexp" + + "go.opentelemetry.io/collector/config/configparser" + "go.opentelemetry.io/collector/service/parserprovider" +) + +var ballastKeyRegexp *regexp.Regexp + +func init() { + const expr = "processors::memory_limiter(/\\w+)?::ballast_size_mib" + ballastKeyRegexp, _ = regexp.Compile(expr) +} + +// memLimitBallastRemoverParserProvider implements ParserProvider, wraps a +// ParserProvider, and removes any ballast_size_mib key from the memory_limiter +// processor, if one exists, from the wrapped config. This is to ensure that the +// Collector will still start when support for this key gets removed. +type memLimitBallastRemoverParserProvider struct { + pp parserprovider.ParserProvider +} + +var _ parserprovider.ParserProvider = (*memLimitBallastRemoverParserProvider)(nil) + +func NewMemLimitRemoverParserProvider(pp parserprovider.ParserProvider) parserprovider.ParserProvider { + return &memLimitBallastRemoverParserProvider{pp: pp} +} + +func (mpp memLimitBallastRemoverParserProvider) Get() (*configparser.ConfigMap, error) { + cfgMap, err := mpp.pp.Get() + if err != nil { + return nil, fmt.Errorf("memLimitBallastRemoverParserProvider.Get(): %w", err) + } + out := configparser.NewParser() + for _, k := range cfgMap.AllKeys() { + if isMemLimitBallastKey(k) { + log.Println("Deprecated memory_limiter processor `ballast_size_mib` key found. Removing from config.") + } else { + out.Set(k, cfgMap.Get(k)) + } + } + return out, nil +} + +func isMemLimitBallastKey(k string) bool { + return ballastKeyRegexp.MatchString(k) +} diff --git a/internal/configprovider/mem_limit_ballast_remover_parser_provider_test.go b/internal/configprovider/mem_limit_ballast_remover_parser_provider_test.go new file mode 100644 index 0000000000..6699d9ffed --- /dev/null +++ b/internal/configprovider/mem_limit_ballast_remover_parser_provider_test.go @@ -0,0 +1,58 @@ +// 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 configprovider + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsMemLimitBallastKey(t *testing.T) { + assert.False(t, isMemLimitBallastKey("processors::ballast_size_mib")) + assert.True(t, isMemLimitBallastKey("processors::memory_limiter::ballast_size_mib")) + assert.True(t, isMemLimitBallastKey("processors::memory_limiter/foo::ballast_size_mib")) +} + +func TestMemLimitBallastRemoverPP(t *testing.T) { + tests := []struct { + name string + fname string + key string + }{ + { + name: "default", + fname: "testdata/ballast_mem_limiter.yaml", + key: "processors::memory_limiter::ballast_size_mib", + }, + { + name: "custom", + fname: "testdata/ballast_mem_limiter_custom.yaml", + key: "processors::memory_limiter/foo::ballast_size_mib", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pp := &memLimitBallastRemoverParserProvider{ + &fileParserProvider{FileName: test.fname}, + } + cfgMap, err := pp.Get() + require.NoError(t, err) + b := cfgMap.IsSet(test.key) + assert.False(t, b) + }) + } +} diff --git a/internal/configprovider/testdata/ballast_custom.yaml b/internal/configprovider/testdata/ballast_custom.yaml deleted file mode 100644 index c4000f0d3b..0000000000 --- a/internal/configprovider/testdata/ballast_custom.yaml +++ /dev/null @@ -1,22 +0,0 @@ -extensions: - memory_ballast/foo: - size_in_percentage: 20 -receivers: - hostmetrics: - collection_interval: 1s - scrapers: - cpu: -exporters: - logging: - loglevel: info - sampling_initial: 2 - sampling_thereafter: 500 -service: - pipelines: - metrics: - receivers: - - hostmetrics - exporters: - - logging - extensions: - - memory_ballast/foo diff --git a/internal/configprovider/testdata/ballast_mib.yaml b/internal/configprovider/testdata/ballast_mem_limiter.yaml similarity index 68% rename from internal/configprovider/testdata/ballast_mib.yaml rename to internal/configprovider/testdata/ballast_mem_limiter.yaml index 6e01605930..7ccd5fb8b0 100644 --- a/internal/configprovider/testdata/ballast_mib.yaml +++ b/internal/configprovider/testdata/ballast_mem_limiter.yaml @@ -6,6 +6,12 @@ receivers: collection_interval: 1s scrapers: cpu: +processors: + memory_limiter: + check_interval: 1s + limit_mib: 4000 + spike_limit_mib: 800 + ballast_size_mib: 64 exporters: logging: loglevel: info @@ -16,6 +22,8 @@ service: metrics: receivers: - hostmetrics + processors: + - memory_limiter exporters: - logging extensions: diff --git a/internal/configprovider/testdata/ballast_percentage.yaml b/internal/configprovider/testdata/ballast_mem_limiter_custom.yaml similarity index 64% rename from internal/configprovider/testdata/ballast_percentage.yaml rename to internal/configprovider/testdata/ballast_mem_limiter_custom.yaml index bc9ef31fd9..b97f16435b 100644 --- a/internal/configprovider/testdata/ballast_percentage.yaml +++ b/internal/configprovider/testdata/ballast_mem_limiter_custom.yaml @@ -1,11 +1,17 @@ extensions: memory_ballast: - size_in_percentage: 20 + size_mib: 64 receivers: hostmetrics: collection_interval: 1s scrapers: cpu: +processors: + memory_limiter/foo: + check_interval: 1s + limit_mib: 4000 + spike_limit_mib: 800 + ballast_size_mib: 64 exporters: logging: loglevel: info @@ -16,6 +22,8 @@ service: metrics: receivers: - hostmetrics + processors: + - memory_limiter/foo exporters: - logging extensions: diff --git a/internal/configprovider/testdata/ballast_missing.yaml b/internal/configprovider/testdata/ballast_missing.yaml deleted file mode 100644 index 57e9fb249e..0000000000 --- a/internal/configprovider/testdata/ballast_missing.yaml +++ /dev/null @@ -1,17 +0,0 @@ -receivers: - hostmetrics: - collection_interval: 1s - scrapers: - cpu: -exporters: - logging: - loglevel: info - sampling_initial: 2 - sampling_thereafter: 500 -service: - pipelines: - metrics: - receivers: - - hostmetrics - exporters: - - logging