From b712c9d2e24801eec3c08358544e8ad482355b4f Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Thu, 26 Nov 2020 20:05:46 +0800 Subject: [PATCH 01/27] add metrics that show agent connection collector status Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 83 +++++++++++++++++++ .../app/reporter/connect_metrics_test.go | 71 ++++++++++++++++ cmd/agent/app/reporter/grpc/builder.go | 26 +++++- cmd/agent/app/reporter/grpc/builder_test.go | 4 +- .../app/reporter/grpc/collector_proxy.go | 2 +- 5 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 cmd/agent/app/reporter/connect_metrics.go create mode 100644 cmd/agent/app/reporter/connect_metrics_test.go diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go new file mode 100644 index 00000000000..7e11f0cffd6 --- /dev/null +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -0,0 +1,83 @@ +// Copyright (c) 2020 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reporter + +import ( + "time" + + "go.uber.org/atomic" + "go.uber.org/zap" + + "github.com/uber/jaeger-lib/metrics" +) + +type connectMetrics struct { + +} + +type ConnectMetricsReporterParams struct { + Logger *zap.Logger // required + MetricsFactory metrics.Factory // required + ExpireFrequency time.Duration + ExpireTTL time.Duration +} + + +type ConnectMetricsReporter struct { + params ConnectMetricsReporterParams + connectMetrics *connectMetrics + shutdown chan struct{} + closed *atomic.Bool + +} + +func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetricsReporter { + if params.ExpireFrequency == 0 { + params.ExpireFrequency = defaultExpireFrequency + } + if params.ExpireTTL == 0 { + params.ExpireTTL = defaultExpireTTL + } + cm := new(connectMetrics) + params.MetricsFactory = params.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) + metrics.MustInit(cm, params.MetricsFactory, nil) + r := &ConnectMetricsReporter{ + params: params, + connectMetrics: cm, + shutdown: make(chan struct{}), + closed: atomic.NewBool(false), + } + return r +} + + +func (r *ConnectMetricsReporter) CollectorConnected(target string, ) { + metric := r.params.MetricsFactory.Gauge(metrics.Options{ + Name: "connected_collector_status", + Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", + Tags: map[string]string{"target": target}, + }) + metric.Update(1) + +} + +func (r *ConnectMetricsReporter)CollectorAborted(target string) { + metric := r.params.MetricsFactory.Gauge(metrics.Options{ + Name: "connected_collector_status", + Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", + Tags: map[string]string{"target": target}, + }) + metric.Update(0) +} diff --git a/cmd/agent/app/reporter/connect_metrics_test.go b/cmd/agent/app/reporter/connect_metrics_test.go new file mode 100644 index 00000000000..4c4004f33cb --- /dev/null +++ b/cmd/agent/app/reporter/connect_metrics_test.go @@ -0,0 +1,71 @@ +// Copyright (c) 2020 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reporter + +import ( + "time" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/uber/jaeger-lib/metrics/metricstest" +) + +type connectMetricsTest struct { + mb *metricstest.Factory + +} + + +func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { + testConnectMetricsWithParams(ConnectMetricsReporterParams{ + }, fn) +} + +func testConnectMetricsWithParams(params ConnectMetricsReporterParams, fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { + mb := metricstest.NewFactory(time.Hour) + params.MetricsFactory = mb + r := WrapWithConnectMetrics(params) + + tr := &connectMetricsTest{ + mb: mb, + } + + fn(tr, r) +} + +func testCollectorConnected(r *ConnectMetricsReporter) { + r.CollectorConnected("127.0.0.1:14250") +} + +func testCollectorAborted(r *ConnectMetricsReporter) { + r.CollectorAborted("127.0.0.1:14250") +} + + +func TestConnectMetrics(t *testing.T) { + + testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetricsReporter) { + getGauge := func() int64{ + _, gauges := tr.mb.Snapshot() + return gauges["connection_status.connected_collector_status|target=127.0.0.1:14250"] + } + + testCollectorAborted(r) + assert.EqualValues(t,0, getGauge()) + + testCollectorConnected(r) + assert.EqualValues(t,1, getGauge()) + }) +} \ No newline at end of file diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index d1a2a04ec04..e490b39092d 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -22,14 +22,19 @@ import ( grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" "go.uber.org/zap" + "github.com/uber/jaeger-lib/metrics" + "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" + "google.golang.org/grpc/connectivity" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/discovery" "github.com/jaegertracing/jaeger/pkg/discovery/grpcresolver" + "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" ) // ConnBuilder Struct to hold configurations @@ -51,7 +56,7 @@ func NewConnBuilder() *ConnBuilder { } // CreateConnection creates the gRPC connection -func (b *ConnBuilder) CreateConnection(logger *zap.Logger) (*grpc.ClientConn, error) { +func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Factory) (*grpc.ClientConn, error) { var dialOptions []grpc.DialOption var dialTarget string if b.TLS.Enabled { // user requested a secure connection @@ -97,14 +102,29 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger) (*grpc.ClientConn, er return nil, err } - go func(cc *grpc.ClientConn) { + grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) + + cm := reporter.WrapWithConnectMetrics(reporter.ConnectMetricsReporterParams{ + Logger: logger, + MetricsFactory: grpcMetrics, + }) + + + go func(cc *grpc.ClientConn, cm *reporter.ConnectMetricsReporter) { logger.Info("Checking connection to collector") for { s := cc.GetState() + if(s != connectivity.Ready){ + cm.CollectorAborted(cc.Target()) + }else{ + cm.CollectorConnected(cc.Target()) + } + logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s)) cc.WaitForStateChange(context.Background(), s) } - }(conn) + }(conn, cm) + return conn, nil } diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index 31949754a2a..4df5c395ba4 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -59,7 +59,7 @@ func TestBuilderFromConfig(t *testing.T) { t, []string{"127.0.0.1:14268", "127.0.0.1:14269"}, cfg.CollectorHostPorts) - r, err := cfg.CreateConnection(zap.NewNop()) + r, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) require.NoError(t, err) assert.NotNil(t, r) } @@ -149,7 +149,7 @@ func TestBuilderWithCollectors(t *testing.T) { cfg.Notifier = test.notifier cfg.Discoverer = test.discoverer - conn, err := cfg.CreateConnection(zap.NewNop()) + conn, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) if test.expectedError == "" { require.NoError(t, err) require.NotNil(t, conn) diff --git a/cmd/agent/app/reporter/grpc/collector_proxy.go b/cmd/agent/app/reporter/grpc/collector_proxy.go index 6c5b19f4e65..2d9dc94783f 100644 --- a/cmd/agent/app/reporter/grpc/collector_proxy.go +++ b/cmd/agent/app/reporter/grpc/collector_proxy.go @@ -37,7 +37,7 @@ type ProxyBuilder struct { // NewCollectorProxy creates ProxyBuilder func NewCollectorProxy(builder *ConnBuilder, agentTags map[string]string, mFactory metrics.Factory, logger *zap.Logger) (*ProxyBuilder, error) { - conn, err := builder.CreateConnection(logger) + conn, err := builder.CreateConnection(logger, mFactory) if err != nil { return nil, err } From 109617cc9a8c41cf7e484e93f01a44d3e6b3826b Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Thu, 26 Nov 2020 21:41:19 +0800 Subject: [PATCH 02/27] update comment Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 7e11f0cffd6..eea5b58c4cb 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -23,10 +23,12 @@ import ( "github.com/uber/jaeger-lib/metrics" ) +// Structure built for future code expansion or add new metrics. type connectMetrics struct { } +// ConnectMetricsReporterParams is used as input to WrapWithConnectMetrics. type ConnectMetricsReporterParams struct { Logger *zap.Logger // required MetricsFactory metrics.Factory // required @@ -34,7 +36,8 @@ type ConnectMetricsReporterParams struct { ExpireTTL time.Duration } - +// ConnectMetricsReporter is a decorator also it not actual use currently. +// Structure built for future code expansion type ConnectMetricsReporter struct { params ConnectMetricsReporterParams connectMetrics *connectMetrics @@ -42,7 +45,7 @@ type ConnectMetricsReporter struct { closed *atomic.Bool } - +// WrapWithConnectMetrics creates ConnectMetricsReporter. func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetricsReporter { if params.ExpireFrequency == 0 { params.ExpireFrequency = defaultExpireFrequency @@ -62,7 +65,7 @@ func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetrics return r } - +// CollectorConnected used for change metric as agent connected. func (r *ConnectMetricsReporter) CollectorConnected(target string, ) { metric := r.params.MetricsFactory.Gauge(metrics.Options{ Name: "connected_collector_status", @@ -73,6 +76,7 @@ func (r *ConnectMetricsReporter) CollectorConnected(target string, ) { } +// CollectorAborted used for change metric as agent disconnected. func (r *ConnectMetricsReporter)CollectorAborted(target string) { metric := r.params.MetricsFactory.Gauge(metrics.Options{ Name: "connected_collector_status", From eee31fdb72a480ef4776d023ba58eb2fa83415c2 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Fri, 27 Nov 2020 00:17:28 +0800 Subject: [PATCH 03/27] exec make fmt Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 22 ++++++++--------- .../app/reporter/connect_metrics_test.go | 24 ++++++++----------- cmd/agent/app/reporter/grpc/builder.go | 14 ++++------- 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index eea5b58c4cb..a76b5879ab5 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -17,15 +17,13 @@ package reporter import ( "time" + "github.com/uber/jaeger-lib/metrics" "go.uber.org/atomic" "go.uber.org/zap" - - "github.com/uber/jaeger-lib/metrics" ) // Structure built for future code expansion or add new metrics. type connectMetrics struct { - } // ConnectMetricsReporterParams is used as input to WrapWithConnectMetrics. @@ -39,12 +37,12 @@ type ConnectMetricsReporterParams struct { // ConnectMetricsReporter is a decorator also it not actual use currently. // Structure built for future code expansion type ConnectMetricsReporter struct { - params ConnectMetricsReporterParams + params ConnectMetricsReporterParams connectMetrics *connectMetrics - shutdown chan struct{} - closed *atomic.Bool - + shutdown chan struct{} + closed *atomic.Bool } + // WrapWithConnectMetrics creates ConnectMetricsReporter. func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetricsReporter { if params.ExpireFrequency == 0 { @@ -57,16 +55,16 @@ func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetrics params.MetricsFactory = params.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) metrics.MustInit(cm, params.MetricsFactory, nil) r := &ConnectMetricsReporter{ - params: params, + params: params, connectMetrics: cm, - shutdown: make(chan struct{}), - closed: atomic.NewBool(false), + shutdown: make(chan struct{}), + closed: atomic.NewBool(false), } return r } // CollectorConnected used for change metric as agent connected. -func (r *ConnectMetricsReporter) CollectorConnected(target string, ) { +func (r *ConnectMetricsReporter) CollectorConnected(target string) { metric := r.params.MetricsFactory.Gauge(metrics.Options{ Name: "connected_collector_status", Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", @@ -77,7 +75,7 @@ func (r *ConnectMetricsReporter) CollectorConnected(target string, ) { } // CollectorAborted used for change metric as agent disconnected. -func (r *ConnectMetricsReporter)CollectorAborted(target string) { +func (r *ConnectMetricsReporter) CollectorAborted(target string) { metric := r.params.MetricsFactory.Gauge(metrics.Options{ Name: "connected_collector_status", Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", diff --git a/cmd/agent/app/reporter/connect_metrics_test.go b/cmd/agent/app/reporter/connect_metrics_test.go index 4c4004f33cb..af7fde76a8e 100644 --- a/cmd/agent/app/reporter/connect_metrics_test.go +++ b/cmd/agent/app/reporter/connect_metrics_test.go @@ -15,22 +15,19 @@ package reporter import ( - "time" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/uber/jaeger-lib/metrics/metricstest" ) type connectMetricsTest struct { - mb *metricstest.Factory - + mb *metricstest.Factory } - func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { - testConnectMetricsWithParams(ConnectMetricsReporterParams{ - }, fn) + testConnectMetricsWithParams(ConnectMetricsReporterParams{}, fn) } func testConnectMetricsWithParams(params ConnectMetricsReporterParams, fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { @@ -39,33 +36,32 @@ func testConnectMetricsWithParams(params ConnectMetricsReporterParams, fn func(t r := WrapWithConnectMetrics(params) tr := &connectMetricsTest{ - mb: mb, + mb: mb, } fn(tr, r) } -func testCollectorConnected(r *ConnectMetricsReporter) { +func testCollectorConnected(r *ConnectMetricsReporter) { r.CollectorConnected("127.0.0.1:14250") } -func testCollectorAborted(r *ConnectMetricsReporter) { +func testCollectorAborted(r *ConnectMetricsReporter) { r.CollectorAborted("127.0.0.1:14250") } - func TestConnectMetrics(t *testing.T) { testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetricsReporter) { - getGauge := func() int64{ + getGauge := func() int64 { _, gauges := tr.mb.Snapshot() return gauges["connection_status.connected_collector_status|target=127.0.0.1:14250"] } testCollectorAborted(r) - assert.EqualValues(t,0, getGauge()) + assert.EqualValues(t, 0, getGauge()) testCollectorConnected(r) - assert.EqualValues(t,1, getGauge()) + assert.EqualValues(t, 1, getGauge()) }) -} \ No newline at end of file +} diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index e490b39092d..b13b6e07b2a 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -21,20 +21,18 @@ import ( "strings" grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" - "go.uber.org/zap" "github.com/uber/jaeger-lib/metrics" - + "go.uber.org/zap" "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" - "google.golang.org/grpc/connectivity" - + "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/discovery" "github.com/jaegertracing/jaeger/pkg/discovery/grpcresolver" - "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" ) // ConnBuilder Struct to hold configurations @@ -109,14 +107,13 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact MetricsFactory: grpcMetrics, }) - go func(cc *grpc.ClientConn, cm *reporter.ConnectMetricsReporter) { logger.Info("Checking connection to collector") for { s := cc.GetState() - if(s != connectivity.Ready){ + if s != connectivity.Ready { cm.CollectorAborted(cc.Target()) - }else{ + } else { cm.CollectorConnected(cc.Target()) } @@ -125,6 +122,5 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact } }(conn, cm) - return conn, nil } From 82ed46eac0a14f7502ccdc0f1f30aeaa708f64ed Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Sat, 28 Nov 2020 19:24:49 +0800 Subject: [PATCH 04/27] simplify function and add testing relevant code in the builder_test.go Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 57 +++++++++---------- .../app/reporter/connect_metrics_test.go | 40 ++++++++----- cmd/agent/app/reporter/grpc/builder.go | 26 +++++---- cmd/agent/app/reporter/grpc/builder_test.go | 28 ++++++++- 4 files changed, 97 insertions(+), 54 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index a76b5879ab5..703a1387421 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -18,68 +18,65 @@ import ( "time" "github.com/uber/jaeger-lib/metrics" - "go.uber.org/atomic" "go.uber.org/zap" ) -// Structure built for future code expansion or add new metrics. +// connectMetrics is real metric, but not support to directly change, need via ConnectMetrics for changed type connectMetrics struct { + // used for reflect current connection stability + ConnectedCollectorReconnect metrics.Counter `metric:"connected_collector_reconnect" help:"Default is 1, the metric can reflect current connection stability, as reconnect action increase the metric increase."` } -// ConnectMetricsReporterParams is used as input to WrapWithConnectMetrics. -type ConnectMetricsReporterParams struct { +// ConnectMetricsParams include connectMetrics necessary params +// Although some are not used, it use in the future +type ConnectMetricsParams struct { Logger *zap.Logger // required MetricsFactory metrics.Factory // required ExpireFrequency time.Duration ExpireTTL time.Duration + + // connection target + Target string } -// ConnectMetricsReporter is a decorator also it not actual use currently. -// Structure built for future code expansion -type ConnectMetricsReporter struct { - params ConnectMetricsReporterParams +// ConnectMetrics include ConnectMetricsParams and connectMetrics, likes connectMetrics API +// If want to modify metrics of connectMetrics, must via ConnectMetrics API +type ConnectMetrics struct { + params ConnectMetricsParams connectMetrics *connectMetrics - shutdown chan struct{} - closed *atomic.Bool } -// WrapWithConnectMetrics creates ConnectMetricsReporter. -func WrapWithConnectMetrics(params ConnectMetricsReporterParams) *ConnectMetricsReporter { +// WrapWithConnectMetrics creates ConnectMetrics. +func WrapWithConnectMetrics(params ConnectMetricsParams, target string) *ConnectMetrics { if params.ExpireFrequency == 0 { params.ExpireFrequency = defaultExpireFrequency } if params.ExpireTTL == 0 { params.ExpireTTL = defaultExpireTTL } + params.Target = target cm := new(connectMetrics) + params.MetricsFactory = params.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) metrics.MustInit(cm, params.MetricsFactory, nil) - r := &ConnectMetricsReporter{ + r := &ConnectMetrics{ params: params, connectMetrics: cm, - shutdown: make(chan struct{}), - closed: atomic.NewBool(false), } return r } -// CollectorConnected used for change metric as agent connected. -func (r *ConnectMetricsReporter) CollectorConnected(target string) { - metric := r.params.MetricsFactory.Gauge(metrics.Options{ - Name: "connected_collector_status", - Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", - Tags: map[string]string{"target": target}, - }) - metric.Update(1) - -} - -// CollectorAborted used for change metric as agent disconnected. -func (r *ConnectMetricsReporter) CollectorAborted(target string) { +// When connection is changed, pass the status parameter +// 0 is disconnected, 1 is connected +// For quick view status via use `sum(jaeger_agent_connection_status_connected_collector_status{}) by (instance) > bool 0` +func (r *ConnectMetrics) OnConnectionStatusChange(status int64) { metric := r.params.MetricsFactory.Gauge(metrics.Options{ Name: "connected_collector_status", Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", - Tags: map[string]string{"target": target}, + Tags: map[string]string{"target": r.params.Target}, }) - metric.Update(0) + metric.Update(status) + if status == 1 { + r.connectMetrics.ConnectedCollectorReconnect.Inc(1) + } } diff --git a/cmd/agent/app/reporter/connect_metrics_test.go b/cmd/agent/app/reporter/connect_metrics_test.go index af7fde76a8e..fb923f37ab1 100644 --- a/cmd/agent/app/reporter/connect_metrics_test.go +++ b/cmd/agent/app/reporter/connect_metrics_test.go @@ -26,14 +26,14 @@ type connectMetricsTest struct { mb *metricstest.Factory } -func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { - testConnectMetricsWithParams(ConnectMetricsReporterParams{}, fn) +func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetrics)) { + testConnectMetricsWithParams(ConnectMetricsParams{}, fn) } -func testConnectMetricsWithParams(params ConnectMetricsReporterParams, fn func(tr *connectMetricsTest, r *ConnectMetricsReporter)) { +func testConnectMetricsWithParams(params ConnectMetricsParams, fn func(tr *connectMetricsTest, r *ConnectMetrics)) { mb := metricstest.NewFactory(time.Hour) params.MetricsFactory = mb - r := WrapWithConnectMetrics(params) + r := WrapWithConnectMetrics(params, "127.0.0.1:14250") tr := &connectMetricsTest{ mb: mb, @@ -42,26 +42,40 @@ func testConnectMetricsWithParams(params ConnectMetricsReporterParams, fn func(t fn(tr, r) } -func testCollectorConnected(r *ConnectMetricsReporter) { - r.CollectorConnected("127.0.0.1:14250") +func testCollectorConnected(r *ConnectMetrics) { + r.OnConnectionStatusChange(1) } -func testCollectorAborted(r *ConnectMetricsReporter) { - r.CollectorAborted("127.0.0.1:14250") +func testCollectorAborted(r *ConnectMetrics) { + r.OnConnectionStatusChange(0) } func TestConnectMetrics(t *testing.T) { - testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetricsReporter) { - getGauge := func() int64 { + testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetrics) { + getGauge := func() map[string]int64 { _, gauges := tr.mb.Snapshot() - return gauges["connection_status.connected_collector_status|target=127.0.0.1:14250"] + return gauges } + getCount := func() map[string]int64 { + counts, _ := tr.mb.Snapshot() + return counts + } + + // testing connect aborted testCollectorAborted(r) - assert.EqualValues(t, 0, getGauge()) + assert.EqualValues(t, 0, getGauge()["connection_status.connected_collector_status|target=127.0.0.1:14250"]) + + // testing connect connected + testCollectorConnected(r) + assert.EqualValues(t, 1, getGauge()["connection_status.connected_collector_status|target=127.0.0.1:14250"]) + assert.EqualValues(t, 1, getCount()["connection_status.connected_collector_reconnect"]) + // testing reconnect counts + testCollectorAborted(r) testCollectorConnected(r) - assert.EqualValues(t, 1, getGauge()) + assert.EqualValues(t, 2, getCount()["connection_status.connected_collector_reconnect"]) + }) } diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index b13b6e07b2a..14fcae35ef4 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -46,6 +46,8 @@ type ConnBuilder struct { DiscoveryMinPeers int Notifier discovery.Notifier Discoverer discovery.Discoverer + + ConnectMetrics *reporter.ConnectMetrics } // NewConnBuilder creates a new grpc connection builder. @@ -100,27 +102,31 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact return nil, err } - grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) + if b.ConnectMetrics == nil { + grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) - cm := reporter.WrapWithConnectMetrics(reporter.ConnectMetricsReporterParams{ - Logger: logger, - MetricsFactory: grpcMetrics, - }) + // for unit test and provide ConnectMetrics and outside call + cm := reporter.WrapWithConnectMetrics(reporter.ConnectMetricsParams{ + Logger: logger, + MetricsFactory: grpcMetrics, + }, dialTarget) + b.ConnectMetrics = cm + } - go func(cc *grpc.ClientConn, cm *reporter.ConnectMetricsReporter) { + go func(cc *grpc.ClientConn, cm *reporter.ConnectMetrics) { logger.Info("Checking connection to collector") for { s := cc.GetState() - if s != connectivity.Ready { - cm.CollectorAborted(cc.Target()) + if s == connectivity.Ready { + cm.OnConnectionStatusChange(1) } else { - cm.CollectorConnected(cc.Target()) + cm.OnConnectionStatusChange(0) } logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s)) cc.WaitForStateChange(context.Background(), s) } - }(conn, cm) + }(conn, b.ConnectMetrics) return conn, nil } diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index 4df5c395ba4..3545ba8a581 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -15,6 +15,7 @@ package grpc import ( "context" + "fmt" "net" "strings" "testing" @@ -28,8 +29,9 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" - yaml "gopkg.in/yaml.v2" + "gopkg.in/yaml.v2" + "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/discovery" "github.com/jaegertracing/jaeger/proto-gen/api_v2" @@ -46,6 +48,10 @@ var testCertKeyLocation = "../../../../../pkg/config/tlscfg/testdata/" type noopNotifier struct{} +type connectMetricsTest struct { + mb *metricstest.Factory +} + func (noopNotifier) Register(chan<- []string) {} func (noopNotifier) Unregister(chan<- []string) {} @@ -149,6 +155,16 @@ func TestBuilderWithCollectors(t *testing.T) { cfg.Notifier = test.notifier cfg.Discoverer = test.discoverer + mb := metricstest.NewFactory(time.Hour) + params := reporter.ConnectMetricsParams{ + MetricsFactory: mb, + } + cm := reporter.WrapWithConnectMetrics(params, test.target) + tr := &connectMetricsTest{ + mb: mb, + } + cfg.ConnectMetrics = cm + conn, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) if test.expectedError == "" { require.NoError(t, err) @@ -161,10 +177,20 @@ func TestBuilderWithCollectors(t *testing.T) { } else { assert.True(t, conn.Target() == test.target) } + if test.expectedState == "READY" { + counts, gauges := tr.mb.Snapshot() + assert.EqualValues(t, 1, gauges[fmt.Sprintf("connection_status.connected_collector_status|target=%s", test.target)]) + assert.EqualValues(t, 1, counts["connection_status.connected_collector_reconnect"]) + } + if test.expectedState == "TRANSIENT_FAILURE" { + _, gauges := tr.mb.Snapshot() + assert.EqualValues(t, 0, gauges[fmt.Sprintf("connection_status.connected_collector_status|target=%s", test.target)]) + } } else { require.Error(t, err) assert.Contains(t, err.Error(), test.expectedError) } + }) } } From ff406aa85d660b604996f6d74dccd9536c337ca3 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Sat, 28 Nov 2020 19:35:15 +0800 Subject: [PATCH 05/27] add comment in connect_metrics.go Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 703a1387421..4ec51f722dd 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -66,7 +66,7 @@ func WrapWithConnectMetrics(params ConnectMetricsParams, target string) *Connect return r } -// When connection is changed, pass the status parameter +// OnConnectionStatusChange used for pass the status parameter when connection is changed // 0 is disconnected, 1 is connected // For quick view status via use `sum(jaeger_agent_connection_status_connected_collector_status{}) by (instance) > bool 0` func (r *ConnectMetrics) OnConnectionStatusChange(status int64) { From c2a64f4f12746635c4fe8da000ad95e953f77af2 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 14:59:14 +0800 Subject: [PATCH 06/27] simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 56 ++++++++----------- .../app/reporter/connect_metrics_test.go | 32 +++++------ cmd/agent/app/reporter/grpc/builder.go | 34 +++++++---- cmd/agent/app/reporter/grpc/builder_test.go | 23 ++++---- 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 4ec51f722dd..ead14d1e49d 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -25,58 +25,48 @@ import ( type connectMetrics struct { // used for reflect current connection stability ConnectedCollectorReconnect metrics.Counter `metric:"connected_collector_reconnect" help:"Default is 1, the metric can reflect current connection stability, as reconnect action increase the metric increase."` + + // Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected + ConnectedCollectorStatus metrics.Gauge `metric:"connected_collector_status" help:"Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected"` } -// ConnectMetricsParams include connectMetrics necessary params -// Although some are not used, it use in the future +// ConnectMetricsParams include connectMetrics necessary params and connectMetrics, likes connectMetrics API +// If want to modify metrics of connectMetrics, must via ConnectMetrics API type ConnectMetricsParams struct { Logger *zap.Logger // required MetricsFactory metrics.Factory // required ExpireFrequency time.Duration ExpireTTL time.Duration + connectMetrics *connectMetrics - // connection target - Target string } -// ConnectMetrics include ConnectMetricsParams and connectMetrics, likes connectMetrics API -// If want to modify metrics of connectMetrics, must via ConnectMetrics API -type ConnectMetrics struct { - params ConnectMetricsParams - connectMetrics *connectMetrics -} -// WrapWithConnectMetrics creates ConnectMetrics. -func WrapWithConnectMetrics(params ConnectMetricsParams, target string) *ConnectMetrics { - if params.ExpireFrequency == 0 { - params.ExpireFrequency = defaultExpireFrequency +// NewConnectMetrics creates ConnectMetricsParams. +func NewConnectMetrics(cmp ConnectMetricsParams) *ConnectMetricsParams { + if cmp.ExpireFrequency == 0 { + cmp.ExpireFrequency = defaultExpireFrequency } - if params.ExpireTTL == 0 { - params.ExpireTTL = defaultExpireTTL + if cmp.ExpireTTL == 0 { + cmp.ExpireTTL = defaultExpireTTL } - params.Target = target + cm := new(connectMetrics) + cmp.MetricsFactory = cmp.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) + metrics.MustInit(cm, cmp.MetricsFactory, nil) + cmp.connectMetrics = cm - params.MetricsFactory = params.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) - metrics.MustInit(cm, params.MetricsFactory, nil) - r := &ConnectMetrics{ - params: params, - connectMetrics: cm, - } - return r + return &cmp } // OnConnectionStatusChange used for pass the status parameter when connection is changed // 0 is disconnected, 1 is connected // For quick view status via use `sum(jaeger_agent_connection_status_connected_collector_status{}) by (instance) > bool 0` -func (r *ConnectMetrics) OnConnectionStatusChange(status int64) { - metric := r.params.MetricsFactory.Gauge(metrics.Options{ - Name: "connected_collector_status", - Help: "Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected", - Tags: map[string]string{"target": r.params.Target}, - }) - metric.Update(status) - if status == 1 { +func (r *ConnectMetricsParams) OnConnectionStatusChange(connected bool) { + if connected { + r.connectMetrics.ConnectedCollectorStatus.Update(1) r.connectMetrics.ConnectedCollectorReconnect.Inc(1) + }else { + r.connectMetrics.ConnectedCollectorStatus.Update(0) } -} +} \ No newline at end of file diff --git a/cmd/agent/app/reporter/connect_metrics_test.go b/cmd/agent/app/reporter/connect_metrics_test.go index fb923f37ab1..be3111b271b 100644 --- a/cmd/agent/app/reporter/connect_metrics_test.go +++ b/cmd/agent/app/reporter/connect_metrics_test.go @@ -23,53 +23,53 @@ import ( ) type connectMetricsTest struct { - mb *metricstest.Factory + mf *metricstest.Factory } -func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetrics)) { +func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetricsParams)) { testConnectMetricsWithParams(ConnectMetricsParams{}, fn) } -func testConnectMetricsWithParams(params ConnectMetricsParams, fn func(tr *connectMetricsTest, r *ConnectMetrics)) { - mb := metricstest.NewFactory(time.Hour) - params.MetricsFactory = mb - r := WrapWithConnectMetrics(params, "127.0.0.1:14250") +func testConnectMetricsWithParams(params ConnectMetricsParams, fn func(tr *connectMetricsTest, r *ConnectMetricsParams)) { + mf := metricstest.NewFactory(time.Hour) + params.MetricsFactory = mf + r := NewConnectMetrics(params) tr := &connectMetricsTest{ - mb: mb, + mf: mf, } fn(tr, r) } -func testCollectorConnected(r *ConnectMetrics) { - r.OnConnectionStatusChange(1) +func testCollectorConnected(r *ConnectMetricsParams) { + r.OnConnectionStatusChange(true) } -func testCollectorAborted(r *ConnectMetrics) { - r.OnConnectionStatusChange(0) +func testCollectorAborted(r *ConnectMetricsParams) { + r.OnConnectionStatusChange(false) } func TestConnectMetrics(t *testing.T) { - testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetrics) { + testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetricsParams) { getGauge := func() map[string]int64 { - _, gauges := tr.mb.Snapshot() + _, gauges := tr.mf.Snapshot() return gauges } getCount := func() map[string]int64 { - counts, _ := tr.mb.Snapshot() + counts, _ := tr.mf.Snapshot() return counts } // testing connect aborted testCollectorAborted(r) - assert.EqualValues(t, 0, getGauge()["connection_status.connected_collector_status|target=127.0.0.1:14250"]) + assert.EqualValues(t, 0, getGauge()["connection_status.connected_collector_status"]) // testing connect connected testCollectorConnected(r) - assert.EqualValues(t, 1, getGauge()["connection_status.connected_collector_status|target=127.0.0.1:14250"]) + assert.EqualValues(t, 1, getGauge()["connection_status.connected_collector_status"]) assert.EqualValues(t, 1, getCount()["connection_status.connected_collector_reconnect"]) // testing reconnect counts diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index 14fcae35ef4..8f326f9a8b2 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -17,10 +17,9 @@ package grpc import ( "context" "errors" + "expvar" "fmt" - "strings" - - grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" + "github.com/grpc-ecosystem/go-grpc-middleware/retry" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" "google.golang.org/grpc" @@ -28,6 +27,7 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" + "strings" "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" @@ -47,7 +47,7 @@ type ConnBuilder struct { Notifier discovery.Notifier Discoverer discovery.Discoverer - ConnectMetrics *reporter.ConnectMetrics + ConnectMetricsParams *reporter.ConnectMetricsParams } // NewConnBuilder creates a new grpc connection builder. @@ -102,31 +102,41 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact return nil, err } - if b.ConnectMetrics == nil { + if b.ConnectMetricsParams == nil { grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) // for unit test and provide ConnectMetrics and outside call - cm := reporter.WrapWithConnectMetrics(reporter.ConnectMetricsParams{ + cmp := reporter.NewConnectMetrics(reporter.ConnectMetricsParams{ Logger: logger, MetricsFactory: grpcMetrics, - }, dialTarget) - b.ConnectMetrics = cm + }) + b.ConnectMetricsParams = cmp } - go func(cc *grpc.ClientConn, cm *reporter.ConnectMetrics) { + go func(cc *grpc.ClientConn, cm *reporter.ConnectMetricsParams) { logger.Info("Checking connection to collector") + + var egt *expvar.String + r := expvar.Get("gRPCTarget") + if r == nil{ + egt = expvar.NewString("gRPCTarget") + }else { + egt = r.(*expvar.String) + } + for { s := cc.GetState() if s == connectivity.Ready { - cm.OnConnectionStatusChange(1) + cm.OnConnectionStatusChange(true) + egt.Set(cc.Target()) } else { - cm.OnConnectionStatusChange(0) + cm.OnConnectionStatusChange(false) } logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s)) cc.WaitForStateChange(context.Background(), s) } - }(conn, b.ConnectMetrics) + }(conn, b.ConnectMetricsParams) return conn, nil } diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index 3545ba8a581..5a04ddfdfe1 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -15,7 +15,7 @@ package grpc import ( "context" - "fmt" + "expvar" "net" "strings" "testing" @@ -49,7 +49,7 @@ var testCertKeyLocation = "../../../../../pkg/config/tlscfg/testdata/" type noopNotifier struct{} type connectMetricsTest struct { - mb *metricstest.Factory + mf *metricstest.Factory } func (noopNotifier) Register(chan<- []string) {} @@ -155,15 +155,15 @@ func TestBuilderWithCollectors(t *testing.T) { cfg.Notifier = test.notifier cfg.Discoverer = test.discoverer - mb := metricstest.NewFactory(time.Hour) + mf := metricstest.NewFactory(time.Hour) params := reporter.ConnectMetricsParams{ - MetricsFactory: mb, + MetricsFactory: mf, } - cm := reporter.WrapWithConnectMetrics(params, test.target) + cm := reporter.NewConnectMetrics(params) tr := &connectMetricsTest{ - mb: mb, + mf: mf, } - cfg.ConnectMetrics = cm + cfg.ConnectMetricsParams = cm conn, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) if test.expectedError == "" { @@ -178,13 +178,14 @@ func TestBuilderWithCollectors(t *testing.T) { assert.True(t, conn.Target() == test.target) } if test.expectedState == "READY" { - counts, gauges := tr.mb.Snapshot() - assert.EqualValues(t, 1, gauges[fmt.Sprintf("connection_status.connected_collector_status|target=%s", test.target)]) + counts, gauges := tr.mf.Snapshot() + assert.EqualValues(t, 1, gauges["connection_status.connected_collector_status"]) assert.EqualValues(t, 1, counts["connection_status.connected_collector_reconnect"]) + assert.Equal(t, test.target, expvar.Get("gRPCTarget").(*expvar.String).Value()) } if test.expectedState == "TRANSIENT_FAILURE" { - _, gauges := tr.mb.Snapshot() - assert.EqualValues(t, 0, gauges[fmt.Sprintf("connection_status.connected_collector_status|target=%s", test.target)]) + _, gauges := tr.mf.Snapshot() + assert.EqualValues(t, 0, gauges["connection_status.connected_collector_status"]) } } else { require.Error(t, err) From 1429e7666e998083b3c77bc032c0bd818e7b78e8 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 15:00:48 +0800 Subject: [PATCH 07/27] simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/grpc/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index 8f326f9a8b2..330367d6d5f 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -105,7 +105,7 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact if b.ConnectMetricsParams == nil { grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) - // for unit test and provide ConnectMetrics and outside call + // for unit test and provide ConnectMetricsParams and outside call cmp := reporter.NewConnectMetrics(reporter.ConnectMetricsParams{ Logger: logger, MetricsFactory: grpcMetrics, From dd68627c901b728c2092c10dbe1ef1332bacbe51 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 15:08:56 +0800 Subject: [PATCH 08/27] exec make fmt Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 8 +++----- cmd/agent/app/reporter/grpc/builder.go | 7 ++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index ead14d1e49d..d019e716228 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -37,11 +37,9 @@ type ConnectMetricsParams struct { MetricsFactory metrics.Factory // required ExpireFrequency time.Duration ExpireTTL time.Duration - connectMetrics *connectMetrics - + connectMetrics *connectMetrics } - // NewConnectMetrics creates ConnectMetricsParams. func NewConnectMetrics(cmp ConnectMetricsParams) *ConnectMetricsParams { if cmp.ExpireFrequency == 0 { @@ -66,7 +64,7 @@ func (r *ConnectMetricsParams) OnConnectionStatusChange(connected bool) { if connected { r.connectMetrics.ConnectedCollectorStatus.Update(1) r.connectMetrics.ConnectedCollectorReconnect.Inc(1) - }else { + } else { r.connectMetrics.ConnectedCollectorStatus.Update(0) } -} \ No newline at end of file +} diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index 330367d6d5f..0556f545be5 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -19,6 +19,8 @@ import ( "errors" "expvar" "fmt" + "strings" + "github.com/grpc-ecosystem/go-grpc-middleware/retry" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" @@ -27,7 +29,6 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" - "strings" "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" @@ -118,9 +119,9 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact var egt *expvar.String r := expvar.Get("gRPCTarget") - if r == nil{ + if r == nil { egt = expvar.NewString("gRPCTarget") - }else { + } else { egt = r.(*expvar.String) } From 83452e4b08411c4b6ed0a30acaaff38407038016 Mon Sep 17 00:00:00 2001 From: Betula-L Date: Fri, 27 Nov 2020 02:51:29 +0800 Subject: [PATCH 09/27] Fix collector panic due to sarama sdk returning nil error (#2654) Signed-off-by: luhualin Co-authored-by: luhualin Signed-off-by: WalkerWang731 --- plugin/storage/kafka/writer.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/storage/kafka/writer.go b/plugin/storage/kafka/writer.go index e82ecea735c..03e23886fd8 100644 --- a/plugin/storage/kafka/writer.go +++ b/plugin/storage/kafka/writer.go @@ -57,7 +57,9 @@ func NewSpanWriter( }() go func() { for e := range producer.Errors() { - logger.Error(e.Err.Error()) + if e != nil && e.Err != nil { + logger.Error(e.Err.Error()) + } writeMetrics.SpansWrittenFailure.Inc(1) } }() From 68c0b2959c9b8e6ed7d04fa98dc6dc6f5003a811 Mon Sep 17 00:00:00 2001 From: Pavel Kositsyn Date: Fri, 27 Nov 2020 03:55:22 +0300 Subject: [PATCH 10/27] Fix flaky tbuffered server test (#2635) * Fix flaky tbuffered server test Signed-off-by: Pavel Kositsyn * Apply suggestions from code review - more readable comments Co-authored-by: Yuri Shkuro Signed-off-by: Pavel Kositsyn Co-authored-by: Yuri Shkuro Signed-off-by: WalkerWang731 --- .../app/servers/tbuffered_server_test.go | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/cmd/agent/app/servers/tbuffered_server_test.go b/cmd/agent/app/servers/tbuffered_server_test.go index 69f2a447182..e77cc164cf4 100644 --- a/cmd/agent/app/servers/tbuffered_server_test.go +++ b/cmd/agent/app/servers/tbuffered_server_test.go @@ -96,13 +96,19 @@ type fakeTransport struct { wg sync.WaitGroup } +// Read simulates three packets received, then blocks until semaphore is released at the end of the test. +// First packet is returned as normal. +// Second packet is simulated as error. +// Third packet is returned as normal, but will be dropped as overflow by the server whose queue size = 1. func (t *fakeTransport) Read(p []byte) (n int, err error) { packet := t.packet.Inc() - if packet > 2 { - if packet > 3 { - // return error once when packet==3, otherwise block - t.wg.Wait() - } + if packet == 2 { + // return some error packet, followed by valid one + return 0, io.ErrNoProgress + } + if packet > 3 { + // block after 3 packets until the server is shutdown and semaphore released + t.wg.Wait() return 0, io.EOF } for i := range p { @@ -128,9 +134,9 @@ func TestTBufferedServer_Metrics(t *testing.T) { go server.Serve() defer server.Stop() - // The fakeTransport will allow the server to read exactly two packets and one error. + // The fakeTransport will allow the server to read exactly two packets and one error in between. // Since we use the server with queue size == 1, the first packet will be - // sent to channel, and the second one dropped. + // sent to channel, the error will increment the metric, and the second valid packet dropped. packetDropped := false for i := 0; i < 5000; i++ { From f651162f0b63daf8fdac0a9e9b39b4d9747f9887 Mon Sep 17 00:00:00 2001 From: Ashmita Date: Sat, 28 Nov 2020 01:41:18 +0800 Subject: [PATCH 11/27] Add github actions for integration tests (#2649) * Add github action for jaeger integration tests Signed-off-by: Ashmita Bohara * Create separate workflow for each integration test Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- .../workflows/cassandra-integration-tests.yml | 20 +++++++++++++++++++ .github/workflows/es-integration-tests.yml | 20 +++++++++++++++++++ .github/workflows/kafka-integration-tests.yml | 20 +++++++++++++++++++ .github/workflows/unit-tests.yml | 6 ++---- Makefile | 4 ++-- 5 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/cassandra-integration-tests.yml create mode 100644 .github/workflows/es-integration-tests.yml create mode 100644 .github/workflows/kafka-integration-tests.yml diff --git a/.github/workflows/cassandra-integration-tests.yml b/.github/workflows/cassandra-integration-tests.yml new file mode 100644 index 00000000000..e0748db372e --- /dev/null +++ b/.github/workflows/cassandra-integration-tests.yml @@ -0,0 +1,20 @@ +name: Integration Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + cassandra: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - name: Run cassandra integration tests + run: bash scripts/travis/cassandra-integration-test.sh diff --git a/.github/workflows/es-integration-tests.yml b/.github/workflows/es-integration-tests.yml new file mode 100644 index 00000000000..d36b0704947 --- /dev/null +++ b/.github/workflows/es-integration-tests.yml @@ -0,0 +1,20 @@ +name: Integration Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + elasticsearch: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - name: Run elasticsearch integration tests + run: bash scripts/travis/es-integration-test.sh diff --git a/.github/workflows/kafka-integration-tests.yml b/.github/workflows/kafka-integration-tests.yml new file mode 100644 index 00000000000..3853f6ebafd --- /dev/null +++ b/.github/workflows/kafka-integration-tests.yml @@ -0,0 +1,20 @@ +name: Integration Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + kafka: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - name: Run kafka integration tests + run: bash scripts/travis/kafka-integration-test.sh diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index f0cb7159724..c74442a4869 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -10,11 +10,9 @@ jobs: unit-tests: runs-on: ubuntu-latest steps: - - name: Check out code - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Setup go env - uses: actions/setup-go@v2 + - uses: actions/setup-go@v2 with: go-version: ^1.15 diff --git a/Makefile b/Makefile index 97d858d73b3..8132503066c 100644 --- a/Makefile +++ b/Makefile @@ -124,8 +124,8 @@ es-otel-exporter-integration-test: go-gen .PHONY: test-compile-es-scripts test-compile-es-scripts: - docker run --rm -it -v ${PWD}:/tmp/jaeger python:3-alpine /usr/local/bin/python -m py_compile /tmp/jaeger/plugin/storage/es/esRollover.py - docker run --rm -it -v ${PWD}:/tmp/jaeger python:3-alpine /usr/local/bin/python -m py_compile /tmp/jaeger/plugin/storage/es/esCleaner.py + docker run --rm -v ${PWD}:/tmp/jaeger python:3-alpine /usr/local/bin/python -m py_compile /tmp/jaeger/plugin/storage/es/esRollover.py + docker run --rm -v ${PWD}:/tmp/jaeger python:3-alpine /usr/local/bin/python -m py_compile /tmp/jaeger/plugin/storage/es/esCleaner.py .PHONY: index-cleaner-integration-test index-cleaner-integration-test: docker-images-elastic From 8ee024ec7cb2b84d0091674d9389dcbdcb217b78 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 27 Nov 2020 12:55:25 -0500 Subject: [PATCH 12/27] Clean-up GH action names (#2661) Signed-off-by: WalkerWang731 --- .github/workflows/codeql.yml | 6 +++--- .github/workflows/fossa.yml | 22 ++++++++++------------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 04f02cce35e..0e268ca5ef2 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,4 +1,4 @@ -name: "Code Scanning - Action" +name: "CodeQL" on: push: @@ -9,8 +9,8 @@ on: - cron: '31 6 * * 1' jobs: - analyze: - name: Analyze + codeql-analyze: + name: CodeQL Analyze runs-on: ubuntu-latest strategy: diff --git a/.github/workflows/fossa.yml b/.github/workflows/fossa.yml index 45e647432a8..87890311a23 100644 --- a/.github/workflows/fossa.yml +++ b/.github/workflows/fossa.yml @@ -6,25 +6,23 @@ on: branches: [master] jobs: - build: + fossa-license-scan: runs-on: ubuntu-latest steps: - - name: Checkout code - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Use Go ^1.14.x - uses: actions/setup-go@v2 + - uses: actions/setup-go@v2 with: go-version: ^1.14.x - - name: Add GOPATH to GITHUB_ENV - run: echo "GOPATH=$(go env GOPATH)" >>"$GITHUB_ENV" - - - name: Add GOPATH to GITHUB_PATH - run: echo "$GOPATH/bin" >>"$GITHUB_PATH" - - - name: Run FOSSA scan and upload build data + - name: Add GOPATH + run: | + echo "GOPATH=$(go env GOPATH)" + echo "GOPATH=$(go env GOPATH)" >>"$GITHUB_ENV" + echo "$GOPATH/bin" >>"$GITHUB_PATH" + + - name: Run FOSSA scan and upload report uses: fossa-contrib/fossa-action@v1 with: # FOSSA Push-Only API Token From 111efb79ed4df35f4e24fed9d2ede0a612c5208a Mon Sep 17 00:00:00 2001 From: Ashmita Date: Sat, 28 Nov 2020 03:09:08 +0800 Subject: [PATCH 13/27] Fix for failures in badger integration tests (#2660) Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- .../memory-badger-integration-tests.yml | 20 +++++++++++++++++ .../storage/integration/badgerstore_test.go | 22 ++++++------------- 2 files changed, 27 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/memory-badger-integration-tests.yml diff --git a/.github/workflows/memory-badger-integration-tests.yml b/.github/workflows/memory-badger-integration-tests.yml new file mode 100644 index 00000000000..3c62cce9ca5 --- /dev/null +++ b/.github/workflows/memory-badger-integration-tests.yml @@ -0,0 +1,20 @@ +name: Integration Tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + memory-badger: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - name: Run in-memory and badger integration tests + run: make mem-and-badger-storage-integration-test diff --git a/plugin/storage/integration/badgerstore_test.go b/plugin/storage/integration/badgerstore_test.go index c5471f1769e..6c2ee01affd 100644 --- a/plugin/storage/integration/badgerstore_test.go +++ b/plugin/storage/integration/badgerstore_test.go @@ -15,8 +15,6 @@ package integration import ( - "fmt" - "io" "os" "testing" @@ -30,22 +28,23 @@ import ( type BadgerIntegrationStorage struct { StorageIntegration - logger *zap.Logger + logger *zap.Logger + factory *badger.Factory } func (s *BadgerIntegrationStorage) initialize() error { - f := badger.NewFactory() + s.factory = badger.NewFactory() - err := f.Initialize(metrics.NullFactory, zap.NewNop()) + err := s.factory.Initialize(metrics.NullFactory, zap.NewNop()) if err != nil { return err } - sw, err := f.CreateSpanWriter() + sw, err := s.factory.CreateSpanWriter() if err != nil { return err } - sr, err := f.CreateSpanReader() + sr, err := s.factory.CreateSpanReader() if err != nil { return err } @@ -65,14 +64,7 @@ func (s *BadgerIntegrationStorage) initialize() error { } func (s *BadgerIntegrationStorage) clear() error { - if closer, ok := s.SpanWriter.(io.Closer); ok { - err := closer.Close() - if err != nil { - return err - } - return nil - } - return fmt.Errorf("BadgerIntegrationStorage did not implement io.Closer, unable to close and cleanup the storage correctly") + return s.factory.Close() } func (s *BadgerIntegrationStorage) cleanUp() error { From e4da60944cb6e5245a18f84cfe2e9fa6c624f2b9 Mon Sep 17 00:00:00 2001 From: Ashmita Date: Sat, 28 Nov 2020 04:00:11 +0800 Subject: [PATCH 14/27] Add protogen validation test (#2662) Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- .github/workflows/protogen-tests.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/workflows/protogen-tests.yml diff --git a/.github/workflows/protogen-tests.yml b/.github/workflows/protogen-tests.yml new file mode 100644 index 00000000000..54f07a177e6 --- /dev/null +++ b/.github/workflows/protogen-tests.yml @@ -0,0 +1,22 @@ +name: Validation + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + protogen: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + submodules: true + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - name: Run protogen validation + run: make proto && git diff --name-status --exit-code From 9ad60335c7bd3e4d60f27d8199256645148259b3 Mon Sep 17 00:00:00 2001 From: Ashmita Date: Mon, 30 Nov 2020 03:45:50 +0800 Subject: [PATCH 15/27] Add github action for jaeger all-in-one image (#2663) * Add github action for jaeger all-in-one image Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara * Feedbacks changes Signed-off-by: Ashmita Bohara * Make steps self-explantory Signed-off-by: Ashmita Bohara * Fix git tags issue Signed-off-by: Ashmita Bohara * Fix ES integration test Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- .github/workflows/all-in-one-build.yml | 57 ++++++++++++++++++++++ .github/workflows/es-integration-tests.yml | 3 ++ RELEASE.md | 2 +- scripts/travis/build-all-in-one-image.sh | 15 ++++-- scripts/travis/es-integration-test.sh | 2 +- 5 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/all-in-one-build.yml diff --git a/.github/workflows/all-in-one-build.yml b/.github/workflows/all-in-one-build.yml new file mode 100644 index 00000000000..14cc9c6c93c --- /dev/null +++ b/.github/workflows/all-in-one-build.yml @@ -0,0 +1,57 @@ +name: Build all-in-one + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + all-in-one: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + submodules: true + + - name: Fetch git tags + run: | + git fetch --prune --unshallow --tags + + - uses: actions/setup-go@v2 + with: + go-version: ^1.15 + + - uses: actions/setup-node@v2-beta + with: + node-version: '10' + + - uses: docker/login-action@v1 + id: dockerhub-login + with: + username: jaegertracingbot + password: ${{ secrets.DOCKERHUB_TOKEN }} + env: + DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }} + if: env.DOCKERHUB_TOKEN != null + + - name: Export DOCKERHUB_LOGIN variable + run: | + echo "DOCKERHUB_LOGIN=true" >> $GITHUB_ENV + if: steps.dockerhub-login.outcome == 'success' + + - name: Export BRANCH variable for pull_request event + run: | + echo "BRANCH=${GITHUB_HEAD_REF}" >> $GITHUB_ENV + if: github.event_name == 'pull_request' + + - name: Export BRANCH variable for push event + run: | + echo "BRANCH=${GITHUB_REF##*/}" >> $GITHUB_ENV + if: github.event_name == 'push' + + - name: Install tools + run: make install-ci + + - name: Build, test, and publish all-in-one image + run: bash scripts/travis/build-all-in-one-image.sh diff --git a/.github/workflows/es-integration-tests.yml b/.github/workflows/es-integration-tests.yml index d36b0704947..92768daed06 100644 --- a/.github/workflows/es-integration-tests.yml +++ b/.github/workflows/es-integration-tests.yml @@ -16,5 +16,8 @@ jobs: with: go-version: ^1.15 + - name: Install tools + run: make install-ci + - name: Run elasticsearch integration tests run: bash scripts/travis/es-integration-test.sh diff --git a/RELEASE.md b/RELEASE.md index 81ced8142f7..b9e390ee6a3 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -16,7 +16,7 @@ * Title "Release X.Y.Z" * Tag `vX.Y.Z` (note the `v` prefix) and choose appropriate branch * Copy the new CHANGELOG.md section into the release notes -4. The release tag will trigger a build of the docker images +4. The release tag will trigger a build of the docker images. Since forks don't have jaegertracingbot dockerhub token, they can never publish images to jaegertracing organisation. 5. Once the images are available on [Docker Hub](https://hub.docker.com/r/jaegertracing/), announce the release on the mailing list, gitter, and twitter. 6. Publish documentation for the new version in [jaegertracing.io](https://github.com/jaegertracing/documentation). diff --git a/scripts/travis/build-all-in-one-image.sh b/scripts/travis/build-all-in-one-image.sh index 46492a387d8..aae4eecf19e 100755 --- a/scripts/travis/build-all-in-one-image.sh +++ b/scripts/travis/build-all-in-one-image.sh @@ -8,8 +8,16 @@ BRANCH=${BRANCH:?'missing BRANCH env var'} # `GOARCH= ./scripts/travis/build-all-in-one-image.sh`. GOARCH=${GOARCH:-$(go env GOARCH)} -source ~/.nvm/nvm.sh -nvm use 10 +expected_version="v10" +version=$(node --version) +major_version=${version%.*.*} +if [ "$major_version" = "$expected_version" ] ; then + echo "Node version is as expected: $version" +else + echo "ERROR: installed Node version $version doesn't match expected version $expected_version" + exit 1 +fi + make build-ui set +e @@ -28,7 +36,7 @@ run_integration_test() { upload_to_docker() { # Only push the docker container to Docker Hub for master branch - if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "$TRAVIS_SECURE_ENV_VARS" == "true" ]]; then + if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "DOCKERHUB_LOGIN" == "true" ]]; then echo "upload $1 to Docker Hub" export REPO=$1 bash ./scripts/travis/upload-to-docker.sh @@ -38,6 +46,7 @@ upload_to_docker() { } make build-all-in-one GOOS=linux GOARCH=$GOARCH +make create-baseimg-debugimg repo=jaegertracing/all-in-one docker build -f cmd/all-in-one/Dockerfile \ --target release \ diff --git a/scripts/travis/es-integration-test.sh b/scripts/travis/es-integration-test.sh index 4abeb4435df..fd9a3af3841 100755 --- a/scripts/travis/es-integration-test.sh +++ b/scripts/travis/es-integration-test.sh @@ -26,7 +26,7 @@ if [ "$ES_OTEL_INTEGRATION_TEST" == true ]; then exit 0 fi -echo "Executing token propatagion test" +echo "Executing token propagation test" # Mock UI, needed only for build query service. make build-crossdock-ui-placeholder From faa61ac629ba4dcbd255dc4804fc9a9a8dcd9e8c Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 29 Nov 2020 14:47:26 -0500 Subject: [PATCH 16/27] Update comment that looks confusing during builds Signed-off-by: Yuri Shkuro Signed-off-by: WalkerWang731 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8132503066c..a7f3d4f7f12 100644 --- a/Makefile +++ b/Makefile @@ -228,7 +228,7 @@ run-all-in-one: build-ui .PHONY: build-ui build-ui: cmd/query/app/ui/actual/gen_assets.go cmd/query/app/ui/placeholder/gen_assets.go - # Do nothing. If you need to force a rebuild of UI assets, run `make clean`. + # UI packaged assets are up-to-date. To force a rebuild, run `make clean`. jaeger-ui/packages/jaeger-ui/build/index.html: cd jaeger-ui && yarn install --frozen-lockfile && cd packages/jaeger-ui && yarn build From 71755cc12f71c03dd29d3763d9625c8398cf9089 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 29 Nov 2020 15:00:06 -0500 Subject: [PATCH 17/27] Use GitHub actions based build badges Signed-off-by: Yuri Shkuro Signed-off-by: WalkerWang731 --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index bb1bc26acd6..f17fd199aba 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![Gitter chat][gitter-img]][gitter] [![Project+Community stats][community-badge]][community-stats] -[![Build Status][ci-img]][ci] [![Coverage Status][cov-img]][cov] [![FOSSA Status](https://app.fossa.com/api/projects/custom%2B162%2Fgithub.com%2Fjaegertracing%2Fjaeger.svg?type=shield)](https://app.fossa.com/projects/custom%2B162%2Fgithub.com%2Fjaegertracing%2Fjaeger?ref=badge_shield) +[![Unit Tests][ci-img]][ci] [![Coverage Status][cov-img]][cov] [![FOSSA Status][fossa-img]][ci] # Jaeger - a Distributed Tracing System @@ -174,10 +174,11 @@ If you would like to add your organization to the list, please comment on our [doc]: https://jaegertracing.io/docs/ [godoc-img]: https://godoc.org/github.com/jaegertracing/jaeger?status.svg [godoc]: https://godoc.org/github.com/jaegertracing/jaeger -[ci-img]: https://travis-ci.com/jaegertracing/jaeger.svg?branch=master -[ci]: https://travis-ci.com/jaegertracing/jaeger +[ci-img]: https://github.com/jaegertracing/jaeger/workflows/Unit%20Tests/badge.svg?branch=master +[ci]: https://github.com/jaegertracing/jaeger/actions?query=branch%3Amaster [cov-img]: https://codecov.io/gh/jaegertracing/jaeger/branch/master/graph/badge.svg [cov]: https://codecov.io/gh/jaegertracing/jaeger/branch/master/ +[fossa-img]: https://github.com/jaegertracing/jaeger/workflows/FOSSA/badge.svg?branch=master [dapper]: https://research.google.com/pubs/pub36356.html [ubeross]: https://uber.github.io [ot-badge]: https://img.shields.io/badge/OpenTracing--1.x-inside-blue.svg From 0274fcfcac1947a59fb0c0131b62f0c20dccf17e Mon Sep 17 00:00:00 2001 From: Ashmita Date: Mon, 30 Nov 2020 09:40:19 +0800 Subject: [PATCH 18/27] Fix and minor improvements to all-in-one github action (#2667) Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- .github/workflows/all-in-one-build.yml | 4 +++- scripts/travis/build-all-in-one-image.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/all-in-one-build.yml b/.github/workflows/all-in-one-build.yml index 14cc9c6c93c..85c6c4669f8 100644 --- a/.github/workflows/all-in-one-build.yml +++ b/.github/workflows/all-in-one-build.yml @@ -42,7 +42,9 @@ jobs: - name: Export BRANCH variable for pull_request event run: | - echo "BRANCH=${GITHUB_HEAD_REF}" >> $GITHUB_ENV + export BRANCH=${GITHUB_HEAD_REF} + echo "we are on branch=$BRANCH" + echo "BRANCH=${BRANCH}" >> $GITHUB_ENV if: github.event_name == 'pull_request' - name: Export BRANCH variable for push event diff --git a/scripts/travis/build-all-in-one-image.sh b/scripts/travis/build-all-in-one-image.sh index aae4eecf19e..d888abbc26e 100755 --- a/scripts/travis/build-all-in-one-image.sh +++ b/scripts/travis/build-all-in-one-image.sh @@ -36,7 +36,7 @@ run_integration_test() { upload_to_docker() { # Only push the docker container to Docker Hub for master branch - if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "DOCKERHUB_LOGIN" == "true" ]]; then + if [[ ("$BRANCH" == "master" || $BRANCH =~ ^v[0-9]+\.[0-9]+\.[0-9]+$) && "$DOCKERHUB_LOGIN" == "true" ]]; then echo "upload $1 to Docker Hub" export REPO=$1 bash ./scripts/travis/upload-to-docker.sh From c222b0d1cbf08d23932b3c5a01aa030bf56dc57a Mon Sep 17 00:00:00 2001 From: Ashmita Date: Mon, 30 Nov 2020 11:59:42 +0800 Subject: [PATCH 19/27] Fix docker login issue with all-in-one build (#2668) * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- scripts/travis/upload-to-docker.sh | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/scripts/travis/upload-to-docker.sh b/scripts/travis/upload-to-docker.sh index 8de81afae01..65d49a6ff74 100755 --- a/scripts/travis/upload-to-docker.sh +++ b/scripts/travis/upload-to-docker.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -euxf -o pipefail BRANCH=${BRANCH:?'missing BRANCH env var'} IMAGE="${REPO:?'missing REPO env var'}:latest" @@ -15,8 +15,7 @@ elif [[ $BRANCH =~ ^v([0-9]+)\.([0-9]+)\.([0-9]+)$ ]]; then TAG=${major}.${minor}.${patch} echo "BRANCH is a release tag: major=$major, minor=$minor, patch=$patch" else - # TODO why do we do /// ? - TAG="${BRANCH///}" + TAG="${BRANCH}" fi echo "TRAVIS_BRANCH=$TRAVIS_BRANCH, REPO=$REPO, BRANCH=$BRANCH, TAG=$TAG, IMAGE=$IMAGE" @@ -31,15 +30,6 @@ if [[ -n $major ]]; then fi fi -if [[ -f $HOME/.docker/config.json ]]; then - rm -f $HOME/.docker/config.json -else - echo "$HOME/.docker/config.json doesn't exist" -fi - -# Do not enable echo before the `docker login` command to avoid revealing the password. -set -x -docker login docker.io -u $DOCKER_USER -p $DOCKER_PASS if [[ "${REPO}" == "jaegertracing/jaeger-opentelemetry-collector" || "${REPO}" == "jaegertracing/jaeger-opentelemetry-agent" || "${REPO}" == "jaegertracing/jaeger-opentelemetry-ingester" || "${REPO}" == "jaegertracing/opentelemetry-all-in-one" ]]; then # TODO remove once Jaeger OTEL collector is stable docker push $REPO:latest From 01d1e9c687a9800a3f3e0a5728acf20a3d855137 Mon Sep 17 00:00:00 2001 From: Ashmita Date: Mon, 30 Nov 2020 15:05:21 +0800 Subject: [PATCH 20/27] Fix issue with all-in-one build (#2669) Signed-off-by: Ashmita Bohara Signed-off-by: WalkerWang731 --- scripts/travis/upload-to-docker.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/travis/upload-to-docker.sh b/scripts/travis/upload-to-docker.sh index 65d49a6ff74..607de83cf6c 100755 --- a/scripts/travis/upload-to-docker.sh +++ b/scripts/travis/upload-to-docker.sh @@ -17,7 +17,7 @@ elif [[ $BRANCH =~ ^v([0-9]+)\.([0-9]+)\.([0-9]+)$ ]]; then else TAG="${BRANCH}" fi -echo "TRAVIS_BRANCH=$TRAVIS_BRANCH, REPO=$REPO, BRANCH=$BRANCH, TAG=$TAG, IMAGE=$IMAGE" +echo "REPO=$REPO, BRANCH=$BRANCH, TAG=$TAG, IMAGE=$IMAGE" # add major, major.minor and major.minor.patch tags if [[ -n $major ]]; then @@ -38,7 +38,7 @@ else docker push $REPO fi -SNAPSHOT_IMAGE="$REPO-snapshot:$TRAVIS_COMMIT" +SNAPSHOT_IMAGE="$REPO-snapshot:$GITHUB_SHA" echo "Pushing snapshot image $SNAPSHOT_IMAGE" docker tag $IMAGE $SNAPSHOT_IMAGE docker push $SNAPSHOT_IMAGE From fb1f57a49b5790208d7f68c8fc2b2e6c39a2768b Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 15:38:41 +0800 Subject: [PATCH 21/27] Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index d019e716228..5774fd87b93 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -24,7 +24,7 @@ import ( // connectMetrics is real metric, but not support to directly change, need via ConnectMetrics for changed type connectMetrics struct { // used for reflect current connection stability - ConnectedCollectorReconnect metrics.Counter `metric:"connected_collector_reconnect" help:"Default is 1, the metric can reflect current connection stability, as reconnect action increase the metric increase."` + Reconnects metrics.Counter `metric:"collector_reconnects" help:"Number of successful connections (including reconnects) to the collector."` // Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected ConnectedCollectorStatus metrics.Gauge `metric:"connected_collector_status" help:"Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected"` From ba18453d97109954bcf59a023f1d86d55e9ca3cc Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 15:39:00 +0800 Subject: [PATCH 22/27] Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 5774fd87b93..8eadf3691e2 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -27,7 +27,7 @@ type connectMetrics struct { Reconnects metrics.Counter `metric:"collector_reconnects" help:"Number of successful connections (including reconnects) to the collector."` // Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected - ConnectedCollectorStatus metrics.Gauge `metric:"connected_collector_status" help:"Connection status that jaeger-agent to jaeger-collector, 1 is connected, 0 is disconnected"` + Status metrics.Gauge `metric:"collector_connected" help:"Status of connection between the agent and the collector; 1 is connected, 0 is disconnected"` } // ConnectMetricsParams include connectMetrics necessary params and connectMetrics, likes connectMetrics API From 68e6d76d305e272572c0cbe8a132e93ece3b3707 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 17:07:32 +0800 Subject: [PATCH 23/27] simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 34 ++++++++----------- .../app/reporter/connect_metrics_test.go | 26 +++++++------- cmd/agent/app/reporter/grpc/builder.go | 22 ++++++------ cmd/agent/app/reporter/grpc/builder_test.go | 12 +++---- 4 files changed, 44 insertions(+), 50 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 8eadf3691e2..f2ef6d17063 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -21,7 +21,6 @@ import ( "go.uber.org/zap" ) -// connectMetrics is real metric, but not support to directly change, need via ConnectMetrics for changed type connectMetrics struct { // used for reflect current connection stability Reconnects metrics.Counter `metric:"collector_reconnects" help:"Number of successful connections (including reconnects) to the collector."` @@ -32,7 +31,7 @@ type connectMetrics struct { // ConnectMetricsParams include connectMetrics necessary params and connectMetrics, likes connectMetrics API // If want to modify metrics of connectMetrics, must via ConnectMetrics API -type ConnectMetricsParams struct { +type ConnectMetrics struct { Logger *zap.Logger // required MetricsFactory metrics.Factory // required ExpireFrequency time.Duration @@ -40,31 +39,28 @@ type ConnectMetricsParams struct { connectMetrics *connectMetrics } -// NewConnectMetrics creates ConnectMetricsParams. -func NewConnectMetrics(cmp ConnectMetricsParams) *ConnectMetricsParams { - if cmp.ExpireFrequency == 0 { - cmp.ExpireFrequency = defaultExpireFrequency +// NewConnectMetrics will be initlizal ConnectMetrics +func (r *ConnectMetrics) NewConnectMetrics() { + if r.ExpireFrequency == 0 { + r.ExpireFrequency = defaultExpireFrequency } - if cmp.ExpireTTL == 0 { - cmp.ExpireTTL = defaultExpireTTL + if r.ExpireTTL == 0 { + r.ExpireTTL = defaultExpireTTL } - cm := new(connectMetrics) - cmp.MetricsFactory = cmp.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) - metrics.MustInit(cm, cmp.MetricsFactory, nil) - cmp.connectMetrics = cm - - return &cmp + r.connectMetrics = new(connectMetrics) + r.MetricsFactory = r.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) + metrics.MustInit(r.connectMetrics, r.MetricsFactory, nil) } // OnConnectionStatusChange used for pass the status parameter when connection is changed // 0 is disconnected, 1 is connected -// For quick view status via use `sum(jaeger_agent_connection_status_connected_collector_status{}) by (instance) > bool 0` -func (r *ConnectMetricsParams) OnConnectionStatusChange(connected bool) { +// For quick view status via use `sum(jaeger_agent_connection_status_collector_connected{}) by (instance) > bool 0` +func (r *ConnectMetrics) OnConnectionStatusChange(connected bool) { if connected { - r.connectMetrics.ConnectedCollectorStatus.Update(1) - r.connectMetrics.ConnectedCollectorReconnect.Inc(1) + r.connectMetrics.Status.Update(1) + r.connectMetrics.Reconnects.Inc(1) } else { - r.connectMetrics.ConnectedCollectorStatus.Update(0) + r.connectMetrics.Status.Update(0) } } diff --git a/cmd/agent/app/reporter/connect_metrics_test.go b/cmd/agent/app/reporter/connect_metrics_test.go index be3111b271b..09bec30987e 100644 --- a/cmd/agent/app/reporter/connect_metrics_test.go +++ b/cmd/agent/app/reporter/connect_metrics_test.go @@ -26,33 +26,33 @@ type connectMetricsTest struct { mf *metricstest.Factory } -func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetricsParams)) { - testConnectMetricsWithParams(ConnectMetricsParams{}, fn) +func testConnectMetrics(fn func(tr *connectMetricsTest, r *ConnectMetrics)) { + testConnectMetricsWithParams(&ConnectMetrics{}, fn) } -func testConnectMetricsWithParams(params ConnectMetricsParams, fn func(tr *connectMetricsTest, r *ConnectMetricsParams)) { +func testConnectMetricsWithParams(cm *ConnectMetrics, fn func(tr *connectMetricsTest, r *ConnectMetrics)) { mf := metricstest.NewFactory(time.Hour) - params.MetricsFactory = mf - r := NewConnectMetrics(params) + cm.MetricsFactory = mf + cm.NewConnectMetrics() tr := &connectMetricsTest{ mf: mf, } - fn(tr, r) + fn(tr, cm) } -func testCollectorConnected(r *ConnectMetricsParams) { +func testCollectorConnected(r *ConnectMetrics) { r.OnConnectionStatusChange(true) } -func testCollectorAborted(r *ConnectMetricsParams) { +func testCollectorAborted(r *ConnectMetrics) { r.OnConnectionStatusChange(false) } func TestConnectMetrics(t *testing.T) { - testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetricsParams) { + testConnectMetrics(func(tr *connectMetricsTest, r *ConnectMetrics) { getGauge := func() map[string]int64 { _, gauges := tr.mf.Snapshot() return gauges @@ -65,17 +65,17 @@ func TestConnectMetrics(t *testing.T) { // testing connect aborted testCollectorAborted(r) - assert.EqualValues(t, 0, getGauge()["connection_status.connected_collector_status"]) + assert.EqualValues(t, 0, getGauge()["connection_status.collector_connected"]) // testing connect connected testCollectorConnected(r) - assert.EqualValues(t, 1, getGauge()["connection_status.connected_collector_status"]) - assert.EqualValues(t, 1, getCount()["connection_status.connected_collector_reconnect"]) + assert.EqualValues(t, 1, getGauge()["connection_status.collector_connected"]) + assert.EqualValues(t, 1, getCount()["connection_status.collector_reconnects"]) // testing reconnect counts testCollectorAborted(r) testCollectorConnected(r) - assert.EqualValues(t, 2, getCount()["connection_status.connected_collector_reconnect"]) + assert.EqualValues(t, 2, getCount()["connection_status.collector_reconnects"]) }) } diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index 0556f545be5..91dc6171efa 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -48,7 +48,8 @@ type ConnBuilder struct { Notifier discovery.Notifier Discoverer discovery.Discoverer - ConnectMetricsParams *reporter.ConnectMetricsParams + // for unit test and provide ConnectMetrics and outside call + ConnectMetrics *reporter.ConnectMetrics } // NewConnBuilder creates a new grpc connection builder. @@ -103,20 +104,17 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact return nil, err } - if b.ConnectMetricsParams == nil { - grpcMetrics := mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}) - - // for unit test and provide ConnectMetricsParams and outside call - cmp := reporter.NewConnectMetrics(reporter.ConnectMetricsParams{ + if b.ConnectMetrics == nil { + cm := reporter.ConnectMetrics{ Logger: logger, - MetricsFactory: grpcMetrics, - }) - b.ConnectMetricsParams = cmp + MetricsFactory: mFactory.Namespace(metrics.NSOptions{Name: "", Tags: map[string]string{"protocol": "grpc"}}), + } + cm.NewConnectMetrics() + b.ConnectMetrics = &cm } - go func(cc *grpc.ClientConn, cm *reporter.ConnectMetricsParams) { + go func(cc *grpc.ClientConn, cm *reporter.ConnectMetrics) { logger.Info("Checking connection to collector") - var egt *expvar.String r := expvar.Get("gRPCTarget") if r == nil { @@ -137,7 +135,7 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s)) cc.WaitForStateChange(context.Background(), s) } - }(conn, b.ConnectMetricsParams) + }(conn, b.ConnectMetrics) return conn, nil } diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index 5a04ddfdfe1..bb18db2fc12 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -156,14 +156,14 @@ func TestBuilderWithCollectors(t *testing.T) { cfg.Discoverer = test.discoverer mf := metricstest.NewFactory(time.Hour) - params := reporter.ConnectMetricsParams{ + cm := reporter.ConnectMetrics{ MetricsFactory: mf, } - cm := reporter.NewConnectMetrics(params) + cm.NewConnectMetrics() tr := &connectMetricsTest{ mf: mf, } - cfg.ConnectMetricsParams = cm + cfg.ConnectMetrics = &cm conn, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) if test.expectedError == "" { @@ -179,13 +179,13 @@ func TestBuilderWithCollectors(t *testing.T) { } if test.expectedState == "READY" { counts, gauges := tr.mf.Snapshot() - assert.EqualValues(t, 1, gauges["connection_status.connected_collector_status"]) - assert.EqualValues(t, 1, counts["connection_status.connected_collector_reconnect"]) + assert.EqualValues(t, 1, gauges["connection_status.collector_connected"]) + assert.EqualValues(t, 1, counts["connection_status.collector_reconnects"]) assert.Equal(t, test.target, expvar.Get("gRPCTarget").(*expvar.String).Value()) } if test.expectedState == "TRANSIENT_FAILURE" { _, gauges := tr.mf.Snapshot() - assert.EqualValues(t, 0, gauges["connection_status.connected_collector_status"]) + assert.EqualValues(t, 0, gauges["connection_status.collector_connected"]) } } else { require.Error(t, err) From ac8483096d2bf8dc42a05326a356b142fabca36b Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 17:09:21 +0800 Subject: [PATCH 24/27] simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index f2ef6d17063..0609d5b318a 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -39,7 +39,7 @@ type ConnectMetrics struct { connectMetrics *connectMetrics } -// NewConnectMetrics will be initlizal ConnectMetrics +// NewConnectMetrics will be initialize ConnectMetrics func (r *ConnectMetrics) NewConnectMetrics() { if r.ExpireFrequency == 0 { r.ExpireFrequency = defaultExpireFrequency From a1ff64283a7eef842570d3db01eb9ea37d73be4f Mon Sep 17 00:00:00 2001 From: "walker.wangxy" Date: Mon, 30 Nov 2020 17:32:59 +0800 Subject: [PATCH 25/27] merage from the lastest master branch and exec make fmt Signed-off-by: walker.wangxy --- cmd/agent/app/reporter/connect_metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 0609d5b318a..d438925b2c3 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -29,7 +29,7 @@ type connectMetrics struct { Status metrics.Gauge `metric:"collector_connected" help:"Status of connection between the agent and the collector; 1 is connected, 0 is disconnected"` } -// ConnectMetricsParams include connectMetrics necessary params and connectMetrics, likes connectMetrics API +// ConnectMetricsParams include connectMetrics necessary params // If want to modify metrics of connectMetrics, must via ConnectMetrics API type ConnectMetrics struct { Logger *zap.Logger // required From d07f8b8e987daa55e58f9359594467e6e1a166ec Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Mon, 30 Nov 2020 18:05:45 +0800 Subject: [PATCH 26/27] add comment on ConnectMetrics Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index d438925b2c3..c6a1a900cfc 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -29,8 +29,7 @@ type connectMetrics struct { Status metrics.Gauge `metric:"collector_connected" help:"Status of connection between the agent and the collector; 1 is connected, 0 is disconnected"` } -// ConnectMetricsParams include connectMetrics necessary params -// If want to modify metrics of connectMetrics, must via ConnectMetrics API +// ConnectMetrics include connectMetrics necessary params if want to modify metrics of connectMetrics, must via ConnectMetrics API type ConnectMetrics struct { Logger *zap.Logger // required MetricsFactory metrics.Factory // required From d00801e8f63f83b7f7db622ce85c59a230af4d75 Mon Sep 17 00:00:00 2001 From: WalkerWang731 Date: Wed, 13 Jan 2021 16:41:45 +0800 Subject: [PATCH 27/27] clear up redundant codes Signed-off-by: WalkerWang731 --- cmd/agent/app/reporter/connect_metrics.go | 17 +++--------- cmd/agent/app/reporter/grpc/builder_test.go | 29 +-------------------- 2 files changed, 4 insertions(+), 42 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index c6a1a900cfc..6efe555a6ba 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -15,8 +15,6 @@ package reporter import ( - "time" - "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" ) @@ -31,22 +29,13 @@ type connectMetrics struct { // ConnectMetrics include connectMetrics necessary params if want to modify metrics of connectMetrics, must via ConnectMetrics API type ConnectMetrics struct { - Logger *zap.Logger // required - MetricsFactory metrics.Factory // required - ExpireFrequency time.Duration - ExpireTTL time.Duration - connectMetrics *connectMetrics + Logger *zap.Logger // required + MetricsFactory metrics.Factory // required + connectMetrics *connectMetrics } // NewConnectMetrics will be initialize ConnectMetrics func (r *ConnectMetrics) NewConnectMetrics() { - if r.ExpireFrequency == 0 { - r.ExpireFrequency = defaultExpireFrequency - } - if r.ExpireTTL == 0 { - r.ExpireTTL = defaultExpireTTL - } - r.connectMetrics = new(connectMetrics) r.MetricsFactory = r.MetricsFactory.Namespace(metrics.NSOptions{Name: "connection_status"}) metrics.MustInit(r.connectMetrics, r.MetricsFactory, nil) diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index bb18db2fc12..4df5c395ba4 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -15,7 +15,6 @@ package grpc import ( "context" - "expvar" "net" "strings" "testing" @@ -29,9 +28,8 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" - "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/discovery" "github.com/jaegertracing/jaeger/proto-gen/api_v2" @@ -48,10 +46,6 @@ var testCertKeyLocation = "../../../../../pkg/config/tlscfg/testdata/" type noopNotifier struct{} -type connectMetricsTest struct { - mf *metricstest.Factory -} - func (noopNotifier) Register(chan<- []string) {} func (noopNotifier) Unregister(chan<- []string) {} @@ -155,16 +149,6 @@ func TestBuilderWithCollectors(t *testing.T) { cfg.Notifier = test.notifier cfg.Discoverer = test.discoverer - mf := metricstest.NewFactory(time.Hour) - cm := reporter.ConnectMetrics{ - MetricsFactory: mf, - } - cm.NewConnectMetrics() - tr := &connectMetricsTest{ - mf: mf, - } - cfg.ConnectMetrics = &cm - conn, err := cfg.CreateConnection(zap.NewNop(), metrics.NullFactory) if test.expectedError == "" { require.NoError(t, err) @@ -177,21 +161,10 @@ func TestBuilderWithCollectors(t *testing.T) { } else { assert.True(t, conn.Target() == test.target) } - if test.expectedState == "READY" { - counts, gauges := tr.mf.Snapshot() - assert.EqualValues(t, 1, gauges["connection_status.collector_connected"]) - assert.EqualValues(t, 1, counts["connection_status.collector_reconnects"]) - assert.Equal(t, test.target, expvar.Get("gRPCTarget").(*expvar.String).Value()) - } - if test.expectedState == "TRANSIENT_FAILURE" { - _, gauges := tr.mf.Snapshot() - assert.EqualValues(t, 0, gauges["connection_status.collector_connected"]) - } } else { require.Error(t, err) assert.Contains(t, err.Error(), test.expectedError) } - }) } }