Skip to content

Commit

Permalink
Merge pull request #204 from zeitlinger/opentracing
Browse files Browse the repository at this point in the history
fix nil pointer when context is nil
  • Loading branch information
casualjim authored Aug 4, 2021
2 parents a98b067 + b80cc08 commit 60c8ebd
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
args: --new-from-rev=master
only-new-issues: true
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 60c8ebd

Please sign in to comment.