From 58cc00db2ccee5e1eb194508932484d61e10c9c0 Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Mon, 14 Oct 2024 15:44:20 +0200 Subject: [PATCH] make config overrides compatible with documented behaviour and do not merge attributes --- config/lura.go | 19 +++++++++ state/config.go | 94 +++++++++++++++---------------------------- state/config_test.go | 96 +++++++++++++++++++++++++------------------- 3 files changed, 105 insertions(+), 104 deletions(-) diff --git a/config/lura.go b/config/lura.go index 9e8c32c..a1167e6 100644 --- a/config/lura.go +++ b/config/lura.go @@ -61,3 +61,22 @@ func LuraExtraCfg(extraCfg luraconfig.ExtraConfig) (*ConfigData, error) { return cfg, nil } + +func LuraLayerExtraCfg(extraCfg luraconfig.ExtraConfig) (*LayersOpts, error) { + tmp, ok := extraCfg[Namespace] + if !ok { + return nil, ErrNoConfig + } + + buf := new(bytes.Buffer) + if err := json.NewEncoder(buf).Encode(tmp); err != nil { + return nil, err + } + + cfg := new(LayersOpts) + if err := json.NewDecoder(buf).Decode(cfg); err != nil { + return nil, err + } + + return cfg, nil +} diff --git a/state/config.go b/state/config.go index d652157..1a54094 100644 --- a/state/config.go +++ b/state/config.go @@ -7,11 +7,20 @@ import ( type Config interface { OTEL() OTEL + // GlobalOpts gets the configuration at the service level. GlobalOpts() *config.GlobalOpts // Gets the OTEL instance for a given endpoint EndpointOTEL(cfg *luraconfig.EndpointConfig) OTEL + // EndpointPipeOpts retrieve "proxy" level configuration for a given + // endpoint. EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts + // EndpointBackendOpts should return a config for all the child + // backend of this endpoint. + // + // Deprecated: the interface should only need to fetch the BackendOpts + // from a luraconfig.Backend when configuring at the Backend level: + // the BackendOpts function must be used instead. EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts BackendOTEL(cfg *luraconfig.Backend) OTEL @@ -39,44 +48,30 @@ func (*StateConfig) EndpointOTEL(_ *luraconfig.EndpointConfig) OTEL { return GlobalState() } +// EndpointPipeOpts checks if there is an override for pipe ("proxy") +// options at the endpoint levels a fully replaces (it DOES NOT MERGE +// attributes) the existing config from the service level configuration. +// If none of those configs are found, it falls back to the defaults. func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts { - var sOpts *config.PipeOpts - var extraPOpts *config.PipeOpts - + var opts *config.PipeOpts if s != nil && s.cfgData.Layers != nil { - sOpts = s.cfgData.Layers.Pipe + opts = s.cfgData.Layers.Pipe } - cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err == nil && cfgExtra != nil && cfgExtra.Layers != nil { - extraPOpts = cfgExtra.Layers.Pipe + cfgLayer, err := config.LuraLayerExtraCfg(cfg.ExtraConfig) + if err == nil && cfgLayer != nil { + opts = cfgLayer.Pipe } - if extraPOpts == nil { - if sOpts == nil { - return new(config.PipeOpts) - } - return sOpts - } else if sOpts == nil { - return extraPOpts + if opts == nil { + return new(config.PipeOpts) } - - pOpts := new(config.PipeOpts) - *pOpts = *sOpts - - pOpts.MetricsStaticAttributes = append( - pOpts.MetricsStaticAttributes, - cfgExtra.Layers.Pipe.MetricsStaticAttributes..., - ) - - pOpts.TracesStaticAttributes = append( - pOpts.TracesStaticAttributes, - cfgExtra.Layers.Pipe.TracesStaticAttributes..., - ) - - return pOpts + return opts } +// EndpointBackendOpts is a bad interface function, as is should receive +// as a param a luraconfig.Endpoint .. but also makes no sense to have it +// because we only need the backend configuration at func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { return s.mergedBackendOpts(cfg) } @@ -90,45 +85,20 @@ func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { } func (s *StateConfig) mergedBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { - var extraBOpts *config.BackendOpts - var sOpts *config.BackendOpts - + var opts *config.BackendOpts if s != nil && s.cfgData.Layers != nil { - sOpts = s.cfgData.Layers.Backend + opts = s.cfgData.Layers.Backend } - cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err == nil && cfgExtra != nil && cfgExtra.Layers != nil { - extraBOpts = cfgExtra.Layers.Backend + cfgLayer, err := config.LuraLayerExtraCfg(cfg.ExtraConfig) + if err == nil && cfgLayer != nil { + opts = cfgLayer.Backend } - if extraBOpts == nil { - if sOpts == nil { - return new(config.BackendOpts) - } - return sOpts - } else if sOpts == nil { - return extraBOpts - } - - bOpts := new(config.BackendOpts) - *bOpts = *sOpts - - if extraBOpts.Metrics != nil { - bOpts.Metrics.StaticAttributes = append( - bOpts.Metrics.StaticAttributes, - cfgExtra.Layers.Backend.Metrics.StaticAttributes..., - ) + if opts == nil { + return new(config.BackendOpts) } - - if extraBOpts.Traces != nil { - bOpts.Traces.StaticAttributes = append( - bOpts.Traces.StaticAttributes, - cfgExtra.Layers.Backend.Traces.StaticAttributes..., - ) - } - - return bOpts + return opts } func (s *StateConfig) SkipEndpoint(endpoint string) bool { diff --git a/state/config_test.go b/state/config_test.go index 3fc84f9..466473e 100644 --- a/state/config_test.go +++ b/state/config_test.go @@ -10,11 +10,9 @@ import ( func TestEndpointPipeConfigOverride(t *testing.T) { globalMetricAttrs := makeGlobalMetricAttr() overrideMetricAttrs := makeOverrideMetricAttr() - expectedMetricAttrs := append(globalMetricAttrs, overrideMetricAttrs...) // skipcq: CRT-D0001 globalTraceAttrs := makeGlobalTraceAttr() overrideTraceAttrs := makeOverrideTraceAttr() - expectedTraceAttrs := append(globalTraceAttrs, overrideTraceAttrs...) // skipcq: CRT-D0001 stateCfg := &StateConfig{ cfgData: makePipeConf(globalMetricAttrs, globalTraceAttrs), @@ -22,26 +20,33 @@ func TestEndpointPipeConfigOverride(t *testing.T) { pipeCfg := &luraconfig.EndpointConfig{ ExtraConfig: map[string]interface{}{ - "telemetry/opentelemetry": makePipeConf(overrideMetricAttrs, overrideTraceAttrs), + "telemetry/opentelemetry": map[string]interface{}{ + "proxy": map[string]interface{}{ + "metrics_static_attributes": overrideMetricAttrs, + "traces_static_attributes": overrideTraceAttrs, + }, + }, }, } pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg) + if pipeOpts == nil { + t.Errorf("Unexpected nil for pipe opts") + return + } - if len(pipeOpts.MetricsStaticAttributes) != len(expectedMetricAttrs) { + if len(pipeOpts.MetricsStaticAttributes) != len(overrideMetricAttrs) { t.Errorf( "Incorrect number of attributes for metrics. returned: %+v - expected: %+v", - pipeOpts.MetricsStaticAttributes, - expectedMetricAttrs, - ) + pipeOpts.MetricsStaticAttributes, overrideMetricAttrs) + return } - if len(pipeOpts.TracesStaticAttributes) != len(expectedTraceAttrs) { + if len(pipeOpts.TracesStaticAttributes) != len(overrideTraceAttrs) { t.Errorf( "Incorrect number of attributes for traces. returned: %+v - expected: %+v", - pipeOpts.TracesStaticAttributes, - expectedTraceAttrs, - ) + pipeOpts.TracesStaticAttributes, overrideTraceAttrs) + return } } @@ -56,12 +61,17 @@ func TestEndpointPipeNoOverride(t *testing.T) { } pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg) + if pipeOpts == nil { + t.Errorf("unextpected nil pipeOpts") + return + } if len(pipeOpts.MetricsStaticAttributes) > 1 { t.Errorf( "Incorrect number of attributes for metrics. returned: %+v", pipeOpts.MetricsStaticAttributes, ) + return } } @@ -83,17 +93,16 @@ func TestEndpointPipeConfigOnlyOverride(t *testing.T) { "Incorrect number of attributes for metrics. returned: %+v", pipeOpts.MetricsStaticAttributes, ) + return } } func TestBackendConfigOverride(t *testing.T) { globalMetricAttrs := makeGlobalMetricAttr() overrideMetricAttrs := makeOverrideMetricAttr() - expectedMetricAttrs := append(globalMetricAttrs, overrideMetricAttrs...) // skipcq: CRT-D0001 globalTraceAttrs := makeGlobalTraceAttr() overrideTraceAttrs := makeOverrideTraceAttr() - expectedTraceAttrs := append(globalTraceAttrs, overrideTraceAttrs...) // skipcq: CRT-D0001 stateCfg := &StateConfig{ cfgData: makeBackendConf(globalMetricAttrs, globalTraceAttrs), @@ -101,26 +110,46 @@ func TestBackendConfigOverride(t *testing.T) { backendCfg := &luraconfig.Backend{ ExtraConfig: map[string]interface{}{ - "telemetry/opentelemetry": makeBackendConf(overrideMetricAttrs, overrideTraceAttrs), + "telemetry/opentelemetry": map[string]interface{}{ + "backend": map[string]interface{}{ + "metrics": map[string]interface{}{ + "static_attributes": overrideMetricAttrs, + }, + "traces": map[string]interface{}{ + "static_attributes": overrideTraceAttrs, + }, + }, + }, }, } backendOpts := stateCfg.BackendOpts(backendCfg) + if backendOpts == nil { + t.Errorf("unexpected nil backendOpts") + return + } + + if backendOpts.Metrics == nil { + t.Errorf("unexpected nil backendOpts.Metrics") + return + } - if len(backendOpts.Metrics.StaticAttributes) != len(expectedMetricAttrs) { + if len(backendOpts.Metrics.StaticAttributes) != len(overrideMetricAttrs) { t.Errorf( "Incorrect number of attributes for metrics. returned: %+v - expected: %+v", - backendOpts.Metrics.StaticAttributes, - expectedMetricAttrs, - ) + backendOpts.Metrics.StaticAttributes, overrideMetricAttrs) + return } - if len(backendOpts.Traces.StaticAttributes) != len(expectedTraceAttrs) { + if backendOpts.Traces == nil { + t.Errorf("unexpected nil backendOpts.Traces") + return + } + if len(backendOpts.Traces.StaticAttributes) != len(overrideTraceAttrs) { t.Errorf( "Incorrect number of attributes for traces. returned: %+v - expected: %+v", - backendOpts.Traces.StaticAttributes, - expectedTraceAttrs, - ) + backendOpts.Traces.StaticAttributes, overrideTraceAttrs) + return } } @@ -135,28 +164,11 @@ func TestBackendConfigNoOverride(t *testing.T) { } backendOpts := stateCfg.BackendOpts(backendCfg) - - if len(backendOpts.Metrics.StaticAttributes) > 1 { - t.Errorf( - "Incorrect number of attributes for metrics. returned: %+v", - backendOpts.Traces.StaticAttributes, - ) - } -} - -func TestBackendConfigOnlyOverride(t *testing.T) { - stateCfg := &StateConfig{ - cfgData: makeBackendConf([]config.KeyValue{}, []config.KeyValue{}), + if backendOpts == nil { + t.Errorf("unexpected nil backendOpts") + return } - backendCfg := &luraconfig.Backend{ - ExtraConfig: map[string]interface{}{ - "telemetry/opentelemetry": makeBackendConf(makeOverrideMetricAttr(), makeOverrideTraceAttr()), - }, - } - - backendOpts := stateCfg.BackendOpts(backendCfg) - if len(backendOpts.Metrics.StaticAttributes) > 1 { t.Errorf( "Incorrect number of attributes for metrics. returned: %+v",