Skip to content

Commit

Permalink
fix nil pointer when context is nil
Browse files Browse the repository at this point in the history
use Params instead of AuthInfo, because that is always present
respect the operationID from swagger (don't snake_case it)

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@zalando.de>
  • Loading branch information
zeitlinger committed Jun 28, 2021
1 parent 539cabc commit 51e3c89
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 23 deletions.
30 changes: 14 additions & 16 deletions client/opentracing.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package client

import (
"fmt"
"net/http"
"regexp"
"strings"

"github.com/go-openapi/strfmt"
"github.com/opentracing/opentracing-go"
Expand All @@ -29,7 +28,11 @@ func newOpenTracingTransport(transport runtime.ClientTransport, host string, opt
}

func (t *tracingTransport) Submit(op *runtime.ClientOperation) (interface{}, error) {
authInfo := op.AuthInfo
if op.Context == nil {
return t.transport.Submit(op)
}

params := op.Params
reader := op.Reader

var span opentracing.Span
Expand All @@ -39,12 +42,9 @@ func (t *tracingTransport) Submit(op *runtime.ClientOperation) (interface{}, err
}
}()

op.AuthInfo = runtime.ClientAuthInfoWriterFunc(func(req runtime.ClientRequest, reg strfmt.Registry) error {
op.Params = runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, reg strfmt.Registry) error {
span = createClientSpan(op, req.GetHeaderParams(), t.host, t.opts)
if authInfo == nil {
return nil
}
return authInfo.AuthenticateRequest(req, reg)
return params.WriteToRequest(req, reg)
})

op.Reader = runtime.ClientResponseReaderFunc(func(response runtime.ClientResponse, consumer runtime.Consumer) (interface{}, error) {
Expand Down Expand Up @@ -74,7 +74,7 @@ func createClientSpan(op *runtime.ClientOperation, header http.Header, host stri
if span != nil {
opts = append(opts, ext.SpanKindRPCClient)
span, _ = opentracing.StartSpanFromContextWithTracer(
ctx, span.Tracer(), toSnakeCase(op.ID), opts...)
ctx, span.Tracer(), operationName(op), opts...)

ext.Component.Set(span, "go-openapi")
ext.PeerHostname.Set(span, host)
Expand All @@ -91,11 +91,9 @@ func createClientSpan(op *runtime.ClientOperation, header http.Header, host stri
return nil
}

var matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)")
var matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])")

func toSnakeCase(str string) string {
snake := matchFirstCap.ReplaceAllString(str, "${1}_${2}")
snake = matchAllCap.ReplaceAllString(snake, "${1}_${2}")
return strings.ToLower(snake)
func operationName(op *runtime.ClientOperation) string {
if op.ID != "" {
return op.ID
}
return fmt.Sprintf("%s_%s", op.Method, op.PathPattern)
}
29 changes: 22 additions & 7 deletions client/opentracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/ioutil"
"testing"

"github.com/go-openapi/strfmt"
"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/mocktracer"
Expand Down Expand Up @@ -37,7 +38,7 @@ type mockRuntime struct {
}

func (m *mockRuntime) Submit(operation *runtime.ClientOperation) (interface{}, error) {
_ = operation.AuthInfo.AuthenticateRequest(&m.req, nil)
_ = operation.Params.WriteToRequest(&m.req, nil)
_, _ = operation.Reader.ReadResponse(&tres{}, nil)
return nil, nil
}
Expand All @@ -53,6 +54,9 @@ func testOperation(ctx context.Context) *runtime.ClientOperation {
Reader: runtime.ClientResponseReaderFunc(func(runtime.ClientResponse, runtime.Consumer) (interface{}, error) {
return nil, nil
}),
Params: runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, reg strfmt.Registry) error {
return nil
}),
AuthInfo: PassThroughAuth,
Context: ctx,
}
Expand All @@ -62,19 +66,28 @@ func Test_TracingRuntime_submit(t *testing.T) {
t.Parallel()
tracer := mocktracer.New()
_, ctx := opentracing.StartSpanFromContextWithTracer(context.Background(), tracer, "op")
testSubmit(t, testOperation(ctx), tracer)
testSubmit(t, testOperation(ctx), tracer, 1)
}

func Test_TracingRuntime_submit_nullAuthInfo(t *testing.T) {
func Test_TracingRuntime_submit_nilAuthInfo(t *testing.T) {
t.Parallel()
tracer := mocktracer.New()
_, ctx := opentracing.StartSpanFromContextWithTracer(context.Background(), tracer, "op")
operation := testOperation(ctx)
operation.AuthInfo = nil
testSubmit(t, operation, tracer)
testSubmit(t, operation, tracer, 1)
}

func Test_TracingRuntime_submit_nilContext(t *testing.T) {
t.Parallel()
tracer := mocktracer.New()
_, ctx := opentracing.StartSpanFromContextWithTracer(context.Background(), tracer, "op")
operation := testOperation(ctx)
operation.Context = nil
testSubmit(t, operation, tracer, 0) // just don't panic
}

func testSubmit(t *testing.T, operation *runtime.ClientOperation, tracer *mocktracer.MockTracer) {
func testSubmit(t *testing.T, operation *runtime.ClientOperation, tracer *mocktracer.MockTracer, expectedSpans int) {

header := map[string][]string{}
r := newOpenTracingTransport(&mockRuntime{runtime.TestClientRequest{Headers: header}},
Expand All @@ -87,9 +100,11 @@ func testSubmit(t *testing.T, operation *runtime.ClientOperation, tracer *mocktr
_, err := r.Submit(operation)
require.NoError(t, err)

if assert.Len(t, tracer.FinishedSpans(), 1) {
assert.Len(t, tracer.FinishedSpans(), expectedSpans)

if expectedSpans == 1 {
span := tracer.FinishedSpans()[0]
assert.Equal(t, "get_cluster", span.OperationName)
assert.Equal(t, "getCluster", span.OperationName)
assert.Equal(t, map[string]interface{}{
"component": "go-openapi",
"http.method": "GET",
Expand Down

0 comments on commit 51e3c89

Please sign in to comment.