From c49a0b0cdcb50602195ad97d50cdb036d1ceadba Mon Sep 17 00:00:00 2001 From: Silvio Sepulveda Date: Wed, 24 Apr 2024 10:15:55 +0200 Subject: [PATCH 1/9] Basic implemetation of static attributes in all layers --- config/config.go | 16 +++++++++------- config/lura.go | 15 +++++++++++++++ http/server/metrics.go | 12 ++++++------ http/server/server.go | 15 +++++++++++---- http/server/tracking.go | 12 ++++++++++++ lura/proxy.go | 8 ++++++++ router/gin/endpoint.go | 18 ++++++++++++++++++ 7 files changed, 79 insertions(+), 17 deletions(-) diff --git a/config/config.go b/config/config.go index 27b8423..5e33c23 100644 --- a/config/config.go +++ b/config/config.go @@ -138,18 +138,20 @@ type LayersOpts struct { // We can select if we want to disable the metrics, // the traces, and / or the trace propagation. type GlobalOpts struct { - DisableMetrics bool `json:"disable_metrics"` - DisableTraces bool `json:"disable_traces"` - DisablePropagation bool `json:"disable_propagation"` - ReportHeaders bool `json:"report_headers"` + DisableMetrics bool `json:"disable_metrics"` + DisableTraces bool `json:"disable_traces"` + DisablePropagation bool `json:"disable_propagation"` + ReportHeaders bool `json:"report_headers"` + StaticAttributes Attributes `json:"static_attributes"` } // PipeOpts has the options for the KrakenD pipe stage // to disable metrics and traces. type PipeOpts struct { - DisableMetrics bool `json:"disable_metrics"` - DisableTraces bool `json:"disable_traces"` - ReportHeaders bool `json:"report_headers"` + DisableMetrics bool `json:"disable_metrics"` + DisableTraces bool `json:"disable_traces"` + ReportHeaders bool `json:"report_headers"` + StaticAttributes Attributes `json:"static_attributes"` } // Enabled returns if either metrics or traces are enabled diff --git a/config/lura.go b/config/lura.go index 2de0e26..6e37ac8 100644 --- a/config/lura.go +++ b/config/lura.go @@ -46,3 +46,18 @@ func FromLura(srvCfg luraconfig.ServiceConfig) (*ConfigData, error) { cfg.UnsetFieldsToDefaults() return cfg, nil } + +func EndpointExtraCfg(endpointExtraCfg luraconfig.ExtraConfig) *ConfigData { + cfg := new(ConfigData) + tmp, ok := endpointExtraCfg[Namespace] + if !ok { + return nil + } + buf := new(bytes.Buffer) + json.NewEncoder(buf).Encode(tmp) + if err := json.NewDecoder(buf).Decode(cfg); err != nil { + return nil + } + + return cfg +} diff --git a/http/server/metrics.go b/http/server/metrics.go index b5ddd87..2010623 100644 --- a/http/server/metrics.go +++ b/http/server/metrics.go @@ -5,7 +5,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/semconv/v1.21.0" + semconv "go.opentelemetry.io/otel/semconv/v1.21.0" kotelconfig "github.com/krakend/krakend-otel/config" ) @@ -36,11 +36,11 @@ func (m *metricsHTTP) report(t *tracking, r *http.Request) { if m == nil || m.latency == nil { return } - dynAttrsOpts := metric.WithAttributes( - semconv.HTTPRoute(t.EndpointPattern()), - semconv.HTTPRequestMethodKey.String(r.Method), - semconv.HTTPResponseStatusCode(t.responseStatus), - ) + dynAttrs := t.staticAttrs + dynAttrs = append(dynAttrs, semconv.HTTPRoute(t.EndpointPattern())) + dynAttrs = append(dynAttrs, semconv.HTTPRequestMethodKey.String(r.Method)) + dynAttrs = append(dynAttrs, semconv.HTTPResponseStatusCode(t.responseStatus)) + dynAttrsOpts := metric.WithAttributes(dynAttrs...) m.latency.Record(t.ctx, t.latencyInSecs, m.fixedAttrsOpts, dynAttrsOpts) m.size.Record(t.ctx, int64(t.responseSize), m.fixedAttrsOpts, dynAttrsOpts) } diff --git a/http/server/server.go b/http/server/server.go index 767557d..63a75e4 100644 --- a/http/server/server.go +++ b/http/server/server.go @@ -72,16 +72,23 @@ func NewTrackingHandler(next http.Handler) http.Handler { prop = s.Propagator() } + // Add configured static attributes + attrs := []attribute.KeyValue{} + for _, kv := range gCfg.StaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + attrs = append(attrs, attribute.String(kv.Key, kv.Value)) + } + } + var m *metricsHTTP if !gCfg.DisableMetrics { - m = newMetricsHTTP(s.Meter(), []attribute.KeyValue{}) + m = newMetricsHTTP(s.Meter(), attrs) } var t *tracesHTTP if !gCfg.DisableTraces { - t = newTracesHTTP(s.Tracer(), []attribute.KeyValue{ - attribute.String("krakend.stage", "global"), - }, gCfg.ReportHeaders) + attrs = append(attrs, attribute.String("krakend.stage", "global")) + t = newTracesHTTP(s.Tracer(), attrs, gCfg.ReportHeaders) } return &trackingHandler{ diff --git a/http/server/tracking.go b/http/server/tracking.go index b640333..b8b3f1e 100644 --- a/http/server/tracking.go +++ b/http/server/tracking.go @@ -4,6 +4,7 @@ import ( "context" "time" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -28,6 +29,7 @@ type tracking struct { writeErrs []error endpointPattern string isHijacked bool + staticAttrs []attribute.KeyValue hijackedErr error } @@ -41,6 +43,10 @@ func (t *tracking) EndpointPattern() string { return t.endpointPattern } +func (t *tracking) StaticAttributes() []attribute.KeyValue { + return t.staticAttrs +} + func newTracking() *tracking { return &tracking{ responseStatus: 200, @@ -64,6 +70,12 @@ func SetEndpointPattern(ctx context.Context, endpointPattern string) { } } +func SetStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { + if t := fromContext(ctx); t != nil { + t.staticAttrs = attrs + } +} + func (t *tracking) Start() { t.startTime = time.Now() } diff --git a/lura/proxy.go b/lura/proxy.go index e268331..50898d1 100644 --- a/lura/proxy.go +++ b/lura/proxy.go @@ -114,6 +114,14 @@ func ProxyFactory(pf proxy.Factory) proxy.FactoryFunc { semconv.HTTPRequestMethodKey.String(cfg.Method), semconv.HTTPRoute(urlPattern), } + + // Add configured static attributes + for _, kv := range pipeOpts.StaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + attrs = append(attrs, attribute.String(kv.Key, kv.Value)) + } + } + return middleware(gs, !pipeOpts.DisableMetrics, !pipeOpts.DisableTraces, "proxy", urlPattern, attrs, pipeOpts.ReportHeaders)(next), nil } diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index e95a721..b46a9a4 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -5,6 +5,7 @@ import ( luraconfig "github.com/luraproject/lura/v2/config" "github.com/luraproject/lura/v2/proxy" krakendgin "github.com/luraproject/lura/v2/router/gin" + "go.opentelemetry.io/otel/attribute" kotelconfig "github.com/krakend/krakend-otel/config" kotelserver "github.com/krakend/krakend-otel/http/server" @@ -23,12 +24,29 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { } urlPattern := kotelconfig.NormalizeURLPattern(cfg.Endpoint) next := hf(cfg, p) + attrs := getAttrib(cfg) + return func(c *gin.Context) { // we set the matched route to a data struct stored in the // context by the outer http layer, so it can be reported // in metrics and traces. kotelserver.SetEndpointPattern(c.Request.Context(), urlPattern) + kotelserver.SetStaticAttributtes(c.Request.Context(), attrs) next(c) } } } + +func getAttrib(cfg *luraconfig.EndpointConfig) []attribute.KeyValue { + attrs := []attribute.KeyValue{} + cfgExtra := kotelconfig.EndpointExtraCfg(cfg.ExtraConfig) + if cfgExtra != nil && cfgExtra.Layers.Global != nil { + for _, kv := range cfgExtra.Layers.Global.StaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + attrs = append(attrs, attribute.String(kv.Key, kv.Value)) + } + } + } + + return attrs +} From 4810e2ecdf87fc02530966a29407814c4176b37b Mon Sep 17 00:00:00 2001 From: Silvio Sepulveda Date: Wed, 24 Apr 2024 14:49:37 +0200 Subject: [PATCH 2/9] Split metrics and traces attributes --- config/config.go | 20 +++++++++++--------- http/server/metrics.go | 2 +- http/server/server.go | 19 +++++++++++++------ http/server/traces.go | 3 ++- http/server/tracking.go | 37 ++++++++++++++++++++++++------------- lura/proxy.go | 25 +++++++++++++++++-------- router/gin/endpoint.go | 35 +++++++++++++++++++---------------- 7 files changed, 87 insertions(+), 54 deletions(-) diff --git a/config/config.go b/config/config.go index 5e33c23..edf0ded 100644 --- a/config/config.go +++ b/config/config.go @@ -138,20 +138,22 @@ type LayersOpts struct { // We can select if we want to disable the metrics, // the traces, and / or the trace propagation. type GlobalOpts struct { - DisableMetrics bool `json:"disable_metrics"` - DisableTraces bool `json:"disable_traces"` - DisablePropagation bool `json:"disable_propagation"` - ReportHeaders bool `json:"report_headers"` - StaticAttributes Attributes `json:"static_attributes"` + DisableMetrics bool `json:"disable_metrics"` + DisableTraces bool `json:"disable_traces"` + DisablePropagation bool `json:"disable_propagation"` + ReportHeaders bool `json:"report_headers"` + MetricsStaticAttributes Attributes `json:"metrics_static_attributes"` + TracesStaticAttributes Attributes `json:"traces_static_attributes"` } // PipeOpts has the options for the KrakenD pipe stage // to disable metrics and traces. type PipeOpts struct { - DisableMetrics bool `json:"disable_metrics"` - DisableTraces bool `json:"disable_traces"` - ReportHeaders bool `json:"report_headers"` - StaticAttributes Attributes `json:"static_attributes"` + DisableMetrics bool `json:"disable_metrics"` + DisableTraces bool `json:"disable_traces"` + ReportHeaders bool `json:"report_headers"` + MetricsStaticAttributes Attributes `json:"metrics_static_attributes"` + TracesStaticAttributes Attributes `json:"traces_static_attributes"` } // Enabled returns if either metrics or traces are enabled diff --git a/http/server/metrics.go b/http/server/metrics.go index 2010623..350e0db 100644 --- a/http/server/metrics.go +++ b/http/server/metrics.go @@ -36,7 +36,7 @@ func (m *metricsHTTP) report(t *tracking, r *http.Request) { if m == nil || m.latency == nil { return } - dynAttrs := t.staticAttrs + dynAttrs := t.metricsStaticAttrs dynAttrs = append(dynAttrs, semconv.HTTPRoute(t.EndpointPattern())) dynAttrs = append(dynAttrs, semconv.HTTPRequestMethodKey.String(r.Method)) dynAttrs = append(dynAttrs, semconv.HTTPResponseStatusCode(t.responseStatus)) diff --git a/http/server/server.go b/http/server/server.go index 63a75e4..5116ce1 100644 --- a/http/server/server.go +++ b/http/server/server.go @@ -73,22 +73,29 @@ func NewTrackingHandler(next http.Handler) http.Handler { } // Add configured static attributes - attrs := []attribute.KeyValue{} - for _, kv := range gCfg.StaticAttributes { + metricsAttrs := []attribute.KeyValue{} + tracesAttrs := []attribute.KeyValue{} + for _, kv := range gCfg.MetricsStaticAttributes { if len(kv.Key) > 0 && len(kv.Value) > 0 { - attrs = append(attrs, attribute.String(kv.Key, kv.Value)) + metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) + } + } + + for _, kv := range gCfg.TracesStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) } } var m *metricsHTTP if !gCfg.DisableMetrics { - m = newMetricsHTTP(s.Meter(), attrs) + m = newMetricsHTTP(s.Meter(), metricsAttrs) } var t *tracesHTTP if !gCfg.DisableTraces { - attrs = append(attrs, attribute.String("krakend.stage", "global")) - t = newTracesHTTP(s.Tracer(), attrs, gCfg.ReportHeaders) + tracesAttrs = append(tracesAttrs, attribute.String("krakend.stage", "global")) + t = newTracesHTTP(s.Tracer(), tracesAttrs, gCfg.ReportHeaders) } return &trackingHandler{ diff --git a/http/server/traces.go b/http/server/traces.go index 9283be2..10a8991 100644 --- a/http/server/traces.go +++ b/http/server/traces.go @@ -6,7 +6,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" - "go.opentelemetry.io/otel/semconv/v1.21.0" + semconv "go.opentelemetry.io/otel/semconv/v1.21.0" "go.opentelemetry.io/otel/trace" otelhttp "github.com/krakend/krakend-otel/http" @@ -71,6 +71,7 @@ func (t *tracesHTTP) end(tr *tracking) { semconv.HTTPRoute(tr.EndpointPattern()), semconv.HTTPResponseStatusCode(tr.responseStatus), semconv.HTTPResponseBodySize(tr.responseSize)) + tr.span.SetAttributes(tr.tracesStaticAttrs...) if tr.responseHeaders != nil { // report all incoming headers diff --git a/http/server/tracking.go b/http/server/tracking.go index b8b3f1e..1c411bd 100644 --- a/http/server/tracking.go +++ b/http/server/tracking.go @@ -22,15 +22,16 @@ type tracking struct { ctx context.Context span trace.Span - latencyInSecs float64 - responseSize int - responseStatus int - responseHeaders map[string][]string - writeErrs []error - endpointPattern string - isHijacked bool - staticAttrs []attribute.KeyValue - hijackedErr error + latencyInSecs float64 + responseSize int + responseStatus int + responseHeaders map[string][]string + writeErrs []error + endpointPattern string + isHijacked bool + metricsStaticAttrs []attribute.KeyValue + tracesStaticAttrs []attribute.KeyValue + hijackedErr error } func (t *tracking) EndpointPattern() string { @@ -43,8 +44,12 @@ func (t *tracking) EndpointPattern() string { return t.endpointPattern } -func (t *tracking) StaticAttributes() []attribute.KeyValue { - return t.staticAttrs +func (t *tracking) MetricsStaticAttributes() []attribute.KeyValue { + return t.metricsStaticAttrs +} + +func (t *tracking) TracesStaticAttributes() []attribute.KeyValue { + return t.tracesStaticAttrs } func newTracking() *tracking { @@ -70,9 +75,15 @@ func SetEndpointPattern(ctx context.Context, endpointPattern string) { } } -func SetStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { +func SetMetricsStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { + if t := fromContext(ctx); t != nil { + t.metricsStaticAttrs = attrs + } +} + +func SetTracesStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { if t := fromContext(ctx); t != nil { - t.staticAttrs = attrs + t.tracesStaticAttrs = attrs } } diff --git a/lura/proxy.go b/lura/proxy.go index 50898d1..e853e62 100644 --- a/lura/proxy.go +++ b/lura/proxy.go @@ -5,7 +5,7 @@ import ( "time" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/semconv/v1.21.0" + semconv "go.opentelemetry.io/otel/semconv/v1.21.0" "github.com/luraproject/lura/v2/config" "github.com/luraproject/lura/v2/proxy" @@ -48,20 +48,21 @@ func metricsAndTracesMiddleware(next proxy.Proxy, mm *middlewareMeter, mt *middl // middleware creates a proxy that instruments the proxy it wraps by creating an span if enabled, // and report the duration of this stage in metrics if enabled. func middleware(gs state.OTEL, metricsEnabled bool, tracesEnabled bool, - stageName string, urlPattern string, attrs []attribute.KeyValue, reportHeaders bool, + stageName string, urlPattern string, metricsAttrs, tracesAttrs []attribute.KeyValue, + reportHeaders bool, ) proxy.Middleware { var mt *middlewareTracer var mm *middlewareMeter var err error if metricsEnabled { - mm, err = newMiddlewareMeter(gs, stageName, attrs) + mm, err = newMiddlewareMeter(gs, stageName, metricsAttrs) if err != nil { // TODO: log the error metricsEnabled = false } } if tracesEnabled { - mt = newMiddlewareTracer(gs, urlPattern, stageName, reportHeaders, attrs) + mt = newMiddlewareTracer(gs, urlPattern, stageName, reportHeaders, tracesAttrs) } return func(next ...proxy.Proxy) proxy.Proxy { @@ -116,14 +117,22 @@ func ProxyFactory(pf proxy.Factory) proxy.FactoryFunc { } // Add configured static attributes - for _, kv := range pipeOpts.StaticAttributes { + metricsAttrs := attrs + tracesAttrs := attrs + for _, kv := range pipeOpts.MetricsStaticAttributes { if len(kv.Key) > 0 && len(kv.Value) > 0 { - attrs = append(attrs, attribute.String(kv.Key, kv.Value)) + metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) + } + } + + for _, kv := range pipeOpts.TracesStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) } } return middleware(gs, !pipeOpts.DisableMetrics, !pipeOpts.DisableTraces, - "proxy", urlPattern, attrs, pipeOpts.ReportHeaders)(next), nil + "proxy", urlPattern, metricsAttrs, tracesAttrs, pipeOpts.ReportHeaders)(next), nil } } @@ -155,6 +164,6 @@ func BackendFactory(bf proxy.BackendFactory) proxy.BackendFactory { attribute.String("krakend.endpoint.method", cfg.ParentEndpointMethod), } return middleware(gs, !backendOpts.Metrics.DisableStage, !backendOpts.Traces.DisableStage, - "backend", urlPattern, attrs, backendOpts.Traces.ReportHeaders)(next) + "backend", urlPattern, attrs, attrs, backendOpts.Traces.ReportHeaders)(next) } } diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index b46a9a4..e24da9d 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -24,29 +24,32 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { } urlPattern := kotelconfig.NormalizeURLPattern(cfg.Endpoint) next := hf(cfg, p) - attrs := getAttrib(cfg) + metricsAttrs := []attribute.KeyValue{} + tracesAttrs := []attribute.KeyValue{} + + cfgExtra := kotelconfig.EndpointExtraCfg(cfg.ExtraConfig) + if cfgExtra != nil && cfgExtra.Layers.Global != nil { + for _, kv := range cfgExtra.Layers.Global.MetricsStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) + } + } + + for _, kv := range cfgExtra.Layers.Global.TracesStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) + } + } + } return func(c *gin.Context) { // we set the matched route to a data struct stored in the // context by the outer http layer, so it can be reported // in metrics and traces. kotelserver.SetEndpointPattern(c.Request.Context(), urlPattern) - kotelserver.SetStaticAttributtes(c.Request.Context(), attrs) + kotelserver.SetMetricsStaticAttributtes(c.Request.Context(), metricsAttrs) + kotelserver.SetTracesStaticAttributtes(c.Request.Context(), tracesAttrs) next(c) } } } - -func getAttrib(cfg *luraconfig.EndpointConfig) []attribute.KeyValue { - attrs := []attribute.KeyValue{} - cfgExtra := kotelconfig.EndpointExtraCfg(cfg.ExtraConfig) - if cfgExtra != nil && cfgExtra.Layers.Global != nil { - for _, kv := range cfgExtra.Layers.Global.StaticAttributes { - if len(kv.Key) > 0 && len(kv.Value) > 0 { - attrs = append(attrs, attribute.String(kv.Key, kv.Value)) - } - } - } - - return attrs -} From 2fc66f8c02c40043775fdc092c1303b9bedb40cf Mon Sep 17 00:00:00 2001 From: Silvio Sepulveda Date: Thu, 25 Apr 2024 08:11:04 +0200 Subject: [PATCH 3/9] Extend static attributes override to proxy and backend layers --- config/lura.go | 28 +++++++++--------- lura/proxy.go | 18 +++++++++++- router/gin/endpoint.go | 4 +-- state/config.go | 65 +++++++++++++++++++++++++++++++++--------- 4 files changed, 83 insertions(+), 32 deletions(-) diff --git a/config/lura.go b/config/lura.go index 6e37ac8..38be5ef 100644 --- a/config/lura.go +++ b/config/lura.go @@ -24,14 +24,8 @@ var ErrNoConfig = errors.New("no config found for opentelemetry") // In case no "Layers" config is provided, a set of defaults with // everything enabled will be used. func FromLura(srvCfg luraconfig.ServiceConfig) (*ConfigData, error) { - cfg := new(ConfigData) - tmp, ok := srvCfg.ExtraConfig[Namespace] - if !ok { - return nil, ErrNoConfig - } - buf := new(bytes.Buffer) - json.NewEncoder(buf).Encode(tmp) - if err := json.NewDecoder(buf).Decode(cfg); err != nil { + cfg, err := LuraExtraCfg(srvCfg.ExtraConfig) + if err != nil { return nil, err } @@ -47,17 +41,21 @@ func FromLura(srvCfg luraconfig.ServiceConfig) (*ConfigData, error) { return cfg, nil } -func EndpointExtraCfg(endpointExtraCfg luraconfig.ExtraConfig) *ConfigData { - cfg := new(ConfigData) - tmp, ok := endpointExtraCfg[Namespace] +func LuraExtraCfg(extraCfg luraconfig.ExtraConfig) (*ConfigData, error) { + tmp, ok := extraCfg[Namespace] if !ok { - return nil + return nil, ErrNoConfig } + buf := new(bytes.Buffer) - json.NewEncoder(buf).Encode(tmp) + if err := json.NewEncoder(buf).Encode(tmp); err != nil { + return nil, err + } + + cfg := new(ConfigData) if err := json.NewDecoder(buf).Decode(cfg); err != nil { - return nil + return nil, err } - return cfg + return cfg, nil } diff --git a/lura/proxy.go b/lura/proxy.go index e853e62..4bc87ae 100644 --- a/lura/proxy.go +++ b/lura/proxy.go @@ -163,7 +163,23 @@ func BackendFactory(bf proxy.BackendFactory) proxy.BackendFactory { attribute.String("krakend.endpoint.route", parentEndpoint), attribute.String("krakend.endpoint.method", cfg.ParentEndpointMethod), } + + // Add configured static attributes + metricsAttrs := attrs + tracesAttrs := attrs + for _, kv := range backendOpts.Metrics.StaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) + } + } + + for _, kv := range backendOpts.Traces.StaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) + } + } + return middleware(gs, !backendOpts.Metrics.DisableStage, !backendOpts.Traces.DisableStage, - "backend", urlPattern, attrs, attrs, backendOpts.Traces.ReportHeaders)(next) + "backend", urlPattern, metricsAttrs, tracesAttrs, backendOpts.Traces.ReportHeaders)(next) } } diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index e24da9d..2e1531b 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -27,8 +27,8 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { metricsAttrs := []attribute.KeyValue{} tracesAttrs := []attribute.KeyValue{} - cfgExtra := kotelconfig.EndpointExtraCfg(cfg.ExtraConfig) - if cfgExtra != nil && cfgExtra.Layers.Global != nil { + cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) + if err == nil && cfgExtra.Layers.Global != nil { for _, kv := range cfgExtra.Layers.Global.MetricsStaticAttributes { if len(kv.Key) > 0 && len(kv.Value) > 0 { metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) diff --git a/state/config.go b/state/config.go index 6ffa116..25c09fe 100644 --- a/state/config.go +++ b/state/config.go @@ -1,22 +1,22 @@ package state import ( - luraconfig "github.com/luraproject/lura/v2/config" - "github.com/krakend/krakend-otel/config" + kotelconfig "github.com/krakend/krakend-otel/config" + luraconfig "github.com/luraproject/lura/v2/config" ) type Config interface { OTEL() OTEL - GlobalOpts() *config.GlobalOpts + GlobalOpts() config.GlobalOpts // Gets the OTEL instance for a given endpoint EndpointOTEL(cfg *luraconfig.EndpointConfig) OTEL - EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts - EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts + EndpointPipeOpts(cfg *luraconfig.EndpointConfig) config.PipeOpts + EndpointBackendOpts(cfg *luraconfig.Backend) config.BackendOpts BackendOTEL(cfg *luraconfig.Backend) OTEL - BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts + BackendOpts(cfg *luraconfig.Backend) config.BackendOpts // SkipEndpoint tells if an endpoint should not be instrumented SkipEndpoint(endpoint string) bool @@ -32,28 +32,65 @@ func (*StateConfig) OTEL() OTEL { return GlobalState() } -func (s *StateConfig) GlobalOpts() *config.GlobalOpts { - return s.cfgData.Layers.Global +func (s *StateConfig) GlobalOpts() config.GlobalOpts { + return *s.cfgData.Layers.Global } func (*StateConfig) EndpointOTEL(_ *luraconfig.EndpointConfig) OTEL { return GlobalState() } -func (s *StateConfig) EndpointPipeOpts(_ *luraconfig.EndpointConfig) *config.PipeOpts { - return s.cfgData.Layers.Pipe +func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) config.PipeOpts { + PipeOpts := *s.cfgData.Layers.Pipe + cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) + if err == nil && cfgExtra.Layers.Pipe != nil { + PipeOpts.MetricsStaticAttributes = append( + PipeOpts.MetricsStaticAttributes, + cfgExtra.Layers.Global.MetricsStaticAttributes..., + ) + + PipeOpts.TracesStaticAttributes = append( + PipeOpts.TracesStaticAttributes, + cfgExtra.Layers.Global.TracesStaticAttributes..., + ) + } + + return PipeOpts } -func (s *StateConfig) EndpointBackendOpts(_ *luraconfig.Backend) *config.BackendOpts { - return s.cfgData.Layers.Backend +func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) config.BackendOpts { + return mergedBackendOpts(s, cfg) } func (*StateConfig) BackendOTEL(_ *luraconfig.Backend) OTEL { return GlobalState() } -func (s *StateConfig) BackendOpts(_ *luraconfig.Backend) *config.BackendOpts { - return s.cfgData.Layers.Backend +func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) config.BackendOpts { + return mergedBackendOpts(s, cfg) +} + +func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) config.BackendOpts { + BackendOpts := *s.cfgData.Layers.Backend + + cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) + if err == nil && cfgExtra.Layers.Backend != nil { + if cfgExtra.Layers.Backend.Metrics != nil { + BackendOpts.Metrics.StaticAttributes = append( + BackendOpts.Metrics.StaticAttributes, + cfgExtra.Layers.Backend.Metrics.StaticAttributes..., + ) + } + + if cfgExtra.Layers.Backend.Traces != nil { + BackendOpts.Traces.StaticAttributes = append( + BackendOpts.Traces.StaticAttributes, + cfgExtra.Layers.Backend.Traces.StaticAttributes..., + ) + } + } + + return BackendOpts } func (s *StateConfig) SkipEndpoint(endpoint string) bool { From 645ccb2f72e74aea01d1121913135d2c47a8416f Mon Sep 17 00:00:00 2001 From: Silvio Sepulveda Date: Fri, 26 Apr 2024 09:26:37 +0200 Subject: [PATCH 4/9] Added new static labels examples to backend example server config --- example/Makefile | 4 +- .../conf/krakend_back/configuration.json | 96 +++++++++++++++++-- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/example/Makefile b/example/Makefile index 4df2068..b60abf7 100644 --- a/example/Makefile +++ b/example/Makefile @@ -1,8 +1,8 @@ KRAKEND_LOCAL_IP ?= 192.168.1.12 -build: +srv: go build -o srv ./server -.PHONY: build +.PHONY: srv image: docker build -f ./server/Dockerfile -t krakend-otel-example:latest .. diff --git a/example/docker_compose/conf/krakend_back/configuration.json b/example/docker_compose/conf/krakend_back/configuration.json index e8c42ad..4a9db0e 100644 --- a/example/docker_compose/conf/krakend_back/configuration.json +++ b/example/docker_compose/conf/krakend_back/configuration.json @@ -28,6 +28,40 @@ }, { "endpoint": "/back_combination/{id}", + "extra_config": { + "telemetry/opentelemetry": { + "layers": { + "global": { + "metrics_static_attributes": [ + { + "key": "my_metric_global_override_attr", + "value": "my_metric_global_override_val" + } + ], + "traces_static_attributes": [ + { + "key": "my_trace_global_override_attr", + "value": "my_trace_global_override_val" + } + ] + }, + "proxy": { + "metrics_static_attributes": [ + { + "key": "my_metric_proxy_override_attr", + "value": "my_metric_proxy_override_val" + } + ], + , + "traces_static_attributes": [ + { + "key": "my_trace_proxy_override_attr", + "value": "my_trace_proxy_override_val" + } + ] + } + } + }, "backend": [ { "host": [ @@ -37,6 +71,30 @@ "is_collection": true, "mapping": { "collection": "posts" + }, + "extra_config": { + "telemetry/opentelemetry": { + "layers": { + "backend": { + "metrics": { + "static_attributes": [ + { + "key": "my_metric_backend_override_attr", + "value": "my_metric_backend_override_val" + } + ] + }, + "traces": { + "static_attributes": [ + { + "key": "my_trace_backend_override_attr", + "value": "my_trace_backend_override_val" + } + ] + } + } + } + } } }, { @@ -60,12 +118,36 @@ "global": { "disable_metrics": false, "disable_traces": false, - "disable_propagation": false + "disable_propagation": false, + "metrics_static_attributes": [ + { + "key": "my_metric_global_attr", + "value": "my_metric_global_val" + } + ], + "traces_static_attributes": [ + { + "key": "my_trace_global_attr", + "value": "my_trace_global_val" + } + ] }, "proxy": { "disable_metrics": false, - "disable_traces": false - }, + "disable_traces": false, + "metrics_static_attributes": [ + { + "key": "my_metric_proxy_attr", + "value": "my_metric_proxy_val" + } + ], + "traces_static_attributes": [ + { + "key": "my_trace_proxy_attr", + "value": "my_trace_proxy_val" + } + ] + }, "backend": { "metrics": { "disable_stage": false, @@ -74,8 +156,8 @@ "detailed_connection": true, "static_attributes": [ { - "key": "my_metric_attr", - "value": "my_metric_val" + "key": "my_metric_backend_attr", + "value": "my_metric_backend_val" } ] }, @@ -86,8 +168,8 @@ "detailed_connection": true, "static_attributes": [ { - "key": "my_trace_attr", - "value": "my_trace_val" + "key": "my_trace_backend_attr", + "value": "my_trace_backend_val" } ] } From 09c95ec3a9339d7dd8e0a3cb2f4932ea2d9f8b34 Mon Sep 17 00:00:00 2001 From: Silvio Sepulveda Date: Fri, 26 Apr 2024 15:17:34 +0200 Subject: [PATCH 5/9] Added some testing --- config/lura.go | 2 + http/server/server.go | 30 +++--- http/server/tracking.go | 13 +-- router/gin/endpoint.go | 7 +- state/config.go | 9 +- state/config_test.go | 225 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 253 insertions(+), 33 deletions(-) create mode 100644 state/config_test.go diff --git a/config/lura.go b/config/lura.go index 38be5ef..9e8c32c 100644 --- a/config/lura.go +++ b/config/lura.go @@ -41,6 +41,8 @@ func FromLura(srvCfg luraconfig.ServiceConfig) (*ConfigData, error) { return cfg, nil } +// LuraExtraCfg extracts the extra config field for the namespace if +// provided func LuraExtraCfg(extraCfg luraconfig.ExtraConfig) (*ConfigData, error) { tmp, ok := extraCfg[Namespace] if !ok { diff --git a/http/server/server.go b/http/server/server.go index 5116ce1..ba56235 100644 --- a/http/server/server.go +++ b/http/server/server.go @@ -72,29 +72,27 @@ func NewTrackingHandler(next http.Handler) http.Handler { prop = s.Propagator() } - // Add configured static attributes - metricsAttrs := []attribute.KeyValue{} - tracesAttrs := []attribute.KeyValue{} - for _, kv := range gCfg.MetricsStaticAttributes { - if len(kv.Key) > 0 && len(kv.Value) > 0 { - metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) - } - } - - for _, kv := range gCfg.TracesStaticAttributes { - if len(kv.Key) > 0 && len(kv.Value) > 0 { - tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) - } - } - var m *metricsHTTP if !gCfg.DisableMetrics { + var metricsAttrs []attribute.KeyValue + for _, kv := range gCfg.MetricsStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) + } + } + m = newMetricsHTTP(s.Meter(), metricsAttrs) } var t *tracesHTTP if !gCfg.DisableTraces { - tracesAttrs = append(tracesAttrs, attribute.String("krakend.stage", "global")) + tracesAttrs := []attribute.KeyValue{attribute.String("krakend.stage", "global")} + for _, kv := range gCfg.TracesStaticAttributes { + if len(kv.Key) > 0 && len(kv.Value) > 0 { + tracesAttrs = append(tracesAttrs, attribute.String(kv.Key, kv.Value)) + } + } + t = newTracesHTTP(s.Tracer(), tracesAttrs, gCfg.ReportHeaders) } diff --git a/http/server/tracking.go b/http/server/tracking.go index 1c411bd..4807738 100644 --- a/http/server/tracking.go +++ b/http/server/tracking.go @@ -75,15 +75,12 @@ func SetEndpointPattern(ctx context.Context, endpointPattern string) { } } -func SetMetricsStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { +// SetStaticAttributtes allows to set metrics and traces static attributes in +// the request context +func SetStaticAttributtes(ctx context.Context, metricAttrs, tracesAttrs []attribute.KeyValue) { if t := fromContext(ctx); t != nil { - t.metricsStaticAttrs = attrs - } -} - -func SetTracesStaticAttributtes(ctx context.Context, attrs []attribute.KeyValue) { - if t := fromContext(ctx); t != nil { - t.tracesStaticAttrs = attrs + t.metricsStaticAttrs = metricAttrs + t.tracesStaticAttrs = tracesAttrs } } diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index 2e1531b..c14afbf 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -24,8 +24,8 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { } urlPattern := kotelconfig.NormalizeURLPattern(cfg.Endpoint) next := hf(cfg, p) - metricsAttrs := []attribute.KeyValue{} - tracesAttrs := []attribute.KeyValue{} + var metricsAttrs []attribute.KeyValue + var tracesAttrs []attribute.KeyValue cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) if err == nil && cfgExtra.Layers.Global != nil { @@ -47,8 +47,7 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { // context by the outer http layer, so it can be reported // in metrics and traces. kotelserver.SetEndpointPattern(c.Request.Context(), urlPattern) - kotelserver.SetMetricsStaticAttributtes(c.Request.Context(), metricsAttrs) - kotelserver.SetTracesStaticAttributtes(c.Request.Context(), tracesAttrs) + kotelserver.SetStaticAttributtes(c.Request.Context(), metricsAttrs, tracesAttrs) next(c) } } diff --git a/state/config.go b/state/config.go index 25c09fe..171cf73 100644 --- a/state/config.go +++ b/state/config.go @@ -2,7 +2,6 @@ package state import ( "github.com/krakend/krakend-otel/config" - kotelconfig "github.com/krakend/krakend-otel/config" luraconfig "github.com/luraproject/lura/v2/config" ) @@ -42,16 +41,16 @@ func (*StateConfig) EndpointOTEL(_ *luraconfig.EndpointConfig) OTEL { func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) config.PipeOpts { PipeOpts := *s.cfgData.Layers.Pipe - cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) + cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) if err == nil && cfgExtra.Layers.Pipe != nil { PipeOpts.MetricsStaticAttributes = append( PipeOpts.MetricsStaticAttributes, - cfgExtra.Layers.Global.MetricsStaticAttributes..., + cfgExtra.Layers.Pipe.MetricsStaticAttributes..., ) PipeOpts.TracesStaticAttributes = append( PipeOpts.TracesStaticAttributes, - cfgExtra.Layers.Global.TracesStaticAttributes..., + cfgExtra.Layers.Pipe.TracesStaticAttributes..., ) } @@ -73,7 +72,7 @@ func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) config.BackendOpts { func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) config.BackendOpts { BackendOpts := *s.cfgData.Layers.Backend - cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) + cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) if err == nil && cfgExtra.Layers.Backend != nil { if cfgExtra.Layers.Backend.Metrics != nil { BackendOpts.Metrics.StaticAttributes = append( diff --git a/state/config_test.go b/state/config_test.go new file mode 100644 index 0000000..763f14c --- /dev/null +++ b/state/config_test.go @@ -0,0 +1,225 @@ +package state + +import ( + "testing" + + "github.com/krakend/krakend-otel/config" + luraconfig "github.com/luraproject/lura/v2/config" +) + +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), + } + + pipeCfg := &luraconfig.EndpointConfig{ + ExtraConfig: map[string]interface{}{ + "telemetry/opentelemetry": makePipeConf(overrideMetricAttrs, overrideTraceAttrs), + }, + } + + pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg) + + if len(pipeOpts.MetricsStaticAttributes) != len(expectedMetricAttrs) { + t.Errorf( + "Incorrect number of attributes for metrics. returned: %+v - expected: %+v", + pipeOpts.MetricsStaticAttributes, + expectedMetricAttrs, + ) + } + + if len(pipeOpts.TracesStaticAttributes) != len(expectedTraceAttrs) { + t.Errorf( + "Incorrect number of attributes for traces. returned: %+v - expected: %+v", + pipeOpts.TracesStaticAttributes, + expectedTraceAttrs, + ) + } +} + +func TestEndpointPipeNoOverride(t *testing.T) { + stateCfg := &StateConfig{ + cfgData: makePipeConf(makeGlobalMetricAttr(), makeGlobalTraceAttr()), + } + + // Empty config + pipeCfg := &luraconfig.EndpointConfig{ + ExtraConfig: map[string]interface{}{}, + } + + pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg) + + if len(pipeOpts.MetricsStaticAttributes) > 1 { + t.Errorf( + "Incorrect number of attributes for metrics. returned: %+v", + pipeOpts.MetricsStaticAttributes, + ) + } +} + +func TestEndpointPipeConfigOnlyOverride(t *testing.T) { + stateCfg := &StateConfig{ + cfgData: makePipeConf([]config.KeyValue{}, []config.KeyValue{}), + } + + pipeCfg := &luraconfig.EndpointConfig{ + ExtraConfig: map[string]interface{}{ + "telemetry/opentelemetry": makePipeConf(makeOverrideMetricAttr(), makeOverrideTraceAttr()), + }, + } + + pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg) + + if len(pipeOpts.MetricsStaticAttributes) > 1 { + t.Errorf( + "Incorrect number of attributes for metrics. returned: %+v", + pipeOpts.MetricsStaticAttributes, + ) + } +} + +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), + } + + backendCfg := &luraconfig.Backend{ + ExtraConfig: map[string]interface{}{ + "telemetry/opentelemetry": makeBackendConf(overrideMetricAttrs, overrideTraceAttrs), + }, + } + + backendOpts := stateCfg.BackendOpts(backendCfg) + + if len(backendOpts.Metrics.StaticAttributes) != len(expectedMetricAttrs) { + t.Errorf( + "Incorrect number of attributes for metrics. returned: %+v - expected: %+v", + backendOpts.Metrics.StaticAttributes, + expectedMetricAttrs, + ) + } + + if len(backendOpts.Traces.StaticAttributes) != len(expectedTraceAttrs) { + t.Errorf( + "Incorrect number of attributes for traces. returned: %+v - expected: %+v", + backendOpts.Traces.StaticAttributes, + expectedTraceAttrs, + ) + } +} + +func TestBackendConfigNoOverride(t *testing.T) { + stateCfg := &StateConfig{ + cfgData: makeBackendConf(makeGlobalMetricAttr(), makeGlobalTraceAttr()), + } + + // Empty config + backendCfg := &luraconfig.Backend{ + ExtraConfig: map[string]interface{}{}, + } + + 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{}), + } + + 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", + backendOpts.Traces.StaticAttributes, + ) + } +} + +func makePipeConf(metricAttrs, traceAttrs []config.KeyValue) config.ConfigData { + return config.ConfigData{ + Layers: &config.LayersOpts{ + Pipe: makePipeOpts(metricAttrs, traceAttrs), + }, + } +} + +func makePipeOpts(metricAttrs, traceAttrs []config.KeyValue) *config.PipeOpts { + return &config.PipeOpts{ + MetricsStaticAttributes: metricAttrs, + TracesStaticAttributes: traceAttrs, + } +} + +func makeBackendConf(metricAttrs, traceAttrs []config.KeyValue) config.ConfigData { + return config.ConfigData{ + Layers: &config.LayersOpts{ + Backend: makeBackendOpts(metricAttrs, traceAttrs), + }, + } +} + +func makeBackendOpts(metricAttrs, traceAttrs []config.KeyValue) *config.BackendOpts { + return &config.BackendOpts{ + Metrics: &config.BackendMetricOpts{ + StaticAttributes: metricAttrs, + }, + Traces: &config.BackendTraceOpts{ + StaticAttributes: traceAttrs, + }, + } +} + +func makeGlobalMetricAttr() []config.KeyValue { + return makeStaticAttr("my_metric_key", "my_metric_value") +} + +func makeOverrideMetricAttr() []config.KeyValue { + return makeStaticAttr("my_metric_override_key", "my_metric_override_value") +} + +func makeGlobalTraceAttr() []config.KeyValue { + return makeStaticAttr("my_trace_key", "my_trace_value") +} + +func makeOverrideTraceAttr() []config.KeyValue { + return makeStaticAttr("my_trace_override_key", "my_trace_override_value") +} + +func makeStaticAttr(key, value string) []config.KeyValue { + return []config.KeyValue{ + config.KeyValue{ + Key: key, + Value: value, + }, + } +} From 80b03e91a9bbd308517a55ef5d4d9052c3f4392e Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Thu, 16 May 2024 19:33:54 +0200 Subject: [PATCH 6/9] some fixes and suggestions to static attributes PR --- lura/proxy.go | 4 ++ lura/proxy_meter.go | 6 +++ lura/proxy_tracer.go | 3 ++ router/gin/endpoint.go | 2 +- state/config.go | 109 ++++++++++++++++++++++++++++------------- state/state.go | 3 ++ 6 files changed, 91 insertions(+), 36 deletions(-) diff --git a/lura/proxy.go b/lura/proxy.go index 4bc87ae..d834913 100644 --- a/lura/proxy.go +++ b/lura/proxy.go @@ -63,6 +63,10 @@ func middleware(gs state.OTEL, metricsEnabled bool, tracesEnabled bool, } if tracesEnabled { mt = newMiddlewareTracer(gs, urlPattern, stageName, reportHeaders, tracesAttrs) + if mt == nil { + // TODO: log the error + tracesEnabled = false + } } return func(next ...proxy.Proxy) proxy.Proxy { diff --git a/lura/proxy_meter.go b/lura/proxy_meter.go index 22e4a87..5fe3160 100644 --- a/lura/proxy_meter.go +++ b/lura/proxy_meter.go @@ -19,10 +19,16 @@ type middlewareMeter struct { } func newMiddlewareMeter(s state.OTEL, stageName string, attrs []attribute.KeyValue) (*middlewareMeter, error) { + if s == nil { + return nil, errors.New("no OTEL state provided") + } mAttrs := make([]attribute.KeyValue, 0, len(attrs)) mAttrs = append(mAttrs, attrs...) meter := s.Meter() + if meter == nil { + return nil, errors.New("OTEL state returned nil meter") + } var err error durationName := "krakend." + stageName + ".duration" duration, err := meter.Float64Histogram(durationName, kotelconfig.TimeBucketsOpt) diff --git a/lura/proxy_tracer.go b/lura/proxy_tracer.go index 790f31b..ca55e22 100644 --- a/lura/proxy_tracer.go +++ b/lura/proxy_tracer.go @@ -23,6 +23,9 @@ type middlewareTracer struct { func newMiddlewareTracer(s state.OTEL, name string, stageName string, reportHeaders bool, attrs []attribute.KeyValue) *middlewareTracer { tracer := s.Tracer() + if tracer == nil { + return nil + } tAttrs := make([]attribute.KeyValue, 0, len(attrs)+1) tAttrs = append(tAttrs, attrs...) tAttrs = append(tAttrs, attribute.String("krakend.stage", stageName)) diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index c14afbf..382864d 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -28,7 +28,7 @@ func New(hf krakendgin.HandlerFactory) krakendgin.HandlerFactory { var tracesAttrs []attribute.KeyValue cfgExtra, err := kotelconfig.LuraExtraCfg(cfg.ExtraConfig) - if err == nil && cfgExtra.Layers.Global != nil { + if err == nil && cfgExtra.Layers != nil && cfgExtra.Layers.Global != nil { for _, kv := range cfgExtra.Layers.Global.MetricsStaticAttributes { if len(kv.Key) > 0 && len(kv.Value) > 0 { metricsAttrs = append(metricsAttrs, attribute.String(kv.Key, kv.Value)) diff --git a/state/config.go b/state/config.go index 171cf73..41c901a 100644 --- a/state/config.go +++ b/state/config.go @@ -7,15 +7,15 @@ import ( type Config interface { OTEL() OTEL - GlobalOpts() config.GlobalOpts + GlobalOpts() *config.GlobalOpts // Gets the OTEL instance for a given endpoint EndpointOTEL(cfg *luraconfig.EndpointConfig) OTEL - EndpointPipeOpts(cfg *luraconfig.EndpointConfig) config.PipeOpts - EndpointBackendOpts(cfg *luraconfig.Backend) config.BackendOpts + EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts + EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts BackendOTEL(cfg *luraconfig.Backend) OTEL - BackendOpts(cfg *luraconfig.Backend) config.BackendOpts + BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts // SkipEndpoint tells if an endpoint should not be instrumented SkipEndpoint(endpoint string) bool @@ -31,33 +31,53 @@ func (*StateConfig) OTEL() OTEL { return GlobalState() } -func (s *StateConfig) GlobalOpts() config.GlobalOpts { - return *s.cfgData.Layers.Global +func (s *StateConfig) GlobalOpts() *config.GlobalOpts { + return s.cfgData.Layers.Global } func (*StateConfig) EndpointOTEL(_ *luraconfig.EndpointConfig) OTEL { return GlobalState() } -func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) config.PipeOpts { - PipeOpts := *s.cfgData.Layers.Pipe +func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts { + var sOpts *config.PipeOpts + var extraPOpts *config.PipeOpts + + if s != nil && s.cfgData.Layers != nil { + sOpts = s.cfgData.Layers.Pipe + } + cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err == nil && cfgExtra.Layers.Pipe != nil { - PipeOpts.MetricsStaticAttributes = append( - PipeOpts.MetricsStaticAttributes, - cfgExtra.Layers.Pipe.MetricsStaticAttributes..., - ) + if err != nil || cfgExtra == nil || cfgExtra.Layers == nil { + extraPOpts = cfgExtra.Layers.Pipe + } - PipeOpts.TracesStaticAttributes = append( - PipeOpts.TracesStaticAttributes, - cfgExtra.Layers.Pipe.TracesStaticAttributes..., - ) + if extraPOpts == nil { + if sOpts == nil { + return new(config.PipeOpts) + } + return sOpts + } else if sOpts == nil { + return extraPOpts } - return 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 } -func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) config.BackendOpts { +func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { return mergedBackendOpts(s, cfg) } @@ -65,31 +85,50 @@ func (*StateConfig) BackendOTEL(_ *luraconfig.Backend) OTEL { return GlobalState() } -func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) config.BackendOpts { +func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { return mergedBackendOpts(s, cfg) } -func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) config.BackendOpts { - BackendOpts := *s.cfgData.Layers.Backend +func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) *config.BackendOpts { + var extraBOpts *config.BackendOpts + var sOpts *config.BackendOpts + + if s != nil && s.cfgData.Layers != nil { + sOpts = s.cfgData.Layers.Backend + } cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err == nil && cfgExtra.Layers.Backend != nil { - if cfgExtra.Layers.Backend.Metrics != nil { - BackendOpts.Metrics.StaticAttributes = append( - BackendOpts.Metrics.StaticAttributes, - cfgExtra.Layers.Backend.Metrics.StaticAttributes..., - ) - } + if err == nil || cfgExtra != nil || cfgExtra.Layers != nil { + extraBOpts = cfgExtra.Layers.Backend + } - if cfgExtra.Layers.Backend.Traces != nil { - BackendOpts.Traces.StaticAttributes = append( - BackendOpts.Traces.StaticAttributes, - cfgExtra.Layers.Backend.Traces.StaticAttributes..., - ) + 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 extraBOpts.Traces != nil { + bOpts.Traces.StaticAttributes = append( + bOpts.Traces.StaticAttributes, + cfgExtra.Layers.Backend.Traces.StaticAttributes..., + ) } - return BackendOpts + return bOpts } func (s *StateConfig) SkipEndpoint(endpoint string) bool { diff --git a/state/state.go b/state/state.go index cd0dd22..c11dd21 100644 --- a/state/state.go +++ b/state/state.go @@ -126,6 +126,9 @@ func NewWithVersion(serviceName string, cfg *OTELStateConfig, version string, // Tracer returns a tracer to start a span. func (s *OTELState) Tracer() trace.Tracer { + if s == nil { + return nil + } return s.tracer } From ba24ed30035df9123df00803ab55ffed4dec9488 Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Thu, 16 May 2024 19:50:57 +0200 Subject: [PATCH 7/9] fix wrong conditions when checking for nil pointers --- state/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/state/config.go b/state/config.go index 41c901a..1d2e5ee 100644 --- a/state/config.go +++ b/state/config.go @@ -48,7 +48,7 @@ func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.P } cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err != nil || cfgExtra == nil || cfgExtra.Layers == nil { + if err == nil && cfgExtra != nil && cfgExtra.Layers != nil { extraPOpts = cfgExtra.Layers.Pipe } @@ -98,7 +98,7 @@ func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) *config.BackendO } cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig) - if err == nil || cfgExtra != nil || cfgExtra.Layers != nil { + if err == nil && cfgExtra != nil && cfgExtra.Layers != nil { extraBOpts = cfgExtra.Layers.Backend } From 5de766c326bb3e5c91e5a27dccbf93aa4d91eb7d Mon Sep 17 00:00:00 2001 From: "deepsource-autofix[bot]" <62050782+deepsource-autofix[bot]@users.noreply.github.com> Date: Thu, 16 May 2024 17:53:29 +0000 Subject: [PATCH 8/9] style: format code with Go fmt and Gofumpt This commit fixes the style issues introduced in ba24ed3 according to the output from Go fmt and Gofumpt. Details: https://github.com/krakend/krakend-otel/pull/27 --- state/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/state/config_test.go b/state/config_test.go index 763f14c..3fc84f9 100644 --- a/state/config_test.go +++ b/state/config_test.go @@ -217,7 +217,7 @@ func makeOverrideTraceAttr() []config.KeyValue { func makeStaticAttr(key, value string) []config.KeyValue { return []config.KeyValue{ - config.KeyValue{ + { Key: key, Value: value, }, From b341e0b885804b523fade7197758ce050bbb0993 Mon Sep 17 00:00:00 2001 From: David Hontecillas Date: Thu, 16 May 2024 20:04:11 +0200 Subject: [PATCH 9/9] make margedBackendOpts member of StateConfig --- state/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/state/config.go b/state/config.go index 1d2e5ee..d652157 100644 --- a/state/config.go +++ b/state/config.go @@ -78,7 +78,7 @@ func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.P } func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { - return mergedBackendOpts(s, cfg) + return s.mergedBackendOpts(cfg) } func (*StateConfig) BackendOTEL(_ *luraconfig.Backend) OTEL { @@ -86,10 +86,10 @@ func (*StateConfig) BackendOTEL(_ *luraconfig.Backend) OTEL { } func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { - return mergedBackendOpts(s, cfg) + return s.mergedBackendOpts(cfg) } -func mergedBackendOpts(s *StateConfig, cfg *luraconfig.Backend) *config.BackendOpts { +func (s *StateConfig) mergedBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts { var extraBOpts *config.BackendOpts var sOpts *config.BackendOpts