From 69eb190e11a2bc5436388fda66534670c24de987 Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Sun, 19 Jan 2025 21:08:59 -0800 Subject: [PATCH] [chore] simplify extension code --- .../components/extensions/healthcheckv1.go | 12 ++--- .../extensions/healthcheckv1_test.go | 14 +++--- internal/components/extensions/helpers.go | 47 +++++-------------- .../components/extensions/helpers_test.go | 19 ++++---- 4 files changed, 34 insertions(+), 58 deletions(-) diff --git a/internal/components/extensions/healthcheckv1.go b/internal/components/extensions/healthcheckv1.go index 49769f2f8b..9ad5cd1614 100644 --- a/internal/components/extensions/healthcheckv1.go +++ b/internal/components/extensions/healthcheckv1.go @@ -23,8 +23,8 @@ import ( ) const ( - DefaultHealthcheckV1Path = "/" - DefaultHealthcheckV1Port = 13133 + defaultHealthcheckV1Path = "/" + defaultHealthcheckV1Port = 13133 ) type healthcheckV1Config struct { @@ -32,18 +32,18 @@ type healthcheckV1Config struct { Path string `mapstructure:"path"` } -// HealthCheckV1Probe returns the probe configuration for the healthcheck v1 extension. +// healthCheckV1Probe returns the probe configuration for the healthcheck v1 extension. // Right now no TLS config is parsed. -func HealthCheckV1Probe(logger logr.Logger, config healthcheckV1Config) (*corev1.Probe, error) { +func healthCheckV1Probe(logger logr.Logger, config healthcheckV1Config) (*corev1.Probe, error) { path := config.Path if len(path) == 0 { - path = DefaultHealthcheckV1Path + path = defaultHealthcheckV1Path } return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: path, - Port: intstr.FromInt32(config.GetPortNumOrDefault(logger, DefaultHealthcheckV1Port)), + Port: intstr.FromInt32(config.GetPortNumOrDefault(logger, defaultHealthcheckV1Port)), }, }, }, nil diff --git a/internal/components/extensions/healthcheckv1_test.go b/internal/components/extensions/healthcheckv1_test.go index 52f857f864..fcc810b914 100644 --- a/internal/components/extensions/healthcheckv1_test.go +++ b/internal/components/extensions/healthcheckv1_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package extensions_test +package extensions import ( "fmt" @@ -22,8 +22,6 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" - - "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" ) func TestHealthCheckV1Probe(t *testing.T) { @@ -66,7 +64,7 @@ func TestHealthCheckV1Probe(t *testing.T) { ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/healthz", - Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + Port: intstr.FromInt32(defaultHealthcheckV1Port), }, }, }, @@ -102,7 +100,7 @@ func TestHealthCheckV1Probe(t *testing.T) { ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/", - Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + Port: intstr.FromInt32(defaultHealthcheckV1Port), }, }, }, @@ -136,7 +134,7 @@ func TestHealthCheckV1Probe(t *testing.T) { ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/", - Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + Port: intstr.FromInt32(defaultHealthcheckV1Port), }, }, }, @@ -165,7 +163,7 @@ func TestHealthCheckV1Probe(t *testing.T) { ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/healthz", - Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + Port: intstr.FromInt32(defaultHealthcheckV1Port), }, }, }, @@ -175,7 +173,7 @@ func TestHealthCheckV1Probe(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - parser := extensions.ParserFor("health_check") + parser := ParserFor("health_check") got, err := parser.GetLivenessProbe(logr.Discard(), tt.args.config) if !tt.wantErr(t, err, fmt.Sprintf("GetLivenessProbe(%v)", tt.args.config)) { return diff --git a/internal/components/extensions/helpers.go b/internal/components/extensions/helpers.go index 87708a60e1..49391b9ab6 100644 --- a/internal/components/extensions/helpers.go +++ b/internal/components/extensions/helpers.go @@ -22,17 +22,19 @@ import ( ) // registry holds a record of all known receiver parsers. -var registry = make(map[string]components.Parser) - -// Register adds a new parser builder to the list of known builders. -func Register(name string, p components.Parser) { - registry[name] = p -} - -// IsRegistered checks whether a parser is registered with the given name. -func IsRegistered(name string) bool { - _, ok := registry[name] - return ok +var registry = map[string]components.Parser{ + "health_check": components.NewBuilder[healthcheckV1Config](). + WithName("health_check"). + WithPort(13133). + WithReadinessGen(healthCheckV1Probe). + WithLivenessGen(healthCheckV1Probe). + WithPortParser(func(logger logr.Logger, name string, defaultPort *corev1.ServicePort, config healthcheckV1Config) ([]corev1.ServicePort, error) { + return components.ParseSingleEndpointSilent(logger, name, defaultPort, &config.SingleEndpointConfig) + }). + MustBuild(), + "jaeger_query": components.NewSinglePortParserBuilder("jaeger_query", 16686). + WithTargetPort(16686). + MustBuild(), } // ParserFor returns a parser builder for the given exporter name. @@ -43,26 +45,3 @@ func ParserFor(name string) components.Parser { // We want the default for exporters to fail silently. return components.NewBuilder[any]().WithName(name).MustBuild() } - -var ( - componentParsers = []components.Parser{ - components.NewBuilder[healthcheckV1Config](). - WithName("health_check"). - WithPort(13133). - WithReadinessGen(HealthCheckV1Probe). - WithLivenessGen(HealthCheckV1Probe). - WithPortParser(func(logger logr.Logger, name string, defaultPort *corev1.ServicePort, config healthcheckV1Config) ([]corev1.ServicePort, error) { - return components.ParseSingleEndpointSilent(logger, name, defaultPort, &config.SingleEndpointConfig) - }). - MustBuild(), - components.NewSinglePortParserBuilder("jaeger_query", 16686). - WithTargetPort(16686). - MustBuild(), - } -) - -func init() { - for _, parser := range componentParsers { - Register(parser.ParserType(), parser) - } -} diff --git a/internal/components/extensions/helpers_test.go b/internal/components/extensions/helpers_test.go index b747e3a9ec..ea158633a9 100644 --- a/internal/components/extensions/helpers_test.go +++ b/internal/components/extensions/helpers_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package extensions_test +package extensions import ( "testing" @@ -21,13 +21,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/open-telemetry/opentelemetry-operator/internal/components" - "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) func TestParserForReturns(t *testing.T) { const testComponentName = "test" - parser := extensions.ParserFor(testComponentName) + parser := ParserFor(testComponentName) assert.Equal(t, "test", parser.ParserType()) assert.Equal(t, "__test", parser.ParserName()) ports, err := parser.Ports(logr.Discard(), testComponentName, map[string]interface{}{ @@ -39,9 +38,8 @@ func TestParserForReturns(t *testing.T) { func TestCanRegister(t *testing.T) { const testComponentName = "test" - extensions.Register(testComponentName, components.NewSinglePortParserBuilder(testComponentName, 9000).MustBuild()) - assert.True(t, extensions.IsRegistered(testComponentName)) - parser := extensions.ParserFor(testComponentName) + registry[testComponentName] = components.NewSinglePortParserBuilder(testComponentName, 9000).MustBuild() + parser := ParserFor(testComponentName) assert.Equal(t, "test", parser.ParserType()) assert.Equal(t, "__test", parser.ParserName()) ports, err := parser.Ports(logr.Discard(), testComponentName, map[string]interface{}{}) @@ -60,11 +58,12 @@ func TestExtensionsComponentParsers(t *testing.T) { } { t.Run(tt.exporterName, func(t *testing.T) { t.Run("is registered", func(t *testing.T) { - assert.True(t, extensions.IsRegistered(tt.exporterName)) + _, ok := registry[tt.exporterName] + assert.True(t, ok) }) t.Run("bad config errors", func(t *testing.T) { // prepare - parser := extensions.ParserFor(tt.exporterName) + parser := ParserFor(tt.exporterName) // test throwing in pure junk _, err := parser.Ports(logr.Discard(), tt.exporterName, func() {}) @@ -75,7 +74,7 @@ func TestExtensionsComponentParsers(t *testing.T) { t.Run("assigns the expected port", func(t *testing.T) { // prepare - parser := extensions.ParserFor(tt.exporterName) + parser := ParserFor(tt.exporterName) // test ports, err := parser.Ports(logr.Discard(), tt.exporterName, map[string]interface{}{}) @@ -93,7 +92,7 @@ func TestExtensionsComponentParsers(t *testing.T) { t.Run("allows port to be overridden", func(t *testing.T) { // prepare - parser := extensions.ParserFor(tt.exporterName) + parser := ParserFor(tt.exporterName) // test ports, err := parser.Ports(logr.Discard(), tt.exporterName, map[string]interface{}{