From 4938724e4481beb7af977292886e90ed3fb1fb82 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 16 Mar 2022 13:58:50 +0100 Subject: [PATCH 01/12] Creating check if services are configured properly Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports.go | 7 +- .../adapters/config_to_ports_test.go | 11 +++ pkg/collector/adapters/config_to_probe.go | 35 ++++---- .../adapters/config_to_probe_test.go | 16 ++-- pkg/collector/adapters/config_validate.go | 85 +++++++++++++++++++ .../adapters/config_validate_test.go | 68 +++++++++++++++ 6 files changed, 196 insertions(+), 26 deletions(-) create mode 100644 pkg/collector/adapters/config_validate.go create mode 100644 pkg/collector/adapters/config_validate_test.go diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go index a3fc2822cb..deafb2715b 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/pkg/collector/adapters/config_to_ports.go @@ -16,6 +16,7 @@ package adapters import ( "errors" + "fmt" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -51,7 +52,11 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ if !ok { return nil, ErrNoReceivers } - + recEnabled, err := ConfigValidate(logger, config) + if err != nil { + return nil, err + } + fmt.Printf("%v", recEnabled) receivers, ok := receiversProperty.(map[interface{}]interface{}) if !ok { return nil, ErrReceiversNotAMap diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go index 65d4d15799..e490d4f481 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/pkg/collector/adapters/config_to_ports_test.go @@ -63,6 +63,17 @@ func TestExtractPortsFromConfig(t *testing.T) { zipkin: zipkin/2: endpoint: 0.0.0.0:33333 +service: + pipelines: + metrics: + receivers: [examplereceiver, examplereceiver/settings] + exporters: [logging] + metrics/1: + receivers: [jaeger, jaeger/custom] + exporters: [logging] + metrics/1: + receivers: [otlp, otlp/2, zipkin] + exporters: [logging] ` // prepare diff --git a/pkg/collector/adapters/config_to_probe.go b/pkg/collector/adapters/config_to_probe.go index 10dfa45cd8..3bdbb0d71d 100644 --- a/pkg/collector/adapters/config_to_probe.go +++ b/pkg/collector/adapters/config_to_probe.go @@ -15,6 +15,7 @@ package adapters import ( + "errors" "strings" @@ -23,18 +24,18 @@ import ( ) var ( - errNoService = errors.New("no service available as part of the configuration") - errNoExtensions = errors.New("no extensions available as part of the configuration") + ErrNoService = errors.New("no service available as part of the configuration") + ErrNoExtensions = errors.New("no extensions available as part of the configuration") - errServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") - errExtensionsNotAMap = errors.New("extensions property in the configuration doesn't contain valid extensions") + ErrServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") + ErrExtensionsNotAMap = errors.New("extensions property in the configuration doesn't contain valid extensions") - errNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") + ErrNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") - errNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") + ErrNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") - errServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") - errNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") + ErrServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") + ErrNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") ) type probeConfiguration struct { @@ -51,21 +52,21 @@ const ( func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, error) { serviceProperty, ok := config["service"] if !ok { - return nil, errNoService + return nil, ErrNoService } service, ok := serviceProperty.(map[interface{}]interface{}) if !ok { - return nil, errServiceNotAMap + return nil, ErrServiceNotAMap } serviceExtensionsProperty, ok := service["extensions"] if !ok { - return nil, errNoServiceExtensions + return nil, ErrNoServiceExtensions } serviceExtensions, ok := serviceExtensionsProperty.([]interface{}) if !ok { - return nil, errServiceExtensionsNotSlice + return nil, ErrServiceExtensionsNotSlice } healthCheckServiceExtensions := make([]string, 0) for _, ext := range serviceExtensions { @@ -76,16 +77,16 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } if len(healthCheckServiceExtensions) == 0 { - return nil, errNoServiceExtensionHealthCheck + return nil, ErrNoServiceExtensionHealthCheck } extensionsProperty, ok := config["extensions"] if !ok { - return nil, errNoExtensions + return nil, ErrNoExtensions } extensions, ok := extensionsProperty.(map[interface{}]interface{}) if !ok { - return nil, errExtensionsNotAMap + return nil, ErrExtensionsNotAMap } // in the event of multiple health_check service extensions defined, we arbitrarily take the first one found for _, healthCheckForProbe := range healthCheckServiceExtensions { @@ -95,7 +96,7 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } } - return nil, errNoExtensionHealthCheck + return nil, ErrNoExtensionHealthCheck } func createProbeFromExtension(extension interface{}) (*corev1.Probe, error) { @@ -155,4 +156,4 @@ func extractPortFromExtensionConfig(cfg map[interface{}]interface{}) intstr.IntO func defaultHealthCheckEndpoint() intstr.IntOrString { return intstr.FromInt(defaultHealthCheckPort) -} +} \ No newline at end of file diff --git a/pkg/collector/adapters/config_to_probe_test.go b/pkg/collector/adapters/config_to_probe_test.go index 65a6a609b3..8ee41d5adb 100644 --- a/pkg/collector/adapters/config_to_probe_test.go +++ b/pkg/collector/adapters/config_to_probe_test.go @@ -130,47 +130,47 @@ func TestConfigToProbeShouldErrorIf(t *testing.T) { pprof: service: extensions: [health_check]`, - expectedErr: errNoExtensionHealthCheck, + expectedErr: ErrNoExtensionHealthCheck, }, { desc: "BadlyFormattedExtensions", config: `extensions: [hi] service: extensions: [health_check]`, - expectedErr: errExtensionsNotAMap, + expectedErr: ErrExtensionsNotAMap, }, { desc: "NoExtensions", config: `service: extensions: [health_check]`, - expectedErr: errNoExtensions, + expectedErr: ErrNoExtensions, }, { desc: "NoHealthCheckInServiceExtensions", config: `service: extensions: [pprof]`, - expectedErr: errNoServiceExtensionHealthCheck, + expectedErr: ErrNoServiceExtensionHealthCheck, }, { desc: "BadlyFormattedServiceExtensions", config: `service: extensions: this: should-not-be-a-map`, - expectedErr: errServiceExtensionsNotSlice, + expectedErr: ErrServiceExtensionsNotSlice, }, { desc: "NoServiceExtensions", config: `service: pipelines: traces: receivers: [otlp]`, - expectedErr: errNoServiceExtensions, + expectedErr: ErrNoServiceExtensions, }, { desc: "BadlyFormattedService", config: `extensions: health_check: service: [hi]`, - expectedErr: errServiceNotAMap, + expectedErr: ErrServiceNotAMap, }, { desc: "NoService", config: `extensions: health_check:`, - expectedErr: errNoService, + expectedErr: ErrNoService, }, } diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go new file mode 100644 index 0000000000..e556d8a14b --- /dev/null +++ b/pkg/collector/adapters/config_validate.go @@ -0,0 +1,85 @@ +// 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 adapters + +import ( + "errors" + "fmt" + + "github.com/go-logr/logr" +) + +var ( + ErrNoPipeline = errors.New("no pipeline available as part of the configuration") +) + +func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map[string]bool, error) { + cfgReceivers, ok := config["receivers"] + if !ok { + return nil, ErrNoReceivers + } + receivers, ok := cfgReceivers.(map[interface{}]interface{}) + if !ok { + return nil, ErrReceiversNotAMap + } + availableReceivers := map[string]bool{} + + for recvID, recvCfg := range receivers { + availableReceivers[recvID.(string)] = false + receiver, ok := recvCfg.(map[interface{}]interface{}) + if !ok { + return nil, fmt.Errorf("receiver %q has invalid configuration: %q", recvID, receiver) + } + } + + cfgService, ok := config["service"].(map[interface{}]interface{}) + if !ok { + return nil, ErrNoService + } + + pipeline, ok := cfgService["pipelines"].(map[interface{}]interface{}) + if !ok { + return nil, ErrNoPipeline + } + availablePipelines := map[string]bool{} + + for pipID := range pipeline { + availablePipelines[pipID.(string)] = true + } + + if len(pipeline) > 0 { + for pipelineID, pipelineCfg := range pipeline { + //Condition will get information if there are multiple configured pipelines. + if len(pipelineID.(string)) > 0 { + pipelineDesc, ok := pipelineCfg.(map[interface{}]interface{}) + if !ok { + return nil, fmt.Errorf("pipeline was not properly configured") + } + for pipSpecID, pipSpecCfg := range pipelineDesc { + if pipSpecID.(string) == "receivers" { + receiversList, ok := pipSpecCfg.([]interface{}) + if !ok { + return nil, fmt.Errorf("no receivers on pipeline configuration %q", receiversList...) + } + for _, recKey := range receiversList { + availableReceivers[recKey.(string)] = true + } + } + } + } + } + } + return availableReceivers, nil +} diff --git a/pkg/collector/adapters/config_validate_test.go b/pkg/collector/adapters/config_validate_test.go new file mode 100644 index 0000000000..c38288b39c --- /dev/null +++ b/pkg/collector/adapters/config_validate_test.go @@ -0,0 +1,68 @@ +// 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 adapters + +import ( + "testing" + + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var logger = logf.Log.WithName("unit-tests") + +func TestConfigValidate(t *testing.T) { + // prepare + + // First Test - Exporters + configStr := ` +receivers: + httpd/mtls: + protocols: + http: + endpoint: mysite.local:55690 + jaeger: + protocols: + grpc: + prometheus: + protocols: + grpc: + +processors: + +exporters: + logging: + +service: + pipelines: + metrics: + receivers: [httpd/mtls, jaeger] + exporters: [logging] + metrics/1: + receivers: [httpd/mtls, jaeger] + exporters: [logging] +` + // // prepare + config, err := ConfigFromString(configStr) + require.NoError(t, err) + require.NotEmpty(t, config) + + // test + check, err := ConfigValidate(logger, config) + assert.NoError(t, err) + require.NotEmpty(t, check) +} From 735af3a86840b58dac1bb025ccb01037be716c81 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 16 Mar 2022 14:01:26 +0100 Subject: [PATCH 02/12] Creating check if services are configured properly Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_probe.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/collector/adapters/config_to_probe.go b/pkg/collector/adapters/config_to_probe.go index 3bdbb0d71d..93a5b9050f 100644 --- a/pkg/collector/adapters/config_to_probe.go +++ b/pkg/collector/adapters/config_to_probe.go @@ -15,7 +15,6 @@ package adapters import ( - "errors" "strings" @@ -156,4 +155,4 @@ func extractPortFromExtensionConfig(cfg map[interface{}]interface{}) intstr.IntO func defaultHealthCheckEndpoint() intstr.IntOrString { return intstr.FromInt(defaultHealthCheckPort) -} \ No newline at end of file +} From c023719db500faca48872e4c49c2b5ba610ccafc Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Thu, 17 Mar 2022 10:12:11 +0100 Subject: [PATCH 03/12] Adding recEnabled logic Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports.go | 7 +++-- .../adapters/config_to_ports_test.go | 9 ++---- pkg/collector/adapters/config_to_probe.go | 28 +++++++++---------- .../adapters/config_to_probe_test.go | 14 +++++----- pkg/collector/adapters/config_validate.go | 19 +++++++------ 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go index deafb2715b..39dd7e7623 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/pkg/collector/adapters/config_to_ports.go @@ -16,7 +16,6 @@ package adapters import ( "errors" - "fmt" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -56,7 +55,6 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ if err != nil { return nil, err } - fmt.Printf("%v", recEnabled) receivers, ok := receiversProperty.(map[interface{}]interface{}) if !ok { return nil, ErrReceiversNotAMap @@ -64,6 +62,11 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ ports := []corev1.ServicePort{} for key, val := range receivers { + // This check will pass only the enabled receivers, + // then only the related ports will be opened. + if !recEnabled[key] { + continue + } receiver, ok := val.(map[interface{}]interface{}) if !ok { logger.Info("receiver doesn't seem to be a map of properties", "receiver", key) diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go index e490d4f481..e37af480a8 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/pkg/collector/adapters/config_to_ports_test.go @@ -62,7 +62,7 @@ func TestExtractPortsFromConfig(t *testing.T) { endpoint: 0.0.0.0:55555 zipkin: zipkin/2: - endpoint: 0.0.0.0:33333 + endpoint: 0.0.0.0:33333 service: pipelines: metrics: @@ -71,7 +71,7 @@ service: metrics/1: receivers: [jaeger, jaeger/custom] exporters: [logging] - metrics/1: + metrics/2: receivers: [otlp, otlp/2, zipkin] exporters: [logging] ` @@ -84,7 +84,7 @@ service: // test ports, err := adapters.ConfigToReceiverPorts(logger, config) assert.NoError(t, err) - assert.Len(t, ports, 12) + assert.Len(t, ports, 11) // verify expectedPorts := map[int32]bool{} @@ -98,7 +98,6 @@ service: expectedPorts[int32(55681)] = false expectedPorts[int32(55555)] = false expectedPorts[int32(9411)] = false - expectedPorts[int32(33333)] = false expectedNames := map[string]bool{} expectedNames["examplereceiver"] = false @@ -112,7 +111,6 @@ service: expectedNames["otlp-http-legacy"] = false expectedNames["otlp-2-grpc"] = false expectedNames["zipkin"] = false - expectedNames["zipkin-2"] = false expectedAppProtocols := map[string]string{} expectedAppProtocols["otlp-grpc"] = "grpc" @@ -122,7 +120,6 @@ service: expectedAppProtocols["jaeger-grpc"] = "grpc" expectedAppProtocols["otlp-2-grpc"] = "grpc" expectedAppProtocols["zipkin"] = "http" - expectedAppProtocols["zipkin-2"] = "http" // make sure we only have the ports in the set for _, port := range ports { diff --git a/pkg/collector/adapters/config_to_probe.go b/pkg/collector/adapters/config_to_probe.go index 93a5b9050f..7d5c57738a 100644 --- a/pkg/collector/adapters/config_to_probe.go +++ b/pkg/collector/adapters/config_to_probe.go @@ -24,17 +24,17 @@ import ( var ( ErrNoService = errors.New("no service available as part of the configuration") - ErrNoExtensions = errors.New("no extensions available as part of the configuration") + errNoExtensions = errors.New("no extensions available as part of the configuration") - ErrServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") - ErrExtensionsNotAMap = errors.New("extensions property in the configuration doesn't contain valid extensions") + errServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") + errExtensionsNotAMap = errors.New("extensions property in the configuration doesn't contain valid extensions") - ErrNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") + errNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") - ErrNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") + errNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") - ErrServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") - ErrNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") + errServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") + errNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") ) type probeConfiguration struct { @@ -55,17 +55,17 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } service, ok := serviceProperty.(map[interface{}]interface{}) if !ok { - return nil, ErrServiceNotAMap + return nil, errServiceNotAMap } serviceExtensionsProperty, ok := service["extensions"] if !ok { - return nil, ErrNoServiceExtensions + return nil, errNoServiceExtensions } serviceExtensions, ok := serviceExtensionsProperty.([]interface{}) if !ok { - return nil, ErrServiceExtensionsNotSlice + return nil, errServiceExtensionsNotSlice } healthCheckServiceExtensions := make([]string, 0) for _, ext := range serviceExtensions { @@ -76,16 +76,16 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } if len(healthCheckServiceExtensions) == 0 { - return nil, ErrNoServiceExtensionHealthCheck + return nil, errNoServiceExtensionHealthCheck } extensionsProperty, ok := config["extensions"] if !ok { - return nil, ErrNoExtensions + return nil, errNoExtensions } extensions, ok := extensionsProperty.(map[interface{}]interface{}) if !ok { - return nil, ErrExtensionsNotAMap + return nil, errExtensionsNotAMap } // in the event of multiple health_check service extensions defined, we arbitrarily take the first one found for _, healthCheckForProbe := range healthCheckServiceExtensions { @@ -95,7 +95,7 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } } - return nil, ErrNoExtensionHealthCheck + return nil, errNoExtensionHealthCheck } func createProbeFromExtension(extension interface{}) (*corev1.Probe, error) { diff --git a/pkg/collector/adapters/config_to_probe_test.go b/pkg/collector/adapters/config_to_probe_test.go index 8ee41d5adb..4feda12a94 100644 --- a/pkg/collector/adapters/config_to_probe_test.go +++ b/pkg/collector/adapters/config_to_probe_test.go @@ -130,42 +130,42 @@ func TestConfigToProbeShouldErrorIf(t *testing.T) { pprof: service: extensions: [health_check]`, - expectedErr: ErrNoExtensionHealthCheck, + expectedErr: errNoExtensionHealthCheck, }, { desc: "BadlyFormattedExtensions", config: `extensions: [hi] service: extensions: [health_check]`, - expectedErr: ErrExtensionsNotAMap, + expectedErr: errExtensionsNotAMap, }, { desc: "NoExtensions", config: `service: extensions: [health_check]`, - expectedErr: ErrNoExtensions, + expectedErr: errNoExtensions, }, { desc: "NoHealthCheckInServiceExtensions", config: `service: extensions: [pprof]`, - expectedErr: ErrNoServiceExtensionHealthCheck, + expectedErr: errNoServiceExtensionHealthCheck, }, { desc: "BadlyFormattedServiceExtensions", config: `service: extensions: this: should-not-be-a-map`, - expectedErr: ErrServiceExtensionsNotSlice, + expectedErr: errServiceExtensionsNotSlice, }, { desc: "NoServiceExtensions", config: `service: pipelines: traces: receivers: [otlp]`, - expectedErr: ErrNoServiceExtensions, + expectedErr: errNoServiceExtensions, }, { desc: "BadlyFormattedService", config: `extensions: health_check: service: [hi]`, - expectedErr: ErrServiceNotAMap, + expectedErr: errServiceNotAMap, }, { desc: "NoService", config: `extensions: diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go index e556d8a14b..c91f3cfc4d 100644 --- a/pkg/collector/adapters/config_validate.go +++ b/pkg/collector/adapters/config_validate.go @@ -22,10 +22,12 @@ import ( ) var ( - ErrNoPipeline = errors.New("no pipeline available as part of the configuration") + errNoPipeline = errors.New("no pipeline available as part of the configuration") ) -func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map[string]bool, error) { +//Following Otel Doc: Configuring a receiver does not enable it. The receivers are enabled via pipelines within the service section. +//ConfigValidate returns all receivers, setting them as true for enabled and false for non-configured services in pipeline set. +func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map[interface{}]bool, error) { cfgReceivers, ok := config["receivers"] if !ok { return nil, ErrNoReceivers @@ -34,14 +36,11 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map if !ok { return nil, ErrReceiversNotAMap } - availableReceivers := map[string]bool{} + availableReceivers := map[interface{}]bool{} - for recvID, recvCfg := range receivers { + for recvID := range receivers { + //Getting all receivers present in the receivers section and setting them to false. availableReceivers[recvID.(string)] = false - receiver, ok := recvCfg.(map[interface{}]interface{}) - if !ok { - return nil, fmt.Errorf("receiver %q has invalid configuration: %q", recvID, receiver) - } } cfgService, ok := config["service"].(map[interface{}]interface{}) @@ -51,11 +50,12 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map pipeline, ok := cfgService["pipelines"].(map[interface{}]interface{}) if !ok { - return nil, ErrNoPipeline + return nil, errNoPipeline } availablePipelines := map[string]bool{} for pipID := range pipeline { + //Getting all the available pipelines. availablePipelines[pipID.(string)] = true } @@ -73,6 +73,7 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map if !ok { return nil, fmt.Errorf("no receivers on pipeline configuration %q", receiversList...) } + // All enabled receivers will be set as true for _, recKey := range receiversList { availableReceivers[recKey.(string)] = true } From d9f63779c474559dd79748692de33a4548815462 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Thu, 17 Mar 2022 10:16:09 +0100 Subject: [PATCH 04/12] Fixing Lint Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go index 39dd7e7623..7697b573e9 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/pkg/collector/adapters/config_to_ports.go @@ -63,7 +63,7 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ ports := []corev1.ServicePort{} for key, val := range receivers { // This check will pass only the enabled receivers, - // then only the related ports will be opened. + // then only the related ports will be opened. if !recEnabled[key] { continue } From 5846a6bed14f6e56a3b7571b51e984b42b849cb0 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Thu, 17 Mar 2022 11:34:23 +0100 Subject: [PATCH 05/12] Fixing Service test Signed-off-by: Yuri Sa --- pkg/collector/testdata/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/testdata/test.yaml b/pkg/collector/testdata/test.yaml index a14808f3bc..c920cee82d 100644 --- a/pkg/collector/testdata/test.yaml +++ b/pkg/collector/testdata/test.yaml @@ -17,6 +17,6 @@ exporters: service: pipelines: metrics: - receivers: [prometheus] + receivers: [prometheus, jaeger] processors: [] exporters: [logging] \ No newline at end of file From a118f07bef71c91a7d3824f4b27abe1b52c710b8 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Fri, 18 Mar 2022 07:55:12 +0100 Subject: [PATCH 06/12] Changing tests Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go index e37af480a8..7b52230275 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/pkg/collector/adapters/config_to_ports_test.go @@ -183,11 +183,11 @@ func TestInvalidReceivers(t *testing.T) { }{ { "receiver isn't a map", - "receivers:\n some-receiver: string", + "receivers:\n some-receiver: string\nservice:\n pipelines:\n metrics:\n receivers: [some-receiver]", }, { "receiver's endpoint isn't string", - "receivers:\n some-receiver:\n endpoint: 123", + "receivers:\n some-receiver:\n endpoint: 123\nservice:\n pipelines:\n metrics:\n receivers: [some-receiver]", }, } { t.Run(tt.desc, func(t *testing.T) { @@ -222,6 +222,11 @@ func TestParserFailed(t *testing.T) { "receivers": map[interface{}]interface{}{ "mock": map[interface{}]interface{}{}, }, + "service": map[interface{}]interface{}{ + "pipelines": map[interface{}]interface{}{}, + "metrics": map[interface{}]interface{}{}, + "receivers": []string{"mock"}, + }, } // test From ceac9c1ef91b57fcdfdf546e28f84526d5213cd6 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Tue, 22 Mar 2022 12:25:55 +0100 Subject: [PATCH 07/12] Changed mock test Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go index 7b52230275..f5766dc67b 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/pkg/collector/adapters/config_to_ports_test.go @@ -220,12 +220,15 @@ func TestParserFailed(t *testing.T) { config := map[interface{}]interface{}{ "receivers": map[interface{}]interface{}{ - "mock": map[interface{}]interface{}{}, + "mock": map[string]interface{}{}, }, "service": map[interface{}]interface{}{ - "pipelines": map[interface{}]interface{}{}, - "metrics": map[interface{}]interface{}{}, - "receivers": []string{"mock"}, + "pipelines": map[interface{}]interface{}{ + "metrics": map[interface{}]interface{}{ + "receivers": []interface{}{"mock"}, + + }, + }, }, } From ebf4fb6f3199321c2b58cabd380c95a571a8faca Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Tue, 22 Mar 2022 12:50:19 +0100 Subject: [PATCH 08/12] Changed configmap test + lint Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports_test.go | 5 ++--- pkg/collector/reconcile/configmap_test.go | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go index f5766dc67b..10f8109c54 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/pkg/collector/adapters/config_to_ports_test.go @@ -220,13 +220,12 @@ func TestParserFailed(t *testing.T) { config := map[interface{}]interface{}{ "receivers": map[interface{}]interface{}{ - "mock": map[string]interface{}{}, + "mock": map[string]interface{}{}, }, "service": map[interface{}]interface{}{ "pipelines": map[interface{}]interface{}{ - "metrics": map[interface{}]interface{}{ + "metrics": map[interface{}]interface{}{ "receivers": []interface{}{"mock"}, - }, }, }, diff --git a/pkg/collector/reconcile/configmap_test.go b/pkg/collector/reconcile/configmap_test.go index 607c651eaa..b41a8ba6ac 100644 --- a/pkg/collector/reconcile/configmap_test.go +++ b/pkg/collector/reconcile/configmap_test.go @@ -62,7 +62,7 @@ exporters: service: pipelines: metrics: - receivers: [prometheus] + receivers: [prometheus, jaeger] processors: [] exporters: [logging]`, } @@ -112,6 +112,7 @@ service: processors: [] receivers: - prometheus + - jaeger `, } From c29fb4e9977e96a64cf5f4495ce44662c05f5ebe Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 23 Mar 2022 13:12:48 +0100 Subject: [PATCH 09/12] Changed scope of errNoService Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_probe.go | 4 ++-- pkg/collector/adapters/config_to_probe_test.go | 2 +- pkg/collector/adapters/config_validate.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/collector/adapters/config_to_probe.go b/pkg/collector/adapters/config_to_probe.go index 7d5c57738a..10dfa45cd8 100644 --- a/pkg/collector/adapters/config_to_probe.go +++ b/pkg/collector/adapters/config_to_probe.go @@ -23,7 +23,7 @@ import ( ) var ( - ErrNoService = errors.New("no service available as part of the configuration") + errNoService = errors.New("no service available as part of the configuration") errNoExtensions = errors.New("no extensions available as part of the configuration") errServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") @@ -51,7 +51,7 @@ const ( func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, error) { serviceProperty, ok := config["service"] if !ok { - return nil, ErrNoService + return nil, errNoService } service, ok := serviceProperty.(map[interface{}]interface{}) if !ok { diff --git a/pkg/collector/adapters/config_to_probe_test.go b/pkg/collector/adapters/config_to_probe_test.go index 4feda12a94..65a6a609b3 100644 --- a/pkg/collector/adapters/config_to_probe_test.go +++ b/pkg/collector/adapters/config_to_probe_test.go @@ -170,7 +170,7 @@ service: [hi]`, desc: "NoService", config: `extensions: health_check:`, - expectedErr: ErrNoService, + expectedErr: errNoService, }, } diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go index c91f3cfc4d..80ecc1bbfe 100644 --- a/pkg/collector/adapters/config_validate.go +++ b/pkg/collector/adapters/config_validate.go @@ -45,7 +45,7 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map cfgService, ok := config["service"].(map[interface{}]interface{}) if !ok { - return nil, ErrNoService + return nil, errNoService } pipeline, ok := cfgService["pipelines"].(map[interface{}]interface{}) From c03d651a8adce173b2c28587065d05e23dbe864d Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Fri, 1 Apr 2022 14:00:42 +0200 Subject: [PATCH 10/12] Include safe cast and Remove non-active receivers from return Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports.go | 2 +- pkg/collector/adapters/config_validate.go | 46 ++++++++++++++++--- .../adapters/config_validate_test.go | 45 +++++++++++++++++- 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go index 7697b573e9..998fbfedf6 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/pkg/collector/adapters/config_to_ports.go @@ -51,7 +51,7 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ if !ok { return nil, ErrNoReceivers } - recEnabled, err := ConfigValidate(logger, config) + recEnabled, err := GetEnabledReceivers(logger, config) if err != nil { return nil, err } diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go index 80ecc1bbfe..47c319dde1 100644 --- a/pkg/collector/adapters/config_validate.go +++ b/pkg/collector/adapters/config_validate.go @@ -27,7 +27,7 @@ var ( //Following Otel Doc: Configuring a receiver does not enable it. The receivers are enabled via pipelines within the service section. //ConfigValidate returns all receivers, setting them as true for enabled and false for non-configured services in pipeline set. -func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map[interface{}]bool, error) { +func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) (map[interface{}]bool, error) { cfgReceivers, ok := config["receivers"] if !ok { return nil, ErrNoReceivers @@ -39,8 +39,14 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map availableReceivers := map[interface{}]bool{} for recvID := range receivers { + + //Safe Cast + receiverID, ok := recvID.(string) + if !ok { + return nil, fmt.Errorf("ReceiverID is not a string: %v", receiverID) + } //Getting all receivers present in the receivers section and setting them to false. - availableReceivers[recvID.(string)] = false + availableReceivers[receiverID] = false } cfgService, ok := config["service"].(map[interface{}]interface{}) @@ -55,14 +61,24 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map availablePipelines := map[string]bool{} for pipID := range pipeline { + //Safe Cast + pipelineID, ok := pipID.(string) + if !ok { + return nil, fmt.Errorf("PipelineID is not a string: %v", pipelineID) + } //Getting all the available pipelines. - availablePipelines[pipID.(string)] = true + availablePipelines[pipelineID] = true } if len(pipeline) > 0 { for pipelineID, pipelineCfg := range pipeline { + //Safe Cast + pipelineV, ok := pipelineID.(string) + if !ok { + return nil, fmt.Errorf("PipelineID is not a string: %v", pipelineV) + } //Condition will get information if there are multiple configured pipelines. - if len(pipelineID.(string)) > 0 { + if len(pipelineV) > 0 { pipelineDesc, ok := pipelineCfg.(map[interface{}]interface{}) if !ok { return nil, fmt.Errorf("pipeline was not properly configured") @@ -73,9 +89,25 @@ func ConfigValidate(logger logr.Logger, config map[interface{}]interface{}) (map if !ok { return nil, fmt.Errorf("no receivers on pipeline configuration %q", receiversList...) } - // All enabled receivers will be set as true - for _, recKey := range receiversList { - availableReceivers[recKey.(string)] = true + // If receiversList is empty means that we haven't any enabled Receiver. + if len(receiversList) == 0 { + availableReceivers = nil + } else { + // All enabled receivers will be set as true + for _, recKey := range receiversList { + //Safe Cast + receiverKey, ok := recKey.(string) + if !ok { + return nil, fmt.Errorf("ReceiverKey is not a string: %v", receiverKey) + } + availableReceivers[receiverKey] = true + } + } + //Removing all non-enabled receivers + for recID, recKey := range availableReceivers { + if !(recKey) { + delete(availableReceivers, recID) + } } } } diff --git a/pkg/collector/adapters/config_validate_test.go b/pkg/collector/adapters/config_validate_test.go index c38288b39c..03a8bb1e6d 100644 --- a/pkg/collector/adapters/config_validate_test.go +++ b/pkg/collector/adapters/config_validate_test.go @@ -62,7 +62,50 @@ service: require.NotEmpty(t, config) // test - check, err := ConfigValidate(logger, config) + check, err := GetEnabledReceivers(logger, config) assert.NoError(t, err) require.NotEmpty(t, check) } + +func TestEmptyEnabledReceivers(t *testing.T) { + // prepare + + // First Test - Exporters + configStr := ` +receivers: + httpd/mtls: + protocols: + http: + endpoint: mysite.local:55690 + jaeger: + protocols: + grpc: + prometheus: + protocols: + grpc: + +processors: + +exporters: + logging: + +service: + pipelines: + metrics: + receivers: [] + exporters: [] + metrics/1: + receivers: [] + exporters: [] +` + // // prepare + config, err := ConfigFromString(configStr) + require.NoError(t, err) + require.NotEmpty(t, config) + + // test + check, err := GetEnabledReceivers(logger, config) + assert.NoError(t, err) + require.Empty(t, check) + //require.NotEmpty(t, check) +} From 0326fe71c6485024ad2e9607f5c86a0f9399ed82 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 6 Apr 2022 12:12:04 +0200 Subject: [PATCH 11/12] Changed function signature removing error return Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_to_ports.go | 6 ++-- pkg/collector/adapters/config_validate.go | 31 +++++++------------ .../adapters/config_validate_test.go | 7 ++--- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go index 998fbfedf6..de1dc2ab08 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/pkg/collector/adapters/config_to_ports.go @@ -51,9 +51,9 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{ if !ok { return nil, ErrNoReceivers } - recEnabled, err := GetEnabledReceivers(logger, config) - if err != nil { - return nil, err + recEnabled := GetEnabledReceivers(logger, config) + if recEnabled == nil { + return nil, ErrReceiversNotAMap } receivers, ok := receiversProperty.(map[interface{}]interface{}) if !ok { diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go index 47c319dde1..1dde38016e 100644 --- a/pkg/collector/adapters/config_validate.go +++ b/pkg/collector/adapters/config_validate.go @@ -15,26 +15,19 @@ package adapters import ( - "errors" - "fmt" - "github.com/go-logr/logr" ) -var ( - errNoPipeline = errors.New("no pipeline available as part of the configuration") -) - //Following Otel Doc: Configuring a receiver does not enable it. The receivers are enabled via pipelines within the service section. //ConfigValidate returns all receivers, setting them as true for enabled and false for non-configured services in pipeline set. -func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) (map[interface{}]bool, error) { +func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) map[interface{}]bool { cfgReceivers, ok := config["receivers"] if !ok { - return nil, ErrNoReceivers + return nil } receivers, ok := cfgReceivers.(map[interface{}]interface{}) if !ok { - return nil, ErrReceiversNotAMap + return nil } availableReceivers := map[interface{}]bool{} @@ -43,7 +36,7 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) //Safe Cast receiverID, ok := recvID.(string) if !ok { - return nil, fmt.Errorf("ReceiverID is not a string: %v", receiverID) + return nil } //Getting all receivers present in the receivers section and setting them to false. availableReceivers[receiverID] = false @@ -51,12 +44,12 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) cfgService, ok := config["service"].(map[interface{}]interface{}) if !ok { - return nil, errNoService + return nil } pipeline, ok := cfgService["pipelines"].(map[interface{}]interface{}) if !ok { - return nil, errNoPipeline + return nil } availablePipelines := map[string]bool{} @@ -64,7 +57,7 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) //Safe Cast pipelineID, ok := pipID.(string) if !ok { - return nil, fmt.Errorf("PipelineID is not a string: %v", pipelineID) + return nil } //Getting all the available pipelines. availablePipelines[pipelineID] = true @@ -75,19 +68,19 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) //Safe Cast pipelineV, ok := pipelineID.(string) if !ok { - return nil, fmt.Errorf("PipelineID is not a string: %v", pipelineV) + return nil } //Condition will get information if there are multiple configured pipelines. if len(pipelineV) > 0 { pipelineDesc, ok := pipelineCfg.(map[interface{}]interface{}) if !ok { - return nil, fmt.Errorf("pipeline was not properly configured") + return nil } for pipSpecID, pipSpecCfg := range pipelineDesc { if pipSpecID.(string) == "receivers" { receiversList, ok := pipSpecCfg.([]interface{}) if !ok { - return nil, fmt.Errorf("no receivers on pipeline configuration %q", receiversList...) + return nil } // If receiversList is empty means that we haven't any enabled Receiver. if len(receiversList) == 0 { @@ -98,7 +91,7 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) //Safe Cast receiverKey, ok := recKey.(string) if !ok { - return nil, fmt.Errorf("ReceiverKey is not a string: %v", receiverKey) + return nil } availableReceivers[receiverKey] = true } @@ -114,5 +107,5 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) } } } - return availableReceivers, nil + return availableReceivers } diff --git a/pkg/collector/adapters/config_validate_test.go b/pkg/collector/adapters/config_validate_test.go index 03a8bb1e6d..b5a6cd8e52 100644 --- a/pkg/collector/adapters/config_validate_test.go +++ b/pkg/collector/adapters/config_validate_test.go @@ -19,7 +19,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -62,8 +61,7 @@ service: require.NotEmpty(t, config) // test - check, err := GetEnabledReceivers(logger, config) - assert.NoError(t, err) + check := GetEnabledReceivers(logger, config) require.NotEmpty(t, check) } @@ -104,8 +102,7 @@ service: require.NotEmpty(t, config) // test - check, err := GetEnabledReceivers(logger, config) - assert.NoError(t, err) + check := GetEnabledReceivers(logger, config) require.Empty(t, check) //require.NotEmpty(t, check) } From a08aab1a458827c696c0d39d738741dc3555addd Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 6 Apr 2022 13:35:06 +0200 Subject: [PATCH 12/12] Fixing comments and function logic Signed-off-by: Yuri Sa --- pkg/collector/adapters/config_validate.go | 6 +++--- pkg/collector/adapters/config_validate_test.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/collector/adapters/config_validate.go b/pkg/collector/adapters/config_validate.go index 1dde38016e..c7fc0fa87a 100644 --- a/pkg/collector/adapters/config_validate.go +++ b/pkg/collector/adapters/config_validate.go @@ -19,7 +19,7 @@ import ( ) //Following Otel Doc: Configuring a receiver does not enable it. The receivers are enabled via pipelines within the service section. -//ConfigValidate returns all receivers, setting them as true for enabled and false for non-configured services in pipeline set. +//GetEnabledReceivers returns all enabled receivers as a true flag set. If it can't find any receiver, it will return a nil interface. func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) map[interface{}]bool { cfgReceivers, ok := config["receivers"] if !ok { @@ -68,7 +68,7 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) //Safe Cast pipelineV, ok := pipelineID.(string) if !ok { - return nil + continue } //Condition will get information if there are multiple configured pipelines. if len(pipelineV) > 0 { @@ -80,7 +80,7 @@ func GetEnabledReceivers(logger logr.Logger, config map[interface{}]interface{}) if pipSpecID.(string) == "receivers" { receiversList, ok := pipSpecCfg.([]interface{}) if !ok { - return nil + continue } // If receiversList is empty means that we haven't any enabled Receiver. if len(receiversList) == 0 { diff --git a/pkg/collector/adapters/config_validate_test.go b/pkg/collector/adapters/config_validate_test.go index b5a6cd8e52..27cd8a7d76 100644 --- a/pkg/collector/adapters/config_validate_test.go +++ b/pkg/collector/adapters/config_validate_test.go @@ -104,5 +104,4 @@ service: // test check := GetEnabledReceivers(logger, config) require.Empty(t, check) - //require.NotEmpty(t, check) }