Use jaeger env for configuration and add chi path method (#8)

**What**
- Remove server parameter from the tracing configuration, this value
will now be set by Jaeger's own method via the env
- Add a new path cleaning method that uses Chi mux route lookup so that
we can support name based resolution on its own

Signed-off-by: Lucas Roesler <[email protected]>

Author: LucasRoesler

go.mod

@@ -3,12 +3,15 @@ module github.com/contiamo/go-base
 require (
 	github.com/bakins/net-http-recover v0.0.0-20141007104922-6cba69d01459
 	github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd // indirect
+	github.com/go-chi/chi v4.0.2+incompatible
 	github.com/golang/protobuf v1.3.2
 	github.com/google/uuid v1.1.1
 	github.com/gorilla/websocket v1.4.0
 	github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/lib/pq v1.1.1
+	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
+	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/opentracing/opentracing-go v1.1.0
 	github.com/pkg/errors v0.8.1 // indirect
 	github.com/prometheus/client_golang v1.1.0
@@ -23,6 +26,7 @@ require (
 	go.uber.org/atomic v1.4.0 // indirect
 	golang.org/x/lint v0.0.0-20190409202823-959b441ac422
 	golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b // indirect
+	google.golang.org/appengine v1.4.0 // indirect
 	google.golang.org/genproto v0.0.0-20190716160619-c506a9f90610 // indirect
 	google.golang.org/grpc v1.22.0
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect

go.sum

@@ -14,6 +14,8 @@ github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/go-chi/chi v4.0.2+incompatible h1:maB6vn6FqCxrpz4FqWdh4+lwpyZIQS7YEAUcHlgXVRs=
+github.com/go-chi/chi v4.0.2+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxmMeXJVKy9tTv1XzQ=
 github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
 github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=

pkg/http/middleware/metrics.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 	"time"
 
-	uuid "github.com/google/uuid"
 	"github.com/urfave/negroni"
 
 	server "github.com/contiamo/go-base/pkg/http"
@@ -95,19 +94,3 @@ func (opt *metricsOption) WrapHandler(handler http.Handler) http.Handler {
 
 	return mw
 }
-
-// PathWithCleanID replace string values that look like ids (uuids and int) with "*"
-func PathWithCleanID(r *http.Request) string {
-	pathParts := strings.Split(r.URL.Path, "/")
-	for i, part := range pathParts {
-		if _, err := uuid.Parse(part); err == nil {
-			pathParts[i] = "*"
-			continue
-		}
-		if _, err := strconv.Atoi(part); err == nil {
-			pathParts[i] = "*"
-		}
-
-	}
-	return strings.Join(pathParts, "/")
-}

pkg/http/middleware/path.go

@@ -0,0 +1,48 @@
+package middleware
+
+import (
+	"net/http"
+	"strconv"
+	"strings"
+
+	"github.com/go-chi/chi"
+	"github.com/google/uuid"
+)
+
+// PathWithCleanID replace string values that look like ids (uuids and int) with "*"
+func PathWithCleanID(r *http.Request) string {
+	pathParts := strings.Split(r.URL.Path, "/")
+	for i, part := range pathParts {
+		if _, err := uuid.Parse(part); err == nil {
+			pathParts[i] = "*"
+			continue
+		}
+		if _, err := strconv.Atoi(part); err == nil {
+			pathParts[i] = "*"
+		}
+
+	}
+	return strings.Join(pathParts, "/")
+}
+
+// MethodAndPathCleanID replace string values that look like ids (uuids and int)
+// with "*"
+func MethodAndPathCleanID(r *http.Request) string {
+	pathParts := strings.Split(r.URL.Path, "/")
+	for i, part := range pathParts {
+		if _, err := uuid.Parse(part); err == nil {
+			pathParts[i] = "*"
+			continue
+		}
+		if _, err := strconv.Atoi(part); err == nil {
+			pathParts[i] = "*"
+		}
+
+	}
+	return "HTTP " + r.Method + " " + strings.Join(pathParts, "/")
+}
+
+// ChiRouteName replace route parameters from the Chi route mux with "*"
+func ChiRouteName(r *http.Request) string {
+	return "HTTP " + r.Method + " " + chi.RouteContext(r.Context()).RoutePattern()
+}

pkg/http/middleware/tracing.go

@@ -2,35 +2,32 @@ package middleware
 
 import (
 	"net/http"
-	"strconv"
-	"strings"
 
-	uuid "github.com/google/uuid"
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/opentracing/opentracing-go/ext"
 	"github.com/urfave/negroni"
 
-	"github.com/contiamo/go-base/pkg/tracing"
 	server "github.com/contiamo/go-base/pkg/http"
+	"github.com/contiamo/go-base/pkg/tracing"
 )
 
 // mainly from "github.com/opentracing-contrib/go-stdlib/nethttp"
 
 // WithTracing configures tracing for that server; if configuration fails, WithTracing will panic
-func WithTracing(server, app string, tags map[string]string, opNameFunc func(r *http.Request) string) server.Option {
+func WithTracing(app string, tags map[string]string, opNameFunc func(r *http.Request) string) server.Option {
 	if opNameFunc == nil {
 		opNameFunc = MethodAndPathCleanID
 	}
-	if err := tracing.InitJaeger(server, app); err != nil {
+	if err := tracing.InitJaeger(app); err != nil {
 		panic(err)
 	}
-	return &tracingOption{server, app, tags, opNameFunc}
+	return &tracingOption{app, tags, opNameFunc}
 }
 
 type tracingOption struct {
-	server, app string
-	tags        map[string]string
-	opNameFunc  func(r *http.Request) string
+	app        string
+	tags       map[string]string
+	opNameFunc func(r *http.Request) string
 }
 
 func (opt *tracingOption) WrapHandler(handler http.Handler) http.Handler {
@@ -68,23 +65,6 @@ func operationNameFunc(f func(r *http.Request) string) mwOption {
 	}
 }
 
-// MethodAndPathCleanID replace string values that look like ids (uuids and int)
-// with "*"
-func MethodAndPathCleanID(r *http.Request) string {
-	pathParts := strings.Split(r.URL.Path, "/")
-	for i, part := range pathParts {
-		if _, err := uuid.Parse(part); err == nil {
-			pathParts[i] = "*"
-			continue
-		}
-		if _, err := strconv.Atoi(part); err == nil {
-			pathParts[i] = "*"
-		}
-
-	}
-	return "HTTP " + r.Method + " " + strings.Join(pathParts, "/")
-}
-
 // mwSpanObserver returns a MWOption that observe the span
 // for the server-side span.
 func mwSpanObserver(f func(span opentracing.Span, r *http.Request)) mwOption {

pkg/http/middleware/tracing_test.go

@@ -18,7 +18,7 @@ func Test_TracingMiddleware(t *testing.T) {
 	opentracing.SetGlobalTracer(tracer)
 
 	t.Run("should be possible to setup tracing", func(t *testing.T) {
-		srv, err := createServer([]server.Option{WithTracing("localhost:test", "test", nil, nil)})
+		srv, err := createServer([]server.Option{WithTracing("test", nil, nil)})
 		require.NoError(t, err)
 
 		ts := httptest.NewServer(srv.Handler)
@@ -34,7 +34,7 @@ func Test_TracingMiddleware(t *testing.T) {
 	t.Run("should be possible to set additional tags", func(t *testing.T) {
 		tagName := "testTag"
 		tagValue := "something to find"
-		srv, err := createServer([]server.Option{WithTracing("localhost:test", "test", map[string]string{tagName: tagValue}, nil)})
+		srv, err := createServer([]server.Option{WithTracing("test", map[string]string{tagName: tagValue}, nil)})
 		require.NoError(t, err)
 
 		ts := httptest.NewServer(srv.Handler)
@@ -51,7 +51,7 @@ func Test_TracingMiddleware(t *testing.T) {
 	})
 
 	t.Run("should replace uuid values with *", func(t *testing.T) {
-		srv, err := createServer([]server.Option{WithTracing("localhost:test", "test", nil, nil)})
+		srv, err := createServer([]server.Option{WithTracing("test", nil, nil)})
 		require.NoError(t, err)
 
 		ts := httptest.NewServer(srv.Handler)
@@ -68,7 +68,7 @@ func Test_TracingMiddleware(t *testing.T) {
 	})
 
 	t.Run("should allow websockets", func(t *testing.T) {
-		srv, err := createServer([]server.Option{WithTracing("localhost:test", "test", nil, nil)})
+		srv, err := createServer([]server.Option{WithTracing("test", nil, nil)})
 		require.NoError(t, err)
 
 		ts := httptest.NewServer(srv.Handler)

pkg/tracing/jaeger.go

@@ -14,14 +14,14 @@ var defaultJaegerServer = fmt.Sprintf("%s:%d", jaeger.DefaultUDPSpanServerHost,
 // InitJaeger asserts that the global tracer is initialized.
 //
 // This will read the configuration from the "JAEGER_*"" environment variables.
-// Overriding the empty values with the supplied server and app value.  If a
+// Overriding the empty values with the supplied app value.  If a
 // sampler type is not configured via the environment variables, then InitJaeger
 // will be configured with the constant sampler.
-func InitJaeger(server, app string) error {
+func InitJaeger(app string) error {
 	global := opentracing.GlobalTracer()
 	if _, ok := global.(opentracing.NoopTracer); ok {
 
-		cfg, err := getConfig(server, app)
+		cfg, err := getConfig(app)
 		if err != nil {
 			return err
 		}
@@ -34,7 +34,7 @@ func InitJaeger(server, app string) error {
 	return nil
 }
 
-func getConfig(server, app string) (*config.Configuration, error) {
+func getConfig(app string) (*config.Configuration, error) {
 	cfg, err := config.FromEnv()
 	if err != nil {
 		return nil, err
@@ -52,9 +52,5 @@ func getConfig(server, app string) (*config.Configuration, error) {
 		cfg.Reporter.BufferFlushInterval = 1 * time.Second
 	}
 
-	if server != "" && (cfg.Reporter.LocalAgentHostPort == "" || cfg.Reporter.LocalAgentHostPort == defaultJaegerServer) {
-		cfg.Reporter.LocalAgentHostPort = server
-	}
-
 	return cfg, nil
 }

pkg/tracing/jaeger_test.go

@@ -12,29 +12,35 @@ func Test_getConfig(t *testing.T) {
 	cases := []struct {
 		name           string
 		server         string
-		serverEnv      string
+		varname        string
 		expectedServer string
 	}{
-		{"set server from config", "example.com:6831", "", "example.com:6831"},
-		{"set server from ENV", "", "example.com", "example.com:6831"},
+		{"set local agent server from ENV", "example.com", "JAEGER_AGENT_HOST", "example.com:6831"},
+		{"set remote agent server from ENV", "http://example.com", "JAEGER_ENDPOINT", "http://example.com"},
+		{"use default local agent from env", "", "", defaultJaegerServer},
 	}
 
+	currentEnvEndpoint := os.Getenv("JAEGER_ENDPOINT")
 	currentEnvHost := os.Getenv("JAEGER_AGENT_HOST")
-	currentEnvPort := os.Getenv("JAEGER_AGENT_PORT")
+	defer os.Setenv("JAEGER_ENDPOINT", currentEnvEndpoint)
 	defer os.Setenv("JAEGER_AGENT_HOST", currentEnvHost)
-	defer os.Setenv("JAEGER_AGENT_PORT", currentEnvPort)
 
 	for _, tc := range cases {
-		os.Unsetenv("JAEGER_AGENT_HOST")
-		os.Unsetenv("JAEGER_AGENT_PORT")
-
 		t.Run(tc.name, func(t *testing.T) {
-			os.Setenv("JAEGER_AGENT_HOST", tc.serverEnv)
-			cfg, err := getConfig(tc.server, "test-app")
+			if tc.varname != "" {
+				os.Setenv(tc.varname, tc.server)
+				defer os.Unsetenv(tc.varname)
+			}
+
+			cfg, err := getConfig("test-app")
 			require.NoError(t, err)
 
-			cfg.Reporter.LocalAgentHostPort = tc.expectedServer
-			cfg.ServiceName = "test-app"
+			require.Equal(t, cfg.ServiceName, "test-app")
+			if tc.varname == "JAEGER_ENDPOINT" {
+				require.Equal(t, tc.expectedServer, cfg.Reporter.CollectorEndpoint)
+			} else {
+				require.Equal(t, tc.expectedServer, cfg.Reporter.LocalAgentHostPort)
+			}
 		})
 	}
 }