Update to jaeger-lib 2 and latest sha for jaeger-client-go, to pick u… (

#1282) * Update to jaeger-lib 2 and latest sha for jaeger-client-go, to pick up refactored metric names Signed-off-by: Gary Brown <[email protected]> * Change go tracer metric root namespace so now 'jaeger_tracer_' rather than 'jaeger_client_jaeger_tracer' Signed-off-by: Gary Brown <[email protected]> * Update the changelog Signed-off-by: Gary Brown <[email protected]> * Update jaeger go client to latest SHA, to fix jaeger tracer metric namespace separator Signed-off-by: Gary Brown <[email protected]>
jaegertracing · Jan 17, 2019 · 7cbb25d · 7cbb25d
1 parent 94e9228
commit 7cbb25d
Show file tree

Hide file tree

Showing 57 changed files with 264 additions and 251 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,16 @@ Changes by Version
 
 ##### Breaking Changes
 
+- Update to jaeger-lib 2 and latest sha for jaeger-client-go, to pick up refactored metric names ([#1282](https://github.com/jaegertracing/jaeger/pull/1282), [@objectiser](https://github.com/objectiser))
+
+Update to latest version of `jaeger-lib`, which includes a change to the naming of counters exported to
+prometheus, to follow the convention of using a `_total` suffix, e.g. `jaeger_query_requests` is now
+`jaeger_query_requests_total`.
+
+Jaeger go client metrics, previously under the namespace `jaeger_client_jaeger_` are now under
+`jaeger_tracer_`.
+
+
 - Add gRPC metrics to agent ([#1180](https://github.com/jaegertracing/jaeger/pull/1180), [@pavolloffay](https://github.com/pavolloffay))
 
 The following metrics:

diff --git a/Gopkg.lock b/Gopkg.lock
diff --git a/Gopkg.toml b/Gopkg.toml
@@ -81,11 +81,11 @@ required = [
 
 [[constraint]]
   name = "github.com/uber/jaeger-client-go"
-  version = "^2.15.0"
+  revision = "6733ee486c780528f2c8088305e16fdb685134c7"
 
 [[constraint]]
   name = "github.com/uber/jaeger-lib"
-  version = "^1.5.0"
+  version = "^2.0.0"
 
 [[constraint]]
   name = "github.com/uber/tchannel-go"

diff --git a/cmd/agent/app/builder.go b/cmd/agent/app/builder.go
@@ -141,10 +141,10 @@ func (b *Builder) getProcessors(rep reporter.Reporter, mFactory metrics.Factory,
 		default:
 			return nil, fmt.Errorf("cannot find agent processor for data model %v", cfg.Model)
 		}
-		metrics := mFactory.Namespace("", map[string]string{
+		metrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{
 			"protocol": string(cfg.Protocol),
 			"model":    string(cfg.Model),
-		})
+		}})
 		processor, err := cfg.GetThriftProcessor(metrics, protoFactory, handler, logger)
 		if err != nil {
 			return nil, err

diff --git a/cmd/agent/app/configmanager/metrics_test.go b/cmd/agent/app/configmanager/metrics_test.go
@@ -21,8 +21,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/uber/jaeger-lib/metrics"
-	mTestutils "github.com/uber/jaeger-lib/metrics/testutils"
+	"github.com/uber/jaeger-lib/metrics/metricstest"
 
 	"github.com/jaegertracing/jaeger/thrift-gen/baggage"
 	"github.com/jaegertracing/jaeger/thrift-gen/sampling"
@@ -46,16 +45,16 @@ func (noopManager) GetBaggageRestrictions(s string) ([]*baggage.BaggageRestricti
 
 func TestMetrics(t *testing.T) {
 	tests := []struct {
-		expected []mTestutils.ExpectedMetric
+		expected []metricstest.ExpectedMetric
 		err      error
 	}{
-		{expected: []mTestutils.ExpectedMetric{
+		{expected: []metricstest.ExpectedMetric{
 			{Name: "collector-proxy", Tags: map[string]string{"result": "ok", "endpoint": "sampling"}, Value: 1},
 			{Name: "collector-proxy", Tags: map[string]string{"result": "err", "endpoint": "sampling"}, Value: 0},
 			{Name: "collector-proxy", Tags: map[string]string{"result": "ok", "endpoint": "baggage"}, Value: 1},
 			{Name: "collector-proxy", Tags: map[string]string{"result": "err", "endpoint": "baggage"}, Value: 0},
 		}},
-		{expected: []mTestutils.ExpectedMetric{
+		{expected: []metricstest.ExpectedMetric{
 			{Name: "collector-proxy", Tags: map[string]string{"result": "ok", "endpoint": "sampling"}, Value: 0},
 			{Name: "collector-proxy", Tags: map[string]string{"result": "err", "endpoint": "sampling"}, Value: 1},
 			{Name: "collector-proxy", Tags: map[string]string{"result": "ok", "endpoint": "baggage"}, Value: 0},
@@ -64,7 +63,7 @@ func TestMetrics(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		metricsFactory := metrics.NewLocalFactory(time.Microsecond)
+		metricsFactory := metricstest.NewFactory(time.Microsecond)
 		mgr := WrapWithMetrics(&noopManager{}, metricsFactory)
 
 		if test.err != nil {
@@ -82,6 +81,6 @@ func TestMetrics(t *testing.T) {
 			require.NoError(t, err)
 			require.NotNil(t, b)
 		}
-		mTestutils.AssertCounterMetrics(t, metricsFactory, test.expected...)
+		metricsFactory.AssertCounterMetrics(t, test.expected...)
 	}
 }
diff --git a/cmd/agent/app/httpserver/server_test.go b/cmd/agent/app/httpserver/server_test.go
@@ -25,17 +25,15 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/uber/jaeger-lib/metrics"
-	"github.com/uber/jaeger-lib/metrics/testutils"
-	mTestutils "github.com/uber/jaeger-lib/metrics/testutils"
+	"github.com/uber/jaeger-lib/metrics/metricstest"
 
 	tSampling092 "github.com/jaegertracing/jaeger/cmd/agent/app/httpserver/thrift-0.9.2"
 	"github.com/jaegertracing/jaeger/thrift-gen/baggage"
 	"github.com/jaegertracing/jaeger/thrift-gen/sampling"
 )
 
 type testServer struct {
-	metricsFactory *metrics.LocalFactory
+	metricsFactory *metricstest.Factory
 	mgr            *mockManager
 	server         *httptest.Server
 }
@@ -45,7 +43,7 @@ func withServer(
 	mockBaggageResponse []*baggage.BaggageRestriction,
 	runTest func(server *testServer),
 ) {
-	metricsFactory := metrics.NewLocalFactory(0)
+	metricsFactory := metricstest.NewFactory(0)
 	mgr := &mockManager{
 		samplingResponse: mockSamplingResponse,
 		baggageResponse:  mockBaggageResponse,
@@ -100,7 +98,7 @@ func TestHTTPHandler(t *testing.T) {
 		})
 
 		// handler must emit metrics
-		testutils.AssertCounterMetrics(t, ts.metricsFactory, []testutils.ExpectedMetric{
+		ts.metricsFactory.AssertCounterMetrics(t, []metricstest.ExpectedMetric{
 			{Name: "http-server.requests", Tags: map[string]string{"type": "sampling"}, Value: 1},
 			{Name: "http-server.requests", Tags: map[string]string{"type": "sampling-legacy"}, Value: 1},
 			{Name: "http-server.requests", Tags: map[string]string{"type": "baggage"}, Value: 1},
@@ -116,14 +114,14 @@ func TestHTTPHandlerErrors(t *testing.T) {
 		url                  string
 		statusCode           int
 		body                 string
-		metrics              []mTestutils.ExpectedMetric
+		metrics              []metricstest.ExpectedMetric
 	}{
 		{
 			description: "no service name",
 			url:         "",
 			statusCode:  http.StatusBadRequest,
 			body:        "'service' parameter must be provided once\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "all", "status": "4xx"}, Value: 1},
 			},
 		},
@@ -132,7 +130,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			url:         "?service=Y&service=Y",
 			statusCode:  http.StatusBadRequest,
 			body:        "'service' parameter must be provided once\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "all", "status": "4xx"}, Value: 1},
 			},
 		},
@@ -141,7 +139,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			url:         "/baggageRestrictions?service=Y&service=Y",
 			statusCode:  http.StatusBadRequest,
 			body:        "'service' parameter must be provided once\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "all", "status": "4xx"}, Value: 1},
 			},
 		},
@@ -150,7 +148,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			url:         "?service=Y",
 			statusCode:  http.StatusInternalServerError,
 			body:        "collector error: no mock response provided\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "collector-proxy", "status": "5xx"}, Value: 1},
 			},
 		},
@@ -159,7 +157,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			url:         "/baggageRestrictions?service=Y",
 			statusCode:  http.StatusInternalServerError,
 			body:        "collector error: no mock response provided\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "collector-proxy", "status": "5xx"}, Value: 1},
 			},
 		},
@@ -169,7 +167,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			url:                  "?service=Y",
 			statusCode:           http.StatusInternalServerError,
 			body:                 "Cannot marshall Thrift to JSON\n",
-			metrics: []mTestutils.ExpectedMetric{
+			metrics: []metricstest.ExpectedMetric{
 				{Name: "http-server.errors", Tags: map[string]string{"source": "thrift", "status": "5xx"}, Value: 1},
 			},
 		},
@@ -188,7 +186,7 @@ func TestHTTPHandlerErrors(t *testing.T) {
 				}
 
 				if len(testCase.metrics) > 0 {
-					mTestutils.AssertCounterMetrics(t, ts.metricsFactory, testCase.metrics...)
+					ts.metricsFactory.AssertCounterMetrics(t, testCase.metrics...)
 				}
 			})
 		})
@@ -202,14 +200,14 @@ func TestHTTPHandlerErrors(t *testing.T) {
 			w := &mockWriter{header: make(http.Header)}
 			handler.serveSamplingHTTP(w, req, false)
 
-			mTestutils.AssertCounterMetrics(t, ts.metricsFactory,
-				mTestutils.ExpectedMetric{Name: "http-server.errors", Tags: map[string]string{"source": "write", "status": "5xx"}, Value: 1})
+			ts.metricsFactory.AssertCounterMetrics(t,
+				metricstest.ExpectedMetric{Name: "http-server.errors", Tags: map[string]string{"source": "write", "status": "5xx"}, Value: 1})
 
 			req = httptest.NewRequest("GET", "http://localhost:80/baggageRestrictions?service=X", nil)
 			handler.serveBaggageHTTP(w, req)
 
-			mTestutils.AssertCounterMetrics(t, ts.metricsFactory,
-				mTestutils.ExpectedMetric{Name: "http-server.errors", Tags: map[string]string{"source": "write", "status": "5xx"}, Value: 2})
+			ts.metricsFactory.AssertCounterMetrics(t,
+				metricstest.ExpectedMetric{Name: "http-server.errors", Tags: map[string]string{"source": "write", "status": "5xx"}, Value: 2})
 		})
 	})
 }

diff --git a/cmd/agent/app/processors/thrift_processor_test.go b/cmd/agent/app/processors/thrift_processor_test.go
@@ -23,7 +23,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/uber/jaeger-lib/metrics"
-	mTestutils "github.com/uber/jaeger-lib/metrics/testutils"
+	"github.com/uber/jaeger-lib/metrics/metricstest"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter"
@@ -75,7 +75,7 @@ func createProcessor(t *testing.T, mFactory metrics.Factory, tFactory thrift.TPr
 	return transport.Addr().String(), processor
 }
 
-func initCollectorAndReporter(t *testing.T) (*metrics.LocalFactory, *testutils.MockTCollector, reporter.Reporter) {
+func initCollectorAndReporter(t *testing.T) (*metricstest.Factory, *testutils.MockTCollector, reporter.Reporter) {
 	metricsFactory, collector := testutils.InitMockCollector(t)
 	reporter := reporter.WrapWithMetrics(tchreporter.New("jaeger-collector", collector.Channel, time.Second, nil, zap.NewNop()), metricsFactory)
 	return metricsFactory, collector, reporter
@@ -115,7 +115,7 @@ func (h failingHandler) Process(iprot, oprot thrift.TProtocol) (success bool, er
 }
 
 func TestProcessor_HandlerError(t *testing.T) {
-	metricsFactory := metrics.NewLocalFactory(0)
+	metricsFactory := metricstest.NewFactory(0)
 
 	handler := failingHandler{err: errors.New("doh")}
 
@@ -137,9 +137,9 @@ func TestProcessor_HandlerError(t *testing.T) {
 		time.Sleep(time.Millisecond)
 	}
 
-	mTestutils.AssertCounterMetrics(t, metricsFactory,
-		mTestutils.ExpectedMetric{Name: "thrift.udp.t-processor.handler-errors", Value: 1},
-		mTestutils.ExpectedMetric{Name: "thrift.udp.server.packets.processed", Value: 1},
+	metricsFactory.AssertCounterMetrics(t,
+		metricstest.ExpectedMetric{Name: "thrift.udp.t-processor.handler-errors", Value: 1},
+		metricstest.ExpectedMetric{Name: "thrift.udp.server.packets.processed", Value: 1},
 	)
 }
 
@@ -170,7 +170,7 @@ func TestJaegerProcessor(t *testing.T) {
 	}
 }
 
-func assertJaegerProcessorCorrectness(t *testing.T, collector *testutils.MockTCollector, metricsFactory *metrics.LocalFactory) {
+func assertJaegerProcessorCorrectness(t *testing.T, collector *testutils.MockTCollector, metricsFactory *metricstest.Factory) {
 	sizeF := func() int {
 		return len(collector.GetJaegerBatches())
 	}
@@ -180,7 +180,7 @@ func assertJaegerProcessorCorrectness(t *testing.T, collector *testutils.MockTCo
 	assertProcessorCorrectness(t, metricsFactory, sizeF, nameF, "jaeger")
 }
 
-func assertZipkinProcessorCorrectness(t *testing.T, collector *testutils.MockTCollector, metricsFactory *metrics.LocalFactory) {
+func assertZipkinProcessorCorrectness(t *testing.T, collector *testutils.MockTCollector, metricsFactory *metricstest.Factory) {
 	sizeF := func() int {
 		return len(collector.GetZipkinSpans())
 	}
@@ -192,7 +192,7 @@ func assertZipkinProcessorCorrectness(t *testing.T, collector *testutils.MockTCo
 
 func assertProcessorCorrectness(
 	t *testing.T,
-	metricsFactory *metrics.LocalFactory,
+	metricsFactory *metricstest.Factory,
 	sizeF func() int,
 	nameF func() string,
 	format string,
@@ -218,7 +218,7 @@ func assertProcessorCorrectness(
 	}
 
 	// agentReporter must emit metrics
-	mTestutils.AssertCounterMetrics(t, metricsFactory, []mTestutils.ExpectedMetric{
+	metricsFactory.AssertCounterMetrics(t, []metricstest.ExpectedMetric{
 		{Name: "reporter.batches.submitted", Tags: map[string]string{"format": format}, Value: 1},
 		{Name: "reporter.spans.submitted", Tags: map[string]string{"format": format}, Value: 1},
 		{Name: "thrift.udp.server.packets.processed", Value: 1},

diff --git a/cmd/agent/app/reporter/grpc/collector_proxy.go b/cmd/agent/app/reporter/grpc/collector_proxy.go
@@ -54,7 +54,7 @@ func NewCollectorProxy(o *Options, mFactory metrics.Factory, logger *zap.Logger)
 		// It does not return error if the collector is not running
 		conn, _ = grpc.Dial(o.CollectorHostPort[0], grpc.WithInsecure())
 	}
-	grpcMetrics := mFactory.Namespace("", map[string]string{"protocol": "grpc"})
+	grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}})
 	return &ProxyBuilder{
 		conn:     conn,
 		reporter: aReporter.WrapWithMetrics(NewReporter(conn, logger), grpcMetrics),

diff --git a/cmd/agent/app/reporter/grpc/collector_proxy_test.go b/cmd/agent/app/reporter/grpc/collector_proxy_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/uber/jaeger-lib/metrics"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
+	"github.com/uber/jaeger-lib/metrics/metricstest"
 
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
 	"github.com/jaegertracing/jaeger/thrift-gen/jaeger"
@@ -57,7 +58,7 @@ func TestMultipleCollectors(t *testing.T) {
 	})
 	defer s2.Stop()
 
-	mFactory := metrics.NewLocalFactory(time.Microsecond)
+	mFactory := metricstest.NewFactory(time.Microsecond)
 	proxy, err := NewCollectorProxy(&Options{CollectorHostPort: []string{addr1.String(), addr2.String()}}, mFactory, zap.NewNop())
 	require.NoError(t, err)
 	require.NotNil(t, proxy)

diff --git a/cmd/agent/app/reporter/metrics.go b/cmd/agent/app/reporter/metrics.go
@@ -55,7 +55,7 @@ func WrapWithMetrics(reporter Reporter, mFactory metrics.Factory) *MetricsReport
 	batchesMetrics := map[string]batchMetrics{}
 	for _, s := range []string{zipkinBatches, jaegerBatches} {
 		bm := batchMetrics{}
-		metrics.Init(&bm, mFactory.Namespace("reporter", map[string]string{"format": s}), nil)
+		metrics.Init(&bm, mFactory.Namespace(metrics.NSOptions{Name: "reporter", Tags: map[string]string{"format": s}}), nil)
 		batchesMetrics[s] = bm
 	}
 	return &MetricsReporter{wrapped: reporter, metrics: batchesMetrics}