From 8519b0d3536143077a02fc349197b4cd5b72866c Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 16 Apr 2018 17:42:03 +0200 Subject: [PATCH] Fix nil value when tracing is enabled --- configuration/configuration.go | 39 ++++++++ configuration/configuration_test.go | 134 ++++++++++++++++++++++++--- middlewares/tracing/jaeger/jaeger.go | 2 +- middlewares/tracing/zipkin/zipkin.go | 3 + 4 files changed, 165 insertions(+), 13 deletions(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index d6f1f14ed..d29bb4465 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -11,6 +11,8 @@ import ( "github.com/containous/traefik/api" "github.com/containous/traefik/log" "github.com/containous/traefik/middlewares/tracing" + "github.com/containous/traefik/middlewares/tracing/jaeger" + "github.com/containous/traefik/middlewares/tracing/zipkin" "github.com/containous/traefik/ping" acmeprovider "github.com/containous/traefik/provider/acme" "github.com/containous/traefik/provider/boltdb" @@ -313,6 +315,43 @@ func (gc *GlobalConfiguration) SetEffectiveConfiguration(configFile string) { } gc.initACMEProvider() + gc.initTracing() +} + +func (gc *GlobalConfiguration) initTracing() { + if gc.Tracing != nil { + switch gc.Tracing.Backend { + case jaeger.Name: + if gc.Tracing.Jaeger == nil { + gc.Tracing.Jaeger = &jaeger.Config{ + SamplingServerURL: "http://localhost:5778/sampling", + SamplingType: "const", + SamplingParam: 1.0, + LocalAgentHostPort: "127.0.0.1:6832", + } + } + if gc.Tracing.Zipkin != nil { + log.Warn("Zipkin configuration will be ignored") + gc.Tracing.Zipkin = nil + } + case zipkin.Name: + if gc.Tracing.Zipkin == nil { + gc.Tracing.Zipkin = &zipkin.Config{ + HTTPEndpoint: "http://localhost:9411/api/v1/spans", + SameSpan: false, + ID128Bit: true, + Debug: false, + } + } + if gc.Tracing.Jaeger != nil { + log.Warn("Jaeger configuration will be ignored") + gc.Tracing.Jaeger = nil + } + default: + log.Warnf("Unknown tracer %q", gc.Tracing.Backend) + return + } + } } func (gc *GlobalConfiguration) initACMEProvider() { diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index df9bb9bc4..e8c5e7b47 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -5,14 +5,18 @@ import ( "time" "github.com/containous/flaeg" + "github.com/containous/traefik/middlewares/tracing" + "github.com/containous/traefik/middlewares/tracing/jaeger" + "github.com/containous/traefik/middlewares/tracing/zipkin" "github.com/containous/traefik/provider" "github.com/containous/traefik/provider/file" + "github.com/stretchr/testify/assert" ) const defaultConfigFile = "traefik.toml" func TestSetEffectiveConfigurationGraceTimeout(t *testing.T) { - tests := []struct { + testCases := []struct { desc string legacyGraceTimeout time.Duration lifeCycleGraceTimeout time.Duration @@ -37,10 +41,11 @@ func TestSetEffectiveConfigurationGraceTimeout(t *testing.T) { }, } - for _, test := range tests { + for _, test := range testCases { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel() + gc := &GlobalConfiguration{ GraceTimeOut: flaeg.Duration(test.legacyGraceTimeout), } @@ -52,17 +57,14 @@ func TestSetEffectiveConfigurationGraceTimeout(t *testing.T) { gc.SetEffectiveConfiguration(defaultConfigFile) - gotGraceTimeout := time.Duration(gc.LifeCycle.GraceTimeOut) - if gotGraceTimeout != test.wantGraceTimeout { - t.Fatalf("got effective grace timeout %d, want %d", gotGraceTimeout, test.wantGraceTimeout) - } + assert.Equal(t, test.wantGraceTimeout, time.Duration(gc.LifeCycle.GraceTimeOut)) }) } } func TestSetEffectiveConfigurationFileProviderFilename(t *testing.T) { - tests := []struct { + testCases := []struct { desc string fileProvider *file.Provider wantFileProviderFilename string @@ -84,20 +86,128 @@ func TestSetEffectiveConfigurationFileProviderFilename(t *testing.T) { }, } - for _, test := range tests { + for _, test := range testCases { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel() + gc := &GlobalConfiguration{ File: test.fileProvider, } gc.SetEffectiveConfiguration(defaultConfigFile) - gotFileProviderFilename := gc.File.Filename - if gotFileProviderFilename != test.wantFileProviderFilename { - t.Fatalf("got file provider file name %q, want %q", gotFileProviderFilename, test.wantFileProviderFilename) - } + assert.Equal(t, test.wantFileProviderFilename, gc.File.Filename) + }) + } +} + +func TestSetEffectiveConfigurationTracing(t *testing.T) { + testCases := []struct { + desc string + tracing *tracing.Tracing + expected *tracing.Tracing + }{ + { + desc: "no tracing configuration", + tracing: &tracing.Tracing{}, + expected: &tracing.Tracing{}, + }, + { + desc: "tracing bad backend name", + tracing: &tracing.Tracing{ + Backend: "powpow", + }, + expected: &tracing.Tracing{ + Backend: "powpow", + }, + }, + { + desc: "tracing jaeger backend name", + tracing: &tracing.Tracing{ + Backend: "jaeger", + Zipkin: &zipkin.Config{ + HTTPEndpoint: "http://localhost:9411/api/v1/spans", + SameSpan: false, + ID128Bit: true, + Debug: false, + }, + }, + expected: &tracing.Tracing{ + Backend: "jaeger", + Jaeger: &jaeger.Config{ + SamplingServerURL: "http://localhost:5778/sampling", + SamplingType: "const", + SamplingParam: 1.0, + LocalAgentHostPort: "127.0.0.1:6832", + }, + Zipkin: nil, + }, + }, + { + desc: "tracing zipkin backend name", + tracing: &tracing.Tracing{ + Backend: "zipkin", + Jaeger: &jaeger.Config{ + SamplingServerURL: "http://localhost:5778/sampling", + SamplingType: "const", + SamplingParam: 1.0, + LocalAgentHostPort: "127.0.0.1:6832", + }, + }, + expected: &tracing.Tracing{ + Backend: "zipkin", + Jaeger: nil, + Zipkin: &zipkin.Config{ + HTTPEndpoint: "http://localhost:9411/api/v1/spans", + SameSpan: false, + ID128Bit: true, + Debug: false, + }, + }, + }, + { + desc: "tracing zipkin backend name value override", + tracing: &tracing.Tracing{ + Backend: "zipkin", + Jaeger: &jaeger.Config{ + SamplingServerURL: "http://localhost:5778/sampling", + SamplingType: "const", + SamplingParam: 1.0, + LocalAgentHostPort: "127.0.0.1:6832", + }, + Zipkin: &zipkin.Config{ + HTTPEndpoint: "http://powpow:9411/api/v1/spans", + SameSpan: true, + ID128Bit: true, + Debug: true, + }, + }, + expected: &tracing.Tracing{ + Backend: "zipkin", + Jaeger: nil, + Zipkin: &zipkin.Config{ + HTTPEndpoint: "http://powpow:9411/api/v1/spans", + SameSpan: true, + ID128Bit: true, + Debug: true, + }, + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + gc := &GlobalConfiguration{ + Tracing: test.tracing, + } + + gc.SetEffectiveConfiguration(defaultConfigFile) + + assert.Equal(t, test.expected, gc.Tracing) }) } } diff --git a/middlewares/tracing/jaeger/jaeger.go b/middlewares/tracing/jaeger/jaeger.go index f363566b0..6a130f18d 100644 --- a/middlewares/tracing/jaeger/jaeger.go +++ b/middlewares/tracing/jaeger/jaeger.go @@ -48,7 +48,7 @@ func (c *Config) Setup(componentName string) (opentracing.Tracer, io.Closer, err log.Warnf("Could not initialize jaeger tracer: %s", err.Error()) return nil, nil, err } - log.Debugf("jaeger tracer configured", err) + log.Debug("Jaeger tracer configured") return opentracing.GlobalTracer(), closer, nil } diff --git a/middlewares/tracing/zipkin/zipkin.go b/middlewares/tracing/zipkin/zipkin.go index f576b349a..2a884810c 100644 --- a/middlewares/tracing/zipkin/zipkin.go +++ b/middlewares/tracing/zipkin/zipkin.go @@ -3,6 +3,7 @@ package zipkin import ( "io" + "github.com/containous/traefik/log" opentracing "github.com/opentracing/opentracing-go" zipkin "github.com/openzipkin/zipkin-go-opentracing" ) @@ -39,5 +40,7 @@ func (c *Config) Setup(serviceName string) (opentracing.Tracer, io.Closer, error // Without this, child spans are getting the NOOP tracer opentracing.SetGlobalTracer(tracer) + log.Debug("Zipkin tracer configured") + return tracer, collector, nil }