From e7d4f5f58147899dcd14f58fc4cb540b497ecb3a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2020 09:02:05 -0700 Subject: [PATCH 1/7] Bump github.com/aws/aws-sdk-go from 1.34.9 to 1.34.10 in /detectors/aws (#286) * Bump github.com/aws/aws-sdk-go from 1.34.9 to 1.34.10 in /detectors/aws Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.34.9 to 1.34.10. - [Release notes](https://github.com/aws/aws-sdk-go/releases) - [Changelog](https://github.com/aws/aws-sdk-go/blob/master/CHANGELOG.md) - [Commits](https://github.com/aws/aws-sdk-go/compare/v1.34.9...v1.34.10) Signed-off-by: dependabot[bot] * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] Co-authored-by: Tyler Yahn --- detectors/aws/go.mod | 2 +- detectors/aws/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/detectors/aws/go.mod b/detectors/aws/go.mod index 9bb9c5a22a4..217262e8915 100644 --- a/detectors/aws/go.mod +++ b/detectors/aws/go.mod @@ -3,7 +3,7 @@ module go.opentelemetry.io/contrib/detectors/aws go 1.14 require ( - github.com/aws/aws-sdk-go v1.34.9 + github.com/aws/aws-sdk-go v1.34.10 go.opentelemetry.io/otel v0.11.0 go.opentelemetry.io/otel/sdk v0.11.0 ) diff --git a/detectors/aws/go.sum b/detectors/aws/go.sum index 076cde8c183..f2dec684e12 100644 --- a/detectors/aws/go.sum +++ b/detectors/aws/go.sum @@ -1,8 +1,8 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= -github.com/aws/aws-sdk-go v1.34.9 h1:cUGBW9CVdi0mS7K1hDzxIqTpfeWhpoQiguq81M1tjK0= -github.com/aws/aws-sdk-go v1.34.9/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= +github.com/aws/aws-sdk-go v1.34.10 h1:VU78gcf/3wA4HNEDCHidK738l7K0Bals4SJnfnvXOtY= +github.com/aws/aws-sdk-go v1.34.10/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= From ee5433f66bb0c67e4dc78af9cd9e0a04cb6d8b1e Mon Sep 17 00:00:00 2001 From: Reginald McDonald <40721169+reggiemcdonald@users.noreply.github.com> Date: Wed, 26 Aug 2020 10:32:18 -0600 Subject: [PATCH 2/7] fix: Update gocql instrumentation to meet latest contrib requirements (#298) * fix: change options to accept providers * fix: include semver in instrumentation creation * fix: lint * fix: Change WithTraceProvider to WithTracerProvider Co-authored-by: Tyler Yahn --- .../github.com/gocql/gocql/README.md | 45 ---------------- .../github.com/gocql/gocql/config.go | 25 ++++++--- .../github.com/gocql/gocql/example/client.go | 4 +- .../gocql/gocql/example_session_test.go | 41 +++++++++++++++ .../github.com/gocql/gocql/gocql.go | 17 +++++-- .../github.com/gocql/gocql/gocql_test.go | 40 ++++++++++----- .../github.com/gocql/gocql/instrument.go | 51 ++++++++++--------- .../github.com/gocql/gocql/observer.go | 24 +++++---- 8 files changed, 143 insertions(+), 104 deletions(-) delete mode 100644 instrumentation/github.com/gocql/gocql/README.md create mode 100644 instrumentation/github.com/gocql/gocql/example_session_test.go diff --git a/instrumentation/github.com/gocql/gocql/README.md b/instrumentation/github.com/gocql/gocql/README.md deleted file mode 100644 index 1c9ca0b9843..00000000000 --- a/instrumentation/github.com/gocql/gocql/README.md +++ /dev/null @@ -1,45 +0,0 @@ -## `go.opentelemetry.io/contrib/instrumentation/github.com/gocql/gocql` - -This package provides tracing and metrics to the golang cassandra client `github.com/gocql/gocql` using the `ConnectObserver`, `QueryObserver` and `BatchObserver` interfaces. - -To enable tracing in your application: - -```go -package main - -import ( - "context" - - "github.com/gocql/gocql" - otelGocql "go.opentelemetry.io/contrib/instrumentation/github.com/gocql/gocql" -) - -func main() { - // Create a cluster - host := "localhost" - cluster := gocql.NewCluster(host) - - // Create a session with tracing - session, err := otelGocql.NewSessionWithTracing( - context.Background(), - cluster, - // Include any options here - ) - - // Begin using the session - -} -``` - -You can customize instrumentation by passing any of the following options to `NewSessionWithTracing`: - -| Function | Description | -| -------- | ----------- | -| `WithQueryObserver(gocql.QueryObserver)` | Specify an additional QueryObserver to be called. | -| `WithBatchObserver(gocql.BatchObserver)` | Specify an additional BatchObserver to be called. | -| `WithConnectObserver(gocql.ConnectObserver)` | Specify an additional ConnectObserver to be called. | -| `WithTracer(trace.Tracer)` | The tracer to be used to create spans for the gocql session. If not specified, `global.Tracer("go.opentelemetry.io/contrib/instrumentation/github.com/gocql/gocql")` will be used. | -| `WithQueryInstrumentation(bool)` | To enable/disable tracing and metrics for queries. | -| `WithBatchInstrumentation(bool)` | To enable/disable tracing and metrics for batch queries. | -| `WithConnectInstrumentation(bool)` | To enable/disable tracing and metrics for new connections. | - diff --git a/instrumentation/github.com/gocql/gocql/config.go b/instrumentation/github.com/gocql/gocql/config.go index c761c9aefe2..deed87e8f9e 100644 --- a/instrumentation/github.com/gocql/gocql/config.go +++ b/instrumentation/github.com/gocql/gocql/config.go @@ -18,13 +18,15 @@ import ( "github.com/gocql/gocql" "go.opentelemetry.io/otel/api/global" + "go.opentelemetry.io/otel/api/metric" "go.opentelemetry.io/otel/api/trace" ) // TracedSessionConfig provides configuration for sessions // created with NewSessionWithTracing. type TracedSessionConfig struct { - tracer trace.Tracer + tracerProvider trace.Provider + meterProvider metric.Provider instrumentQuery bool instrumentBatch bool instrumentConnect bool @@ -78,12 +80,20 @@ func WithConnectObserver(observer gocql.ConnectObserver) TracedSessionOption { }) } -// WithTracer will set tracer to be the tracer used to create spans -// for query, batch query, and connection instrumentation. -// Defaults to global.Tracer("go.opentelemetry.io/contrib/instrumentation/github.com/gocql/gocql"). -func WithTracer(tracer trace.Tracer) TracedSessionOption { +// WithTracerProvider will set the trace provider used to get a tracer +// for creating spans. Defaults to global.TraceProvider() +func WithTracerProvider(provider trace.Provider) TracedSessionOption { return TracedSessionOptionFunc(func(c *TracedSessionConfig) { - c.tracer = tracer + c.tracerProvider = provider + }) +} + +// WithMeterProvider will set the meter provider used to get a meter +// for creating instruments. +// Defaults to global.MeterProvider(). +func WithMeterProvider(provider metric.Provider) TracedSessionOption { + return TracedSessionOptionFunc(func(c *TracedSessionConfig) { + c.meterProvider = provider }) } @@ -115,7 +125,8 @@ func WithConnectInstrumentation(enabled bool) TracedSessionOption { func configure(options ...TracedSessionOption) *TracedSessionConfig { config := &TracedSessionConfig{ - tracer: global.Tracer(instrumentationName), + tracerProvider: global.TraceProvider(), + meterProvider: global.MeterProvider(), instrumentQuery: true, instrumentBatch: true, instrumentConnect: true, diff --git a/instrumentation/github.com/gocql/gocql/example/client.go b/instrumentation/github.com/gocql/gocql/example/client.go index 4a296a73db3..e4320d86565 100644 --- a/instrumentation/github.com/gocql/gocql/example/client.go +++ b/instrumentation/github.com/gocql/gocql/example/client.go @@ -110,7 +110,7 @@ func main() { func initMetrics() { // Start prometheus - metricExporter, err := prometheus.NewExportPipeline(prometheus.Config{}) + metricExporter, err := prometheus.InstallNewPipeline(prometheus.Config{}) if err != nil { log.Fatalf("failed to install metric exporter, %v", err) } @@ -133,8 +133,6 @@ func initMetrics() { log.Print("gracefully shutting down server") } }() - - otelGocql.InstrumentWithProvider(metricExporter.Provider()) } func initTracer() { diff --git a/instrumentation/github.com/gocql/gocql/example_session_test.go b/instrumentation/github.com/gocql/gocql/example_session_test.go new file mode 100644 index 00000000000..b3a268fba81 --- /dev/null +++ b/instrumentation/github.com/gocql/gocql/example_session_test.go @@ -0,0 +1,41 @@ +// Copyright The OpenTelemetry 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 gocql + +import ( + "context" + "log" + + "github.com/gocql/gocql" +) + +func ExampleNewSessionWithTracing() { + // Create a cluster + host := "localhost" + cluster := gocql.NewCluster(host) + + // Create a session with tracing + _, err := NewSessionWithTracing( + context.Background(), + cluster, + // Include any options here + ) + + if err != nil { + log.Fatalf("failed to create session, %v", err) + } + + // Begin using the session +} diff --git a/instrumentation/github.com/gocql/gocql/gocql.go b/instrumentation/github.com/gocql/gocql/gocql.go index 1dfa14c08fb..5bf084b4228 100644 --- a/instrumentation/github.com/gocql/gocql/gocql.go +++ b/instrumentation/github.com/gocql/gocql/gocql.go @@ -18,6 +18,9 @@ import ( "context" "github.com/gocql/gocql" + + otelcontrib "go.opentelemetry.io/contrib" + "go.opentelemetry.io/otel/api/trace" ) // NewSessionWithTracing creates a new session using the given cluster @@ -25,21 +28,29 @@ import ( // You may use additional observers and disable specific tracing using the provided `TracedSessionOption`s. func NewSessionWithTracing(ctx context.Context, cluster *gocql.ClusterConfig, options ...TracedSessionOption) (*gocql.Session, error) { config := configure(options...) + instruments := newInstruments(config.meterProvider) + tracer := config.tracerProvider.Tracer( + instrumentationName, + trace.WithInstrumentationVersion(otelcontrib.SemVersion()), + ) cluster.QueryObserver = &OTelQueryObserver{ enabled: config.instrumentQuery, observer: config.queryObserver, - tracer: config.tracer, + tracer: tracer, + inst: instruments, } cluster.BatchObserver = &OTelBatchObserver{ enabled: config.instrumentBatch, observer: config.batchObserver, - tracer: config.tracer, + tracer: tracer, + inst: instruments, } cluster.ConnectObserver = &OTelConnectObserver{ ctx: ctx, enabled: config.instrumentConnect, observer: config.connectObserver, - tracer: config.tracer, + tracer: tracer, + inst: instruments, } return cluster.CreateSession() } diff --git a/instrumentation/github.com/gocql/gocql/gocql_test.go b/instrumentation/github.com/gocql/gocql/gocql_test.go index 09ed21e730d..6d0050b85c2 100644 --- a/instrumentation/github.com/gocql/gocql/gocql_test.go +++ b/instrumentation/github.com/gocql/gocql/gocql_test.go @@ -89,6 +89,20 @@ func mockExportPipeline(t *testing.T) *push.Controller { return controller } +type mockTracerProvider struct { + tracer *mocktracer.Tracer +} + +func (p *mockTracerProvider) Tracer(name string, options ...trace.TracerOption) trace.Tracer { + return p.tracer +} + +func newTracerProvider() *mockTracerProvider { + return &mockTracerProvider{ + mocktracer.NewTracer(instrumentationName), + } +} + type mockConnectObserver struct { callCount int } @@ -108,14 +122,15 @@ func TestQuery(t *testing.T) { controller := getController(t) defer afterEach() cluster := getCluster() - tracer := mocktracer.NewTracer("gocql-test") + tracerProvider := newTracerProvider() - ctx, parentSpan := tracer.Start(context.Background(), "gocql-test") + ctx, parentSpan := tracerProvider.tracer.Start(context.Background(), "gocql-test") session, err := NewSessionWithTracing( ctx, cluster, - WithTracer(tracer), + WithTracerProvider(tracerProvider), + WithMeterProvider(controller.Provider()), WithConnectInstrumentation(false), ) require.NoError(t, err) @@ -132,7 +147,7 @@ func TestQuery(t *testing.T) { parentSpan.End() // Get the spans and ensure that they are child spans to the local parent - spans := tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() // Collect all the connection spans // total spans: @@ -229,14 +244,15 @@ func TestBatch(t *testing.T) { controller := getController(t) defer afterEach() cluster := getCluster() - tracer := mocktracer.NewTracer("gocql-test") + tracerProvider := newTracerProvider() - ctx, parentSpan := tracer.Start(context.Background(), "gocql-test") + ctx, parentSpan := tracerProvider.tracer.Start(context.Background(), "gocql-test") session, err := NewSessionWithTracing( ctx, cluster, - WithTracer(tracer), + WithTracerProvider(tracerProvider), + WithMeterProvider(controller.Provider()), WithConnectInstrumentation(false), ) require.NoError(t, err) @@ -255,7 +271,7 @@ func TestBatch(t *testing.T) { parentSpan.End() - spans := tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() // total spans: // 1 span for the query // 1 span for the local span @@ -326,21 +342,22 @@ func TestConnection(t *testing.T) { controller := getController(t) defer afterEach() cluster := getCluster() - tracer := mocktracer.NewTracer("gocql-test") + tracerProvider := newTracerProvider() connectObserver := &mockConnectObserver{0} ctx := context.Background() session, err := NewSessionWithTracing( ctx, cluster, - WithTracer(tracer), + WithTracerProvider(tracerProvider), + WithMeterProvider(controller.Provider()), WithConnectObserver(connectObserver), ) require.NoError(t, err) defer session.Close() require.NoError(t, session.AwaitSchemaAgreement(ctx)) - spans := tracer.EndedSpans() + spans := tracerProvider.tracer.EndedSpans() assert.Less(t, 0, connectObserver.callCount) @@ -422,7 +439,6 @@ func getCluster() *gocql.ClusterConfig { // export pipeline. func getController(t *testing.T) *push.Controller { controller := mockExportPipeline(t) - InstrumentWithProvider(controller.Provider()) return controller } diff --git a/instrumentation/github.com/gocql/gocql/instrument.go b/instrumentation/github.com/gocql/gocql/instrument.go index bbc13762956..7808ffa86b7 100644 --- a/instrumentation/github.com/gocql/gocql/instrument.go +++ b/instrumentation/github.com/gocql/gocql/instrument.go @@ -17,73 +17,74 @@ package gocql import ( "log" - "go.opentelemetry.io/otel/api/global" - + otelcontrib "go.opentelemetry.io/contrib" "go.opentelemetry.io/otel/api/metric" "go.opentelemetry.io/otel/api/unit" ) -var ( - // iQueryCount is the number of queries executed. - iQueryCount metric.Int64Counter +type instruments struct { + // queryCount is the number of queries executed. + queryCount metric.Int64Counter - // iQueryRows is the number of rows returned by a query. - iQueryRows metric.Int64ValueRecorder + // queryRows is the number of rows returned by a query. + queryRows metric.Int64ValueRecorder - // iBatchCount is the number of batch queries executed. - iBatchCount metric.Int64Counter + // batchCount is the number of batch queries executed. + batchCount metric.Int64Counter - // iConnectionCount is the number of connections made + // connectionCount is the number of connections made // with the traced session. - iConnectionCount metric.Int64Counter + connectionCount metric.Int64Counter - // iLatency is the sum of attempt latencies. - iLatency metric.Int64ValueRecorder -) + // latency is the sum of attempt latencies. + latency metric.Int64ValueRecorder +} -// InstrumentWithProvider will recreate instruments using a meter +// newInstruments will create instruments using a meter // from the given provider p. -func InstrumentWithProvider(p metric.Provider) { - meter := p.Meter(instrumentationName) +func newInstruments(p metric.Provider) *instruments { + meter := p.Meter( + instrumentationName, + metric.WithInstrumentationVersion(otelcontrib.SemVersion()), + ) + instruments := &instruments{} var err error - if iQueryCount, err = meter.NewInt64Counter( + if instruments.queryCount, err = meter.NewInt64Counter( "db.cassandra.queries", metric.WithDescription("Number queries executed"), ); err != nil { log.Printf("failed to create iQueryCount instrument, %v", err) } - if iQueryRows, err = meter.NewInt64ValueRecorder( + if instruments.queryRows, err = meter.NewInt64ValueRecorder( "db.cassandra.rows", metric.WithDescription("Number of rows returned from query"), ); err != nil { log.Printf("failed to create iQueryRows instrument, %v", err) } - if iBatchCount, err = meter.NewInt64Counter( + if instruments.batchCount, err = meter.NewInt64Counter( "db.cassandra.batch.queries", metric.WithDescription("Number of batch queries executed"), ); err != nil { log.Printf("failed to create iBatchCount instrument, %v", err) } - if iConnectionCount, err = meter.NewInt64Counter( + if instruments.connectionCount, err = meter.NewInt64Counter( "db.cassandra.connections", metric.WithDescription("Number of connections created"), ); err != nil { log.Printf("failed to create iConnectionCount instrument, %v", err) } - if iLatency, err = meter.NewInt64ValueRecorder( + if instruments.latency, err = meter.NewInt64ValueRecorder( "db.cassandra.latency", metric.WithDescription("Sum of latency to host in milliseconds"), metric.WithUnit(unit.Milliseconds), ); err != nil { log.Printf("failed to create iLatency instrument, %v", err) } -} -func init() { - InstrumentWithProvider(global.MeterProvider()) + return instruments } diff --git a/instrumentation/github.com/gocql/gocql/observer.go b/instrumentation/github.com/gocql/gocql/observer.go index 8465ad86929..41723495a93 100644 --- a/instrumentation/github.com/gocql/gocql/observer.go +++ b/instrumentation/github.com/gocql/gocql/observer.go @@ -32,6 +32,7 @@ type OTelQueryObserver struct { enabled bool observer gocql.QueryObserver tracer trace.Tracer + inst *instruments } // OTelBatchObserver implements the gocql.BatchObserver interface @@ -40,6 +41,7 @@ type OTelBatchObserver struct { enabled bool observer gocql.BatchObserver tracer trace.Tracer + inst *instruments } // OTelConnectObserver implements the gocql.ConnectObserver interface @@ -49,6 +51,7 @@ type OTelConnectObserver struct { enabled bool observer gocql.ConnectObserver tracer trace.Tracer + inst *instruments } // ------------------------------------------ Observer Functions @@ -58,6 +61,7 @@ func (o *OTelQueryObserver) ObserveQuery(ctx context.Context, observedQuery gocq if o.enabled { host := observedQuery.Host keyspace := observedQuery.Keyspace + inst := o.inst attributes := includeKeyValues(host, cassKeyspace(keyspace), @@ -76,7 +80,7 @@ func (o *OTelQueryObserver) ObserveQuery(ctx context.Context, observedQuery gocq if observedQuery.Err != nil { span.SetAttributes(cassErrMsg(observedQuery.Err.Error())) - iQueryCount.Add( + inst.queryCount.Add( ctx, 1, includeKeyValues(host, @@ -86,7 +90,7 @@ func (o *OTelQueryObserver) ObserveQuery(ctx context.Context, observedQuery gocq )..., ) } else { - iQueryCount.Add( + inst.queryCount.Add( ctx, 1, includeKeyValues(host, @@ -98,12 +102,12 @@ func (o *OTelQueryObserver) ObserveQuery(ctx context.Context, observedQuery gocq span.End(trace.WithEndTime(observedQuery.End)) - iQueryRows.Record( + inst.queryRows.Record( ctx, int64(observedQuery.Rows), includeKeyValues(host, cassKeyspace(keyspace))..., ) - iLatency.Record( + inst.latency.Record( ctx, nanoToMilliseconds(observedQuery.Metrics.TotalLatency), includeKeyValues(host, cassKeyspace(keyspace))..., @@ -120,6 +124,7 @@ func (o *OTelBatchObserver) ObserveBatch(ctx context.Context, observedBatch gocq if o.enabled { host := observedBatch.Host keyspace := observedBatch.Keyspace + inst := o.inst attributes := includeKeyValues(host, cassKeyspace(keyspace), @@ -137,7 +142,7 @@ func (o *OTelBatchObserver) ObserveBatch(ctx context.Context, observedBatch gocq if observedBatch.Err != nil { span.SetAttributes(cassErrMsg(observedBatch.Err.Error())) - iBatchCount.Add( + inst.batchCount.Add( ctx, 1, includeKeyValues(host, @@ -146,7 +151,7 @@ func (o *OTelBatchObserver) ObserveBatch(ctx context.Context, observedBatch gocq )..., ) } else { - iBatchCount.Add( + inst.batchCount.Add( ctx, 1, includeKeyValues(host, cassKeyspace(keyspace))..., @@ -155,7 +160,7 @@ func (o *OTelBatchObserver) ObserveBatch(ctx context.Context, observedBatch gocq span.End(trace.WithEndTime(observedBatch.End)) - iLatency.Record( + inst.latency.Record( ctx, nanoToMilliseconds(observedBatch.Metrics.TotalLatency), includeKeyValues(host, cassKeyspace(keyspace))..., @@ -171,6 +176,7 @@ func (o *OTelBatchObserver) ObserveBatch(ctx context.Context, observedBatch gocq func (o *OTelConnectObserver) ObserveConnect(observedConnect gocql.ObservedConnect) { if o.enabled { host := observedConnect.Host + inst := o.inst attributes := includeKeyValues(host, cassConnectOperation()) @@ -184,13 +190,13 @@ func (o *OTelConnectObserver) ObserveConnect(observedConnect gocql.ObservedConne if observedConnect.Err != nil { span.SetAttributes(cassErrMsg(observedConnect.Err.Error())) - iConnectionCount.Add( + inst.connectionCount.Add( o.ctx, 1, includeKeyValues(host, cassErrMsg(observedConnect.Err.Error()))..., ) } else { - iConnectionCount.Add( + inst.connectionCount.Add( o.ctx, 1, includeKeyValues(host)..., From 6128d594126498e2609c8ff81d0ca9fab4de9093 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2020 09:49:37 -0700 Subject: [PATCH 3/7] Bump github.com/google/go-cmp from 0.5.1 to 0.5.2 in /instrumentation/net/http/httptrace (#301) * Bump github.com/google/go-cmp in /instrumentation/net/http/httptrace Bumps [github.com/google/go-cmp](https://github.com/google/go-cmp) from 0.5.1 to 0.5.2. - [Release notes](https://github.com/google/go-cmp/releases) - [Commits](https://github.com/google/go-cmp/compare/v0.5.1...v0.5.2) Signed-off-by: dependabot[bot] * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] Co-authored-by: Tyler Yahn --- instrumentation/net/http/httptrace/example/go.sum | 2 ++ instrumentation/net/http/httptrace/go.mod | 2 +- instrumentation/net/http/httptrace/go.sum | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/instrumentation/net/http/httptrace/example/go.sum b/instrumentation/net/http/httptrace/example/go.sum index 39dde9d56fa..4078113f137 100644 --- a/instrumentation/net/http/httptrace/example/go.sum +++ b/instrumentation/net/http/httptrace/example/go.sum @@ -27,6 +27,8 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k= github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= diff --git a/instrumentation/net/http/httptrace/go.mod b/instrumentation/net/http/httptrace/go.mod index bc725688c27..d315766a99d 100644 --- a/instrumentation/net/http/httptrace/go.mod +++ b/instrumentation/net/http/httptrace/go.mod @@ -5,7 +5,7 @@ go 1.14 replace go.opentelemetry.io/contrib => ../../../.. require ( - github.com/google/go-cmp v0.5.1 + github.com/google/go-cmp v0.5.2 github.com/stretchr/testify v1.6.1 go.opentelemetry.io/otel v0.11.0 ) diff --git a/instrumentation/net/http/httptrace/go.sum b/instrumentation/net/http/httptrace/go.sum index 1413876cac0..e26b4687052 100644 --- a/instrumentation/net/http/httptrace/go.sum +++ b/instrumentation/net/http/httptrace/go.sum @@ -2,6 +2,8 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k= github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= From a58691531ab8b0195e93b1d0225d22dc696267e5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2020 10:04:19 -0700 Subject: [PATCH 4/7] Bump google.golang.org/grpc from 1.31.0 to 1.31.1 in /instrumentation/google.golang.org/grpc (#302) * Bump google.golang.org/grpc in /instrumentation/google.golang.org/grpc Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.31.0 to 1.31.1. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](https://github.com/grpc/grpc-go/compare/v1.31.0...v1.31.1) Signed-off-by: dependabot[bot] * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] Co-authored-by: Tyler Yahn --- instrumentation/google.golang.org/grpc/example/go.mod | 2 +- instrumentation/google.golang.org/grpc/example/go.sum | 2 ++ instrumentation/google.golang.org/grpc/go.mod | 2 +- instrumentation/google.golang.org/grpc/go.sum | 4 ++-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/example/go.mod b/instrumentation/google.golang.org/grpc/example/go.mod index c6d73b28a87..f34457e6d77 100644 --- a/instrumentation/google.golang.org/grpc/example/go.mod +++ b/instrumentation/google.golang.org/grpc/example/go.mod @@ -11,5 +11,5 @@ require ( go.opentelemetry.io/otel/exporters/stdout v0.11.0 go.opentelemetry.io/otel/sdk v0.11.0 golang.org/x/net v0.0.0-20200707034311-ab3426394381 - google.golang.org/grpc v1.31.0 + google.golang.org/grpc v1.31.1 ) diff --git a/instrumentation/google.golang.org/grpc/example/go.sum b/instrumentation/google.golang.org/grpc/example/go.sum index ac3d505ee1f..86ebb2e5057 100644 --- a/instrumentation/google.golang.org/grpc/example/go.sum +++ b/instrumentation/google.golang.org/grpc/example/go.sum @@ -84,6 +84,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= google.golang.org/grpc v1.31.0 h1:T7P4R73V3SSDPhH7WW7ATbfViLtmamH0DKrP3f9AuDI= google.golang.org/grpc v1.31.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= +google.golang.org/grpc v1.31.1 h1:SfXqXS5hkufcdZ/mHtYCh53P2b+92WQq/DZcKLgsFRs= +google.golang.org/grpc v1.31.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/instrumentation/google.golang.org/grpc/go.mod b/instrumentation/google.golang.org/grpc/go.mod index 3beff3b2f00..2be2b3ecc7d 100644 --- a/instrumentation/google.golang.org/grpc/go.mod +++ b/instrumentation/google.golang.org/grpc/go.mod @@ -6,5 +6,5 @@ require ( github.com/golang/protobuf v1.4.2 github.com/stretchr/testify v1.6.1 go.opentelemetry.io/otel v0.11.0 - google.golang.org/grpc v1.31.0 + google.golang.org/grpc v1.31.1 ) diff --git a/instrumentation/google.golang.org/grpc/go.sum b/instrumentation/google.golang.org/grpc/go.sum index 8a061d673c4..66f01f70227 100644 --- a/instrumentation/google.golang.org/grpc/go.sum +++ b/instrumentation/google.golang.org/grpc/go.sum @@ -67,8 +67,8 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= -google.golang.org/grpc v1.31.0 h1:T7P4R73V3SSDPhH7WW7ATbfViLtmamH0DKrP3f9AuDI= -google.golang.org/grpc v1.31.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= +google.golang.org/grpc v1.31.1 h1:SfXqXS5hkufcdZ/mHtYCh53P2b+92WQq/DZcKLgsFRs= +google.golang.org/grpc v1.31.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= From d6b87c88232bafb259f717a2a8e3a0139903a912 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 26 Aug 2020 11:16:02 -0700 Subject: [PATCH 5/7] Add benchmark tests for gRPC instrumentation (#296) * Add benchmark test to grpc instrumentation * Set instrumentation name as const in testing * Update to v0.11.0 otel * Update Changelog * lint --- CHANGELOG.md | 4 + .../google.golang.org/grpc/benchmark_test.go | 108 ++++++++++++++++++ .../google.golang.org/grpc/example/go.sum | 3 + instrumentation/google.golang.org/grpc/go.sum | 3 + 4 files changed, 118 insertions(+) create mode 100644 instrumentation/google.golang.org/grpc/benchmark_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 79187be307f..0290780f354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Benchmark tests for the gRPC instrumentation. (#296) + ## [0.11.0] - 2020-08-25 ### Added diff --git a/instrumentation/google.golang.org/grpc/benchmark_test.go b/instrumentation/google.golang.org/grpc/benchmark_test.go new file mode 100644 index 00000000000..fca62e70459 --- /dev/null +++ b/instrumentation/google.golang.org/grpc/benchmark_test.go @@ -0,0 +1,108 @@ +// Copyright The OpenTelemetry 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 grpc_test + +import ( + "context" + "net" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/interop" + pb "google.golang.org/grpc/interop/grpc_testing" + "google.golang.org/grpc/test/bufconn" + + otelgrpc "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc" + "go.opentelemetry.io/otel/api/trace/tracetest" +) + +const ( + bufSize = 2048 + instName = "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc" +) + +var tracer = tracetest.NewProvider().Tracer(instName) + +func benchmark(b *testing.B, cOpt []grpc.DialOption, sOpt []grpc.ServerOption) { + l := bufconn.Listen(bufSize) + defer l.Close() + + s := grpc.NewServer(sOpt...) + pb.RegisterTestServiceServer(s, interop.NewTestServer()) + go func() { + if err := s.Serve(l); err != nil { + panic(err) + } + }() + defer s.Stop() + + ctx := context.Background() + dial := func(context.Context, string) (net.Conn, error) { return l.Dial() } + conn, err := grpc.DialContext( + ctx, + "bufnet", + append([]grpc.DialOption{ + grpc.WithContextDialer(dial), + grpc.WithInsecure(), + }, cOpt...)..., + ) + if err != nil { + b.Fatalf("Failed to dial bufnet: %v", err) + } + defer conn.Close() + client := pb.NewTestServiceClient(conn) + + b.ReportAllocs() + b.ResetTimer() + + for n := 0; n < b.N; n++ { + interop.DoEmptyUnaryCall(client) + interop.DoLargeUnaryCall(client) + interop.DoClientStreaming(client) + interop.DoServerStreaming(client) + interop.DoPingPong(client) + interop.DoEmptyStream(client) + } + + b.StopTimer() +} + +func BenchmarkNoInstrumentation(b *testing.B) { + benchmark(b, nil, nil) +} + +func BenchmarkUnaryServerInterceptor(b *testing.B) { + benchmark(b, nil, []grpc.ServerOption{ + grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor(tracer)), + }) +} + +func BenchmarkStreamServerInterceptor(b *testing.B) { + benchmark(b, nil, []grpc.ServerOption{ + grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor(tracer)), + }) +} + +func BenchmarkUnaryClientInterceptor(b *testing.B) { + benchmark(b, []grpc.DialOption{ + grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(tracer)), + }, nil) +} + +func BenchmarkStreamClientInterceptor(b *testing.B) { + benchmark(b, []grpc.DialOption{ + grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor(tracer)), + }, nil) +} diff --git a/instrumentation/google.golang.org/grpc/example/go.sum b/instrumentation/google.golang.org/grpc/example/go.sum index 86ebb2e5057..c6c0f6b16be 100644 --- a/instrumentation/google.golang.org/grpc/example/go.sum +++ b/instrumentation/google.golang.org/grpc/example/go.sum @@ -1,3 +1,4 @@ +cloud.google.com/go v0.26.0 h1:e0WKqKTd5BnrG8aKH3J3h+QvEIQtSUcf2n5UZ5ZgLtQ= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= @@ -57,6 +58,7 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU= golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= +golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -75,6 +77,7 @@ golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= +google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 h1:gSJIx1SDwno+2ElGhA4+qG2zF97qiUzTM+rQ0klBOcE= diff --git a/instrumentation/google.golang.org/grpc/go.sum b/instrumentation/google.golang.org/grpc/go.sum index 66f01f70227..9ecc690d480 100644 --- a/instrumentation/google.golang.org/grpc/go.sum +++ b/instrumentation/google.golang.org/grpc/go.sum @@ -1,3 +1,4 @@ +cloud.google.com/go v0.26.0 h1:e0WKqKTd5BnrG8aKH3J3h+QvEIQtSUcf2n5UZ5ZgLtQ= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -44,6 +45,7 @@ golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -60,6 +62,7 @@ golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= +google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 h1:gSJIx1SDwno+2ElGhA4+qG2zF97qiUzTM+rQ0klBOcE= From 0afc4ae97a76e619c385d489548c25b690e8ddb1 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 26 Aug 2020 11:38:57 -0700 Subject: [PATCH 6/7] Add integration testing for gRPC instrumentation (#297) * Add integration testing for unary client * Update to test both client and server unary interceptors * Add testing for streaming gRPC * lint * Update CHANGELOG * Update Changelog with PR number * Remove duplicate testing const --- CHANGELOG.md | 1 + .../google.golang.org/grpc/grpc_test.go | 613 ++++++++++++++++++ 2 files changed, 614 insertions(+) create mode 100644 instrumentation/google.golang.org/grpc/grpc_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0290780f354..21e5a26ecb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Benchmark tests for the gRPC instrumentation. (#296) +- Integration testing for the gRPC instrumentation. (#297) ## [0.11.0] - 2020-08-25 diff --git a/instrumentation/google.golang.org/grpc/grpc_test.go b/instrumentation/google.golang.org/grpc/grpc_test.go new file mode 100644 index 00000000000..c8005713d90 --- /dev/null +++ b/instrumentation/google.golang.org/grpc/grpc_test.go @@ -0,0 +1,613 @@ +// Copyright The OpenTelemetry 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 grpc_test + +import ( + "context" + "net" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/interop" + pb "google.golang.org/grpc/interop/grpc_testing" + "google.golang.org/grpc/test/bufconn" + + otelgrpc "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc" + "go.opentelemetry.io/otel/api/trace" + "go.opentelemetry.io/otel/api/trace/tracetest" + "go.opentelemetry.io/otel/label" + "go.opentelemetry.io/otel/semconv" +) + +func doCalls(cOpt []grpc.DialOption, sOpt []grpc.ServerOption) error { + l := bufconn.Listen(bufSize) + defer l.Close() + + s := grpc.NewServer(sOpt...) + pb.RegisterTestServiceServer(s, interop.NewTestServer()) + go func() { + if err := s.Serve(l); err != nil { + panic(err) + } + }() + defer s.Stop() + + ctx := context.Background() + dial := func(context.Context, string) (net.Conn, error) { return l.Dial() } + conn, err := grpc.DialContext( + ctx, + "bufnet", + append([]grpc.DialOption{ + grpc.WithContextDialer(dial), + grpc.WithInsecure(), + }, cOpt...)..., + ) + if err != nil { + return err + } + defer conn.Close() + client := pb.NewTestServiceClient(conn) + + interop.DoEmptyUnaryCall(client) + interop.DoLargeUnaryCall(client) + interop.DoClientStreaming(client) + interop.DoServerStreaming(client) + interop.DoPingPong(client) + + return nil +} + +func TestInterceptors(t *testing.T) { + clientUnarySR := new(tracetest.StandardSpanRecorder) + clientUnaryTracer := tracetest.NewProvider(tracetest.WithSpanRecorder(clientUnarySR)).Tracer("TestUnaryClientInterceptor") + + clientStreamSR := new(tracetest.StandardSpanRecorder) + clientStreamTracer := tracetest.NewProvider(tracetest.WithSpanRecorder(clientStreamSR)).Tracer("TestStreamClientInterceptor") + + serverUnarySR := new(tracetest.StandardSpanRecorder) + serverUnaryTracer := tracetest.NewProvider(tracetest.WithSpanRecorder(serverUnarySR)).Tracer("TestUnaryServerInterceptor") + + serverStreamSR := new(tracetest.StandardSpanRecorder) + serverStreamTracer := tracetest.NewProvider(tracetest.WithSpanRecorder(serverStreamSR)).Tracer("TestStreamServerInterceptor") + + assert.NoError(t, doCalls( + []grpc.DialOption{ + grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(clientUnaryTracer)), + grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor(clientStreamTracer)), + }, + []grpc.ServerOption{ + grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor(serverUnaryTracer)), + grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor(serverStreamTracer)), + }, + )) + + t.Run("UnaryClientSpans", func(t *testing.T) { + checkUnaryClientSpans(t, clientUnaryTracer, clientUnarySR.Completed()) + }) + + t.Run("StreamClientSpans", func(t *testing.T) { + checkStreamClientSpans(t, clientStreamTracer, clientStreamSR.Completed()) + }) + + t.Run("UnaryServerSpans", func(t *testing.T) { + checkUnaryServerSpans(t, serverUnaryTracer, serverUnarySR.Completed()) + }) + + t.Run("StreamServerSpans", func(t *testing.T) { + checkStreamServerSpans(t, serverStreamTracer, serverStreamSR.Completed()) + }) +} + +func checkUnaryClientSpans(t *testing.T, tracer trace.Tracer, spans []*tracetest.Span) { + require.Len(t, spans, 2) + + emptySpan := spans[0] + assert.True(t, emptySpan.Ended()) + assert.Equal(t, tracer, emptySpan.Tracer()) + assert.Equal(t, "grpc.testing.TestService/EmptyCall", emptySpan.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(0), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(0), + }, + }, + }, noTimestamp(emptySpan.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("EmptyCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, emptySpan.Attributes()) + + largeSpan := spans[1] + assert.True(t, largeSpan.Ended()) + assert.Equal(t, tracer, largeSpan.Tracer()) + assert.Equal(t, "grpc.testing.TestService/UnaryCall", largeSpan.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + // largeReqSize from "google.golang.org/grpc/interop" + 12 (overhead). + semconv.RPCMessageUncompressedSizeKey: label.IntValue(271840), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + // largeRespSize from "google.golang.org/grpc/interop" + 8 (overhead). + semconv.RPCMessageUncompressedSizeKey: label.IntValue(314167), + }, + }, + }, noTimestamp(largeSpan.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("UnaryCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, largeSpan.Attributes()) +} + +func checkStreamClientSpans(t *testing.T, tracer trace.Tracer, spans []*tracetest.Span) { + require.Len(t, spans, 3) + + streamInput := spans[0] + assert.True(t, streamInput.Ended()) + assert.Equal(t, tracer, streamInput.Tracer()) + assert.Equal(t, "grpc.testing.TestService/StreamingInputCall", streamInput.Name()) + // sizes from reqSizes in "google.golang.org/grpc/interop". + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(27190), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(12), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(1834), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(45912), + }, + }, + // client does not record an event for the server response. + }, noTimestamp(streamInput.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("StreamingInputCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, streamInput.Attributes()) + + streamOutput := spans[1] + assert.True(t, streamOutput.Ended()) + assert.Equal(t, tracer, streamOutput.Tracer()) + assert.Equal(t, "grpc.testing.TestService/StreamingOutputCall", streamOutput.Name()) + // sizes from respSizes in "google.golang.org/grpc/interop". + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(21), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(31423), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(13), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(2659), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(58987), + }, + }, + }, noTimestamp(streamOutput.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("StreamingOutputCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, streamOutput.Attributes()) + + pingPong := spans[2] + assert.True(t, pingPong.Ended()) + assert.Equal(t, tracer, pingPong.Tracer()) + assert.Equal(t, "grpc.testing.TestService/FullDuplexCall", pingPong.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(27196), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(31423), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(16), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(13), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(1839), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(2659), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(45918), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(58987), + }, + }, + }, noTimestamp(pingPong.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("FullDuplexCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, pingPong.Attributes()) +} + +func checkStreamServerSpans(t *testing.T, tracer trace.Tracer, spans []*tracetest.Span) { + require.Len(t, spans, 3) + + streamInput := spans[0] + assert.True(t, streamInput.Ended()) + assert.Equal(t, tracer, streamInput.Tracer()) + assert.Equal(t, "grpc.testing.TestService/StreamingInputCall", streamInput.Name()) + // sizes from reqSizes in "google.golang.org/grpc/interop". + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(27190), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(12), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(1834), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(45912), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(4), + }, + }, + }, noTimestamp(streamInput.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("StreamingInputCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, streamInput.Attributes()) + + streamOutput := spans[1] + assert.True(t, streamOutput.Ended()) + assert.Equal(t, tracer, streamOutput.Tracer()) + assert.Equal(t, "grpc.testing.TestService/StreamingOutputCall", streamOutput.Name()) + // sizes from respSizes in "google.golang.org/grpc/interop". + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(21), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(31423), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(13), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(2659), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(58987), + }, + }, + }, noTimestamp(streamOutput.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("StreamingOutputCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, streamOutput.Attributes()) + + pingPong := spans[2] + assert.True(t, pingPong.Ended()) + assert.Equal(t, tracer, pingPong.Tracer()) + assert.Equal(t, "grpc.testing.TestService/FullDuplexCall", pingPong.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(27196), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(31423), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(16), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(2), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(13), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(1839), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(3), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(2659), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(45918), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(4), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(58987), + }, + }, + }, noTimestamp(pingPong.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("FullDuplexCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, pingPong.Attributes()) +} + +func checkUnaryServerSpans(t *testing.T, tracer trace.Tracer, spans []*tracetest.Span) { + require.Len(t, spans, 2) + + emptySpan := spans[0] + assert.True(t, emptySpan.Ended()) + assert.Equal(t, tracer, emptySpan.Tracer()) + assert.Equal(t, "grpc.testing.TestService/EmptyCall", emptySpan.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(0), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + semconv.RPCMessageUncompressedSizeKey: label.IntValue(0), + }, + }, + }, noTimestamp(emptySpan.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("EmptyCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, emptySpan.Attributes()) + + largeSpan := spans[1] + assert.True(t, largeSpan.Ended()) + assert.Equal(t, tracer, largeSpan.Tracer()) + assert.Equal(t, "grpc.testing.TestService/UnaryCall", largeSpan.Name()) + assert.Equal(t, []tracetest.Event{ + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("RECEIVED"), + // largeReqSize from "google.golang.org/grpc/interop" + 12 (overhead). + semconv.RPCMessageUncompressedSizeKey: label.IntValue(271840), + }, + }, + { + Name: "message", + Attributes: map[label.Key]label.Value{ + semconv.RPCMessageIDKey: label.IntValue(1), + semconv.RPCMessageTypeKey: label.StringValue("SENT"), + // largeRespSize from "google.golang.org/grpc/interop" + 8 (overhead). + semconv.RPCMessageUncompressedSizeKey: label.IntValue(314167), + }, + }, + }, noTimestamp(largeSpan.Events())) + assert.Equal(t, map[label.Key]label.Value{ + semconv.RPCMethodKey: label.StringValue("UnaryCall"), + semconv.RPCServiceKey: label.StringValue("grpc.testing.TestService"), + semconv.RPCSystemGRPC.Key: semconv.RPCSystemGRPC.Value, + }, largeSpan.Attributes()) +} + +func noTimestamp(events []tracetest.Event) []tracetest.Event { + out := make([]tracetest.Event, 0, len(events)) + for _, e := range events { + out = append(out, tracetest.Event{ + Name: e.Name, + Attributes: e.Attributes, + }) + } + return out +} From 1f7546cabd597f84c076529103f5deeec40521d6 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 26 Aug 2020 16:35:31 -0700 Subject: [PATCH 7/7] [Prometheus Remote Write Exporter for Cortex] Add TLS Support and Default HTTP Client (#255) * Start buildClient for creating a default HTTP client * Add TestBuildClient and first subtest * Update TestBuildClient with TLS server, TLS Config, and Proxy URL * Add methods for loading user-supplied certificates * Add buildTLSConfig for creating a new TLS Config struct * Add TLS Config and Proxy URL to buildClient, and update TestBuildClient * Add additional tests for TestBuildClient * Add helper function for generating certificate files * Add helper function for generating CA certificate files * Add helper function for generating serving certificate files * Add helper function for generating client certificate files * Add part of integration test with certificate creation and TLS server * Add helper function for creating the test server's TLS Config struct * Update TestMutualTLS by adding TLS Config to server and client * Run make precommit and fix lint errors * Adjust test for BuildClient * Change certificate loading functions into inline conditionals * Change ProxyURL to be a url.URL instead of a string * Add check for InsecureSkipVerify to avoid parse errors * Change client Transport to use http.DefaultTransport as base * Change require.Nil to require.NoError for error checks * Change require.Error to assert.Error in some areas * Write certificate and key files directly instead of to memory first * Update DialContext timeout and KeepAlive for retrying CI test * Revert increase to DialContext timeout and keepalive to retry CI test --- exporters/metric/cortex/auth.go | 91 +++++ exporters/metric/cortex/auth_test.go | 383 +++++++++++++++++- exporters/metric/cortex/config.go | 3 +- exporters/metric/cortex/cortex.go | 7 +- exporters/metric/cortex/cortex_test.go | 18 +- .../cortex/utils/config_utils_data_test.go | 2 +- .../metric/cortex/utils/config_utils_test.go | 8 +- 7 files changed, 490 insertions(+), 22 deletions(-) diff --git a/exporters/metric/cortex/auth.go b/exporters/metric/cortex/auth.go index b6de32c61f6..f351ae9817e 100644 --- a/exporters/metric/cortex/auth.go +++ b/exporters/metric/cortex/auth.go @@ -15,9 +15,14 @@ package cortex import ( + "crypto/tls" + "crypto/x509" "fmt" "io/ioutil" + "net" "net/http" + "strconv" + "time" ) // ErrFailedToReadFile occurs when a password / bearer token file exists, but could @@ -88,3 +93,89 @@ func (e *Exporter) addBearerTokenAuth(req *http.Request) error { return nil } + +// buildClient returns a http client that uses TLS and has the user-specified proxy and +// timeout. +func (e *Exporter) buildClient() (*http.Client, error) { + // Create a TLS Config struct for use in a custom HTTP Transport. + tlsConfig, err := e.buildTLSConfig() + if err != nil { + return nil, err + } + + // Create a custom HTTP Transport for the client. This is the same as + // http.DefaultTransport other than the TLSClientConfig. + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: tlsConfig, + } + + // Convert proxy url to proxy function for use in the created Transport. + if e.config.ProxyURL != nil { + proxy := http.ProxyURL(e.config.ProxyURL) + transport.Proxy = proxy + } + + client := http.Client{ + Transport: transport, + Timeout: e.config.RemoteTimeout, + } + return &client, nil +} + +// buildTLSConfig creates a new TLS Config struct with the properties from the exporter's +// Config struct. +func (e *Exporter) buildTLSConfig() (*tls.Config, error) { + tlsConfig := &tls.Config{} + if e.config.TLSConfig == nil { + return tlsConfig, nil + } + + // Set the server name if it exists. + if e.config.TLSConfig["server_name"] != "" { + tlsConfig.ServerName = e.config.TLSConfig["server_name"] + } + + // Set InsecureSkipVerify. Viper reads the bool as a string since it is in a map. + if isv, ok := e.config.TLSConfig["insecure_skip_verify"]; ok { + var err error + if tlsConfig.InsecureSkipVerify, err = strconv.ParseBool(isv); err != nil { + return nil, err + } + } + + // Load certificates from CA file if it exists. + caFile := e.config.TLSConfig["ca_file"] + if caFile != "" { + caFileData, err := ioutil.ReadFile(caFile) + if err != nil { + return nil, err + } + certPool := x509.NewCertPool() + certPool.AppendCertsFromPEM(caFileData) + tlsConfig.RootCAs = certPool + } + + // Load the client certificate if it exists. + certFile := e.config.TLSConfig["cert_file"] + keyFile := e.config.TLSConfig["key_file"] + if certFile != "" && keyFile != "" { + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, err + } + tlsConfig.Certificates = []tls.Certificate{cert} + } + + return tlsConfig, nil +} diff --git a/exporters/metric/cortex/auth_test.go b/exporters/metric/cortex/auth_test.go index 61aa57dfa68..a0283c83385 100644 --- a/exporters/metric/cortex/auth_test.go +++ b/exporters/metric/cortex/auth_test.go @@ -15,13 +15,26 @@ package cortex import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" "encoding/base64" + "encoding/pem" + "fmt" "io/ioutil" + "math/big" + "net" "net/http" "net/http/httptest" + "net/url" "os" + "strings" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -98,7 +111,7 @@ func TestAuthentication(t *testing.T) { handler := func(rw http.ResponseWriter, req *http.Request) { authHeaderValue := req.Header.Get("Authorization") _, err := rw.Write([]byte(authHeaderValue)) - require.Nil(t, err) + require.NoError(t, err) } server := httptest.NewServer(http.HandlerFunc(handler)) defer server.Close() @@ -109,14 +122,14 @@ func TestAuthentication(t *testing.T) { if passwordFile != "" && test.basicAuthPasswordFileContents != nil { filepath := "./" + test.basicAuth["password_file"] err := createFile(test.basicAuthPasswordFileContents, filepath) - require.Nil(t, err) + require.NoError(t, err) defer os.Remove(filepath) } } if test.bearerTokenFile != "" && test.bearerTokenFileContents != nil { filepath := "./" + test.bearerTokenFile err := createFile(test.bearerTokenFileContents, filepath) - require.Nil(t, err) + require.NoError(t, err) defer os.Remove(filepath) } @@ -130,14 +143,14 @@ func TestAuthentication(t *testing.T) { }, } req, err := http.NewRequest(http.MethodPost, server.URL, nil) - require.Nil(t, err) + require.NoError(t, err) err = exporter.addHeaders(req) // Verify the error and if the Authorization header was correctly set. if err != nil { require.Equal(t, err.Error(), test.expectedError.Error()) } else { - require.Nil(t, test.expectedError) + require.NoError(t, test.expectedError) authHeaderValue := req.Header.Get("Authorization") require.Equal(t, authHeaderValue, test.expectedAuthHeaderValue) } @@ -153,3 +166,363 @@ func createFile(bytes []byte, filepath string) error { } return nil } + +// TestBuildClient checks whether the buildClient successfully creates a client that can +// connect over TLS and has the correct remote timeout and proxy url. +func TestBuildClient(t *testing.T) { + testProxyURL, err := url.Parse("123.4.5.6") + require.NoError(t, err) + + tests := []struct { + testName string + config Config + expectedRemoteTimeout time.Duration + expectedErrorSubstring string + }{ + { + testName: "Remote Timeout with Proxy URL", + config: Config{ + ProxyURL: testProxyURL, + RemoteTimeout: 123 * time.Second, + TLSConfig: map[string]string{ + "ca_file": "./ca_cert.pem", + "insecure_skip_verify": "0", + }, + }, + expectedRemoteTimeout: 123 * time.Second, + expectedErrorSubstring: "proxyconnect tcp", + }, + { + testName: "No Timeout or Proxy URL, InsecureSkipVerify is false", + config: Config{ + TLSConfig: map[string]string{ + "ca_file": "./ca_cert.pem", + "insecure_skip_verify": "0", + }, + }, + expectedErrorSubstring: "", + }, + { + testName: "No Timeout or Proxy URL, InsecureSkipVerify is true", + config: Config{ + TLSConfig: map[string]string{ + "ca_file": "./ca_cert.pem", + "insecure_skip_verify": "1", + }, + }, + expectedErrorSubstring: "", + }, + } + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + // Create and start the TLS server. + handler := func(rw http.ResponseWriter, req *http.Request) { + fmt.Fprint(rw, "Successfully received HTTP request!") + } + server := httptest.NewTLSServer(http.HandlerFunc(handler)) + defer server.Close() + + // Create a certicate for the CA from the TLS server. This will be used to + // verify the test server by the client. + encodedCACert := server.TLS.Certificates[0].Certificate[0] + caCertPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: encodedCACert, + }) + err := createFile(caCertPEM, "./ca_cert.pem") + require.NoError(t, err) + defer os.Remove("ca_cert.pem") + + // Create an Exporter client and check the timeout. + exporter := Exporter{ + config: test.config, + } + client, err := exporter.buildClient() + require.NoError(t, err) + assert.Equal(t, client.Timeout, test.expectedRemoteTimeout) + + // Attempt to send the request and verify that the correct error occurred. If + // an error is expected, the test checks if the error string contains the + // expected error substring since the error can contain the server URL, which + // changes every test. + _, err = client.Get(server.URL) + if test.expectedErrorSubstring != "" { + if assert.Error(t, err) { + hasErrorSubstring := strings.Contains(err.Error(), test.expectedErrorSubstring) + assert.True(t, hasErrorSubstring, "missing error message") + } + } else { + require.NoError(t, err) + } + }) + } +} + +// TestMutualTLS is an integration test that checks whether the Exporter's client can +// successfully verify a server and send a HTTP request and whether a server can +// successfully verify the Exporter client and receive the HTTP request. +func TestMutualTLS(t *testing.T) { + // Generate certificate authority certificate to sign other certificates. + caCert, caPrivateKey, err := generateCACertFiles("./ca_cert.pem", "./ca_key.pem") + require.NoError(t, err) + defer os.Remove("./ca_cert.pem") + defer os.Remove("./ca_key.pem") + + // Generate certificate for the server. The client will check this certificate against + // its certificate authority to verify the server. + _, _, err = generateServingCertFiles( + caCert, + caPrivateKey, + "./serving_cert.pem", + "./serving_key.pem", + ) + require.NoError(t, err) + defer os.Remove("./serving_cert.pem") + defer os.Remove("./serving_key.pem") + + // Generate certificate for the client. The server will check this certificate against + // its certificate authority to verify the client. + _, _, err = generateClientCertFiles( + caCert, + caPrivateKey, + "./client_cert.pem", + "./client_key.pem", + ) + require.NoError(t, err) + defer os.Remove("./client_cert.pem") + defer os.Remove("./client_key.pem") + + // Generate the tls Config to set up mutual TLS on the server. + serverTLSConfig, err := generateServerTLSConfig( + "ca_cert.pem", + "serving_cert.pem", + "serving_key.pem", + ) + require.NoError(t, err) + + // Create and start the TLS server. + handler := func(rw http.ResponseWriter, req *http.Request) { + fmt.Fprint(rw, "Successfully verified client and received request!") + } + server := httptest.NewUnstartedServer(http.HandlerFunc(handler)) + server.TLS = serverTLSConfig + server.StartTLS() + defer server.Close() + + // Create an Exporter client with the client and CA certificate files. + exporter := Exporter{ + Config{ + TLSConfig: map[string]string{ + "ca_file": "./ca_cert.pem", + "cert_file": "./client_cert.pem", + "key_file": "./client_key.pem", + "insecure_skip_verify": "0", + }, + }, + } + client, err := exporter.buildClient() + require.NoError(t, err) + + // Send the request and verify that the request was successfully received. + res, err := client.Get(server.URL) + require.NoError(t, err) + defer res.Body.Close() +} + +// generateCertFiles generates new certificate files from a template that is signed with +// the provided signer certificate and key. +func generateCertFiles( + template *x509.Certificate, + signer *x509.Certificate, + signerKey *rsa.PrivateKey, + certFilepath string, + keyFilepath string, +) (*x509.Certificate, *rsa.PrivateKey, error) { + // Generate a private key for the new certificate. This does not have to be rsa 4096. + privateKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return nil, nil, err + } + + // Check if a signer key was provided. If not, then use the newly created key. + if signerKey == nil { + signerKey = privateKey + } + + // Create a new certificate in DER encoding. + encodedCert, err := x509.CreateCertificate( + rand.Reader, template, signer, privateKey.Public(), signerKey, + ) + if err != nil { + return nil, nil, err + } + + // Write the certificate to the local directory. + certFile, err := os.Create(certFilepath) + if err != nil { + return nil, nil, err + } + err = pem.Encode(certFile, &pem.Block{ + Type: "CERTIFICATE", + Bytes: encodedCert, + }) + if err != nil { + return nil, nil, err + } + + // Write the private key to the local directory. + encodedPrivateKey, err := x509.MarshalPKCS8PrivateKey(privateKey) + if err != nil { + return nil, nil, err + } + keyFile, err := os.Create(keyFilepath) + if err != nil { + return nil, nil, err + } + err = pem.Encode(keyFile, &pem.Block{ + Type: "PRIVATE KEY", + Bytes: encodedPrivateKey, + }) + if err != nil { + return nil, nil, err + } + + // Parse the newly created certificate so it can be returned. + cert, err := x509.ParseCertificate(encodedCert) + if err != nil { + return nil, nil, err + } + return cert, privateKey, nil +} + +// generateCACertFiles creates a CA certificate and key in the local directory. This +// certificate is used to sign other certificates. +func generateCACertFiles(certFilepath string, keyFilepath string) (*x509.Certificate, *rsa.PrivateKey, error) { + // Create a template for CA certificates. + certTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(123), + Subject: pkix.Name{ + Organization: []string{"CA Certificate"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(5 * time.Minute), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign, + IsCA: true, + BasicConstraintsValid: true, + } + + // Create the certificate files. CA certificates are used to sign other certificates + // so it signs itself with its own template and private key during creation. + cert, privateKey, err := generateCertFiles( + certTemplate, + certTemplate, + nil, + certFilepath, + keyFilepath, + ) + if err != nil { + return nil, nil, err + } + + return cert, privateKey, nil +} + +// generateServingCertFiles creates a new certificate that a client will check against its +// certificate authority to verify the server. The certificate is signed by a certificate +// authority. +func generateServingCertFiles( + caCert *x509.Certificate, + caPrivateKey *rsa.PrivateKey, + certFilepath string, + keyFilepath string, +) (*x509.Certificate, *rsa.PrivateKey, error) { + certTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(456), + Subject: pkix.Name{ + Organization: []string{"Serving Certificate"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(5 * time.Minute), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + } + + // Create the certificate files. The CA certificate is used to sign the new + // certificate. + cert, privateKey, err := generateCertFiles( + certTemplate, + caCert, + caPrivateKey, + "./serving_cert.pem", + "./serving_key.pem", + ) + if err != nil { + return nil, nil, err + } + + return cert, privateKey, nil +} + +// generateClientCertFiles creates a new certificate that a server will check against its +// certificate authority to verify the client. The certificate is signed by a certificate +// authority. +func generateClientCertFiles( + caCert *x509.Certificate, + caPrivateKey *rsa.PrivateKey, + certFilepath string, + keyFilepath string, +) (*x509.Certificate, *rsa.PrivateKey, error) { + certTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(789), + Subject: pkix.Name{ + Organization: []string{"Client Certificate"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(5 * time.Minute), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + } + + // Create the certificate files. The CA certificate is used to sign the new + // certificate. + cert, privateKey, err := generateCertFiles( + certTemplate, + caCert, + caPrivateKey, + "./client_cert.pem", + "./client_key.pem", + ) + if err != nil { + return nil, nil, err + } + + return cert, privateKey, nil +} + +// generateServerTLSConfig creates a tls Config struct for a server that wants to both +// verify servers and have clients verify itself. +func generateServerTLSConfig(caCertFile string, servingCertFile string, servingKeyFile string) (*tls.Config, error) { + // Create the server's serving certificate. This allows clients to verify the server. + servingCert, err := tls.LoadX509KeyPair(servingCertFile, servingKeyFile) + if err != nil { + return nil, err + } + + // Create a certificate pool to store the CA certificate. This allows the server to + // verify client certificates signed by the stored certicate authority. + encodedCACert, err := ioutil.ReadFile(caCertFile) + if err != nil { + return nil, err + } + certPool := x509.NewCertPool() + certPool.AppendCertsFromPEM(encodedCACert) + + // Create the tls Config struct and set it to always verify the client with the CAs. + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{servingCert}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: certPool, + } + return tlsConfig, nil +} diff --git a/exporters/metric/cortex/config.go b/exporters/metric/cortex/config.go index 911be193b6d..51fa2851b41 100644 --- a/exporters/metric/cortex/config.go +++ b/exporters/metric/cortex/config.go @@ -17,6 +17,7 @@ package cortex import ( "fmt" "net/http" + "net/url" "time" ) @@ -51,7 +52,7 @@ type Config struct { BearerToken string `mapstructure:"bearer_token"` BearerTokenFile string `mapstructure:"bearer_token_file"` TLSConfig map[string]string `mapstructure:"tls_config"` - ProxyURL string `mapstructure:"proxy_url"` + ProxyURL *url.URL `mapstructure:"proxy_url"` PushInterval time.Duration `mapstructure:"push_interval"` Quantiles []float64 `mapstructure:"quantiles"` HistogramBoundaries []float64 `mapstructure:"histogram_boundaries"` diff --git a/exporters/metric/cortex/cortex.go b/exporters/metric/cortex/cortex.go index 9b7ff363314..2311b9f7b31 100644 --- a/exporters/metric/cortex/cortex.go +++ b/exporters/metric/cortex/cortex.go @@ -468,8 +468,11 @@ func (e *Exporter) buildRequest(message []byte) (*http.Request, error) { func (e *Exporter) sendRequest(req *http.Request) error { // Set a client if the user didn't provide one. if e.config.Client == nil { - e.config.Client = http.DefaultClient - e.config.Client.Timeout = e.config.RemoteTimeout + client, err := e.buildClient() + if err != nil { + return err + } + e.config.Client = client } // Attempt to send request. diff --git a/exporters/metric/cortex/cortex_test.go b/exporters/metric/cortex/cortex_test.go index bc5cc52d5d6..0d4ca1a9c1a 100644 --- a/exporters/metric/cortex/cortex_test.go +++ b/exporters/metric/cortex/cortex_test.go @@ -56,7 +56,7 @@ var validConfig = Config{ "server_name": "server", "insecure_skip_verify": "1", }, - ProxyURL: "", + ProxyURL: nil, PushInterval: 10 * time.Second, Headers: map[string]string{ "x-prometheus-remote-write-version": "0.1.0", @@ -190,9 +190,9 @@ func TestAddHeaders(t *testing.T) { // Create http request to add headers to. req, err := http.NewRequest("POST", "test.com", nil) - require.Nil(t, err) + require.NoError(t, err) err = exporter.addHeaders(req) - require.Nil(t, err) + require.NoError(t, err) // Check that all the headers are there. for name, field := range testConfig.Headers { @@ -213,7 +213,7 @@ func TestBuildMessage(t *testing.T) { // package has its own tests, buildMessage should work as expected as long as there // are no errors. _, err := exporter.buildMessage(timeseries) - require.Nil(t, err) + require.NoError(t, err) } // TestBuildRequest tests whether a http request is a POST request, has the correct body, @@ -225,14 +225,14 @@ func TestBuildRequest(t *testing.T) { // Create the http request. req, err := exporter.buildRequest(testMessage) - require.Nil(t, err) + require.NoError(t, err) // Verify the http method, url, and body. require.Equal(t, req.Method, http.MethodPost) require.Equal(t, req.URL.String(), validConfig.Endpoint) reqMessage, err := ioutil.ReadAll(req.Body) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, reqMessage, testMessage) // Verify headers. @@ -329,11 +329,11 @@ func TestSendRequest(t *testing.T) { // Create an empty Snappy-compressed message. msg, err := exporter.buildMessage([]*prompb.TimeSeries{}) - require.Nil(t, err) + require.NoError(t, err) // Create a http POST request with the compressed message. req, err := exporter.buildRequest(msg) - require.Nil(t, err) + require.NoError(t, err) // Send the request to the test server and verify the error. err = exporter.sendRequest(req) @@ -341,7 +341,7 @@ func TestSendRequest(t *testing.T) { errorString := err.Error() require.Equal(t, errorString, test.expectedError.Error()) } else { - require.Nil(t, test.expectedError) + require.NoError(t, test.expectedError) } }) } diff --git a/exporters/metric/cortex/utils/config_utils_data_test.go b/exporters/metric/cortex/utils/config_utils_data_test.go index 3a968278bb8..fa0d38cef69 100644 --- a/exporters/metric/cortex/utils/config_utils_data_test.go +++ b/exporters/metric/cortex/utils/config_utils_data_test.go @@ -126,7 +126,7 @@ var validConfig = cortex.Config{ "server_name": "server", "insecure_skip_verify": "1", }, - ProxyURL: "", + ProxyURL: nil, PushInterval: 5 * time.Second, Headers: map[string]string{ "test": "header", diff --git a/exporters/metric/cortex/utils/config_utils_test.go b/exporters/metric/cortex/utils/config_utils_test.go index ee8da05530f..72d99bfed21 100644 --- a/exporters/metric/cortex/utils/config_utils_test.go +++ b/exporters/metric/cortex/utils/config_utils_test.go @@ -105,7 +105,7 @@ func TestNewConfig(t *testing.T) { // Create YAML file. fullPath := test.directoryPath + "/" + test.fileName fs, err := initYAML(test.yamlByteString, fullPath) - require.Nil(t, err) + require.NoError(t, err) // Create new Config struct from the specified YAML file with an in-memory // filesystem. @@ -153,7 +153,7 @@ func TestWithFilepath(t *testing.T) { // Create YAML file. fullPath := test.directoryPath + "/" + test.fileName fs, err := initYAML(test.yamlByteString, fullPath) - require.Nil(t, err) + require.NoError(t, err) // Create new Config struct from the specified YAML file with an in-memory // filesystem. If a path is added, Viper should be able to find the file and @@ -165,7 +165,7 @@ func TestWithFilepath(t *testing.T) { utils.WithFilepath(test.directoryPath), utils.WithFilesystem(fs), ) - require.Nil(t, err) + require.NoError(t, err) } else { _, err := utils.NewConfig(test.fileName, utils.WithFilesystem(fs)) require.Error(t, err) @@ -179,7 +179,7 @@ func TestWithFilepath(t *testing.T) { func TestWithClient(t *testing.T) { // Create a YAML file. fs, err := initYAML(validYAML, "/test/config.yml") - require.Nil(t, err) + require.NoError(t, err) // Create a new Config struct with a custom HTTP client. customClient := &http.Client{