Skip to content

Commit

Permalink
Removed all direct mocktracer.New() calls and explicit waits for fini…
Browse files Browse the repository at this point in the history
…shed spans

Signed-off-by: Roman Zavodskikh <[email protected]>
  • Loading branch information
Roman Zavodskikh committed Nov 26, 2024
1 parent adf38fc commit 4738392
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 62 deletions.
8 changes: 3 additions & 5 deletions filters/auth/jwt_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import (
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/opentracing/opentracing-go/mocktracer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/zalando/skipper/eskip"
Expand All @@ -20,6 +18,7 @@ import (
"github.com/zalando/skipper/proxy"
"github.com/zalando/skipper/proxy/proxytest"
"github.com/zalando/skipper/routing"
"github.com/zalando/skipper/tracing/tracingtest"
)

func TestJwtMetrics(t *testing.T) {
Expand Down Expand Up @@ -275,7 +274,7 @@ func TestJwtMetrics(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
m := &metricstest.MockMetrics{}
defer m.Close()
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()

fr := builtin.MakeRegistry()
fr.Register(auth.NewJwtMetrics())
Expand Down Expand Up @@ -304,8 +303,7 @@ func TestJwtMetrics(t *testing.T) {
resp.Body.Close()
require.Equal(t, tc.status, resp.StatusCode)

// wait for the span to be finished
require.Eventually(t, func() bool { return len(tracer.FinishedSpans()) == 1 }, time.Second, 100*time.Millisecond)
require.Equal(t, 1, len(tracer.FinishedSpans()))

span := tracer.FinishedSpans()[0]
if tc.expectedTag == "" {
Expand Down
2 changes: 1 addition & 1 deletion filters/scheduler/lifo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ func TestLifoErrors(t *testing.T) {

<-rt.FirstLoad()

tracer := tracingtest.NewMockTracer()
tracer := tracingtest.NewTracer()
pr := proxy.WithParams(proxy.Params{
Routing: rt,
OpenTracing: &proxy.OpenTracingParams{Tracer: tracer},
Expand Down
3 changes: 2 additions & 1 deletion filters/tracing/tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/zalando/skipper/eskip"
"github.com/zalando/skipper/filters"
"github.com/zalando/skipper/filters/filtertest"
"github.com/zalando/skipper/tracing/tracingtest"
)

func TestTracingTagNil(t *testing.T) {
Expand Down Expand Up @@ -196,7 +197,7 @@ func TestTagCreateFilter(t *testing.T) {
}

func TestTracingTag(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()

for _, ti := range []struct {
name string
Expand Down
30 changes: 4 additions & 26 deletions net/httpclient_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,12 @@ import (
"github.com/lightstep/lightstep-tracer-go"
"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/mocktracer"
"github.com/sirupsen/logrus"
"github.com/zalando/skipper/net"
"github.com/zalando/skipper/secrets"
"github.com/zalando/skipper/tracing/tracingtest"
)

func waitForSpanViaMockTracer(mockTracer *mocktracer.MockTracer) {
for i := 0; i < 20; i++ {
if n := len(mockTracer.FinishedSpans()); n > 0 {
logrus.Printf("found %d spans", n)
return
}
time.Sleep(100 * time.Millisecond)
}
logrus.Println("no span found")
}

func ExampleTransport() {
tracer := lightstep.NewTracer(lightstep.Options{})

Expand Down Expand Up @@ -221,7 +210,7 @@ func (t *customTracer) StartSpan(operationName string, opts ...opentracing.Start
}

func ExampleClient_customTracer() {
mockTracer := mocktracer.New()
mockTracer := tracingtest.NewTracer()
cli := net.NewClient(net.Options{
Tracer: &customTracer{mockTracer},
OpentracingSpanName: "clientSpan",
Expand All @@ -232,10 +221,6 @@ func ExampleClient_customTracer() {
defer srv.Close()

cli.Get("http://" + srv.Listener.Addr().String() + "/")

// wait for the span to be finished
waitForSpanViaMockTracer(mockTracer)

fmt.Printf("customtag: %s", mockTracer.FinishedSpans()[0].Tags()["customtag"])

// Output:
Expand Down Expand Up @@ -343,7 +328,7 @@ func ExampleClient_hostSecret() {
}

func ExampleClient_withBeforeSendHook() {
mockTracer := mocktracer.New()
mockTracer := tracingtest.NewTracer()
peerService := "my-peer-service"
cli := net.NewClient(net.Options{
Tracer: &customTracer{mockTracer},
Expand All @@ -369,10 +354,6 @@ func ExampleClient_withBeforeSendHook() {
defer srv.Close()

cli.Get("http://" + srv.Listener.Addr().String() + "/")

// wait for the span to be finished
waitForSpanViaMockTracer(mockTracer)

fmt.Printf("request tag %q set to %q", string(ext.PeerService), mockTracer.FinishedSpans()[0].Tags()[string(ext.PeerService)])

// Output:
Expand All @@ -381,7 +362,7 @@ func ExampleClient_withBeforeSendHook() {
}

func ExampleClient_withAfterResponseHook() {
mockTracer := mocktracer.New()
mockTracer := tracingtest.NewTracer()
cli := net.NewClient(net.Options{
Tracer: &customTracer{mockTracer},
OpentracingComponentTag: "testclient",
Expand Down Expand Up @@ -411,9 +392,6 @@ func ExampleClient_withAfterResponseHook() {
log.Fatalf("Failed to get: %v", err)
}

// wait for the span to be finished
waitForSpanViaMockTracer(mockTracer)

fmt.Printf("response code: %d\n", rsp.StatusCode)
fmt.Printf("span status.code: %d", mockTracer.FinishedSpans()[0].Tags()["status.code"])

Expand Down
4 changes: 2 additions & 2 deletions net/httpclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import (
"github.com/AlexanderYastrebov/noleak"
"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/mocktracer"
"github.com/zalando/skipper/secrets"
"github.com/zalando/skipper/tracing/tracers/basic"
"github.com/zalando/skipper/tracing/tracingtest"
)

var testToken = []byte("mytoken1")
Expand Down Expand Up @@ -208,7 +208,7 @@ func TestClient(t *testing.T) {
}

func TestTransport(t *testing.T) {
mtracer := mocktracer.New()
mtracer := tracingtest.NewTracer()
tracer, err := basic.InitTracer(nil)
if err != nil {
t.Fatalf("Failed to get a tracer: %v", err)
Expand Down
38 changes: 13 additions & 25 deletions proxy/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestTracingIngressSpan(t *testing.T) {
routeID := "ingressRoute"
doc := fmt.Sprintf(`%s: Path("/hello") -> setPath("/bye") -> setQuery("void") -> "%s"`, routeID, s.URL)

tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
params := Params{
OpenTracing: &OpenTracingParams{
Tracer: tracer,
Expand Down Expand Up @@ -117,9 +117,6 @@ func TestTracingIngressSpan(t *testing.T) {
t.Fatal(err)
}

// client may get response before proxy finishes span
time.Sleep(10 * time.Millisecond)

span, ok := findSpan(tracer, "ingress")
if !ok {
t.Fatal("ingress span not found")
Expand All @@ -143,7 +140,7 @@ func TestTracingIngressSpanShunt(t *testing.T) {
routeID := "ingressShuntRoute"
doc := fmt.Sprintf(`%s: Path("/hello") -> setPath("/bye") -> setQuery("void") -> status(205) -> <shunt>`, routeID)

tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
params := Params{
OpenTracing: &OpenTracingParams{
Tracer: tracer,
Expand Down Expand Up @@ -175,9 +172,6 @@ func TestTracingIngressSpanShunt(t *testing.T) {
defer rsp.Body.Close()
io.Copy(io.Discard, rsp.Body)

// client may get response before proxy finishes span
time.Sleep(10 * time.Millisecond)

span, ok := findSpan(tracer, "ingress")
if !ok {
t.Fatal("ingress span not found")
Expand Down Expand Up @@ -214,7 +208,7 @@ func TestTracingIngressSpanLoopback(t *testing.T) {
%s: Path("/loop2") -> setPath("/loop1") -> <loopback>;
`, shuntRouteID, loop1RouteID, loop2RouteID)

tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
params := Params{
OpenTracing: &OpenTracingParams{
Tracer: tracer,
Expand Down Expand Up @@ -247,9 +241,6 @@ func TestTracingIngressSpanLoopback(t *testing.T) {
io.Copy(io.Discard, rsp.Body)
t.Logf("got response %d", rsp.StatusCode)

// client may get response before proxy finishes span
time.Sleep(10 * time.Millisecond)

sp, ok := findSpanByRouteID(tracer, loop2RouteID)
if !ok {
t.Fatalf("span for route %q not found", loop2RouteID)
Expand Down Expand Up @@ -375,7 +366,7 @@ func TestTracingProxySpan(t *testing.T) {
defer s.Close()

doc := fmt.Sprintf(`hello: Path("/hello") -> setPath("/bye") -> setQuery("void") -> "%s"`, s.URL)
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()

t.Setenv("HOSTNAME", "proxy.tracing.test")

Expand All @@ -399,9 +390,6 @@ func TestTracingProxySpan(t *testing.T) {
t.Fatal(err)
}

// client may get response before proxy finishes span
time.Sleep(10 * time.Millisecond)

span, ok := findSpan(tracer, "proxy")
if !ok {
t.Fatal("proxy span not found")
Expand Down Expand Up @@ -532,7 +520,7 @@ func TestFilterTracing(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tc.params.Tracer = tracer
tracing := newProxyTracing(tc.params)

Expand Down Expand Up @@ -574,7 +562,7 @@ func spanLogs(span *mocktracer.MockSpan) string {
}

func TestEnabledLogStreamEvents(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
LogStreamEvents: true,
Expand All @@ -593,7 +581,7 @@ func TestEnabledLogStreamEvents(t *testing.T) {
}

func TestDisabledLogStreamEvents(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
LogStreamEvents: false,
Expand All @@ -612,7 +600,7 @@ func TestDisabledLogStreamEvents(t *testing.T) {
}

func TestSetEnabledTags(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
ExcludeTags: []string{},
Expand All @@ -636,7 +624,7 @@ func TestSetEnabledTags(t *testing.T) {
}

func TestSetDisabledTags(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
ExcludeTags: []string{
Expand Down Expand Up @@ -668,7 +656,7 @@ func TestSetDisabledTags(t *testing.T) {
}

func TestLogEventWithEmptySpan(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
})
Expand All @@ -679,7 +667,7 @@ func TestLogEventWithEmptySpan(t *testing.T) {
}

func TestSetTagWithEmptySpan(t *testing.T) {
tracer := mocktracer.New()
tracer := tracingtest.NewTracer()
tracing := newProxyTracing(&OpenTracingParams{
Tracer: tracer,
})
Expand All @@ -688,7 +676,7 @@ func TestSetTagWithEmptySpan(t *testing.T) {
tracing.setTag(nil, "test", "val")
}

func findSpan(tracer *mocktracer.MockTracer, name string) (*mocktracer.MockSpan, bool) {
func findSpan(tracer *tracingtest.MockTracer, name string) (*mocktracer.MockSpan, bool) {
for _, s := range tracer.FinishedSpans() {
if s.OperationName == name {
return s, true
Expand All @@ -697,7 +685,7 @@ func findSpan(tracer *mocktracer.MockTracer, name string) (*mocktracer.MockSpan,
return nil, false
}

func findSpanByRouteID(tracer *mocktracer.MockTracer, routeID string) (*mocktracer.MockSpan, bool) {
func findSpanByRouteID(tracer *tracingtest.MockTracer, routeID string) (*mocktracer.MockSpan, bool) {
for _, s := range tracer.FinishedSpans() {
if s.Tag(SkipperRouteIDTag) == routeID {
return s, true
Expand Down
4 changes: 2 additions & 2 deletions tracing/tracingtest/mocktracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ type MockTracer struct {
spans atomic.Int32
}

func NewMockTracer() *MockTracer {
return &MockTracer{mockTracer: &mocktracer.MockTracer{}}
func NewTracer() *MockTracer {
return &MockTracer{mockTracer: mocktracer.New()}
}

func (t *MockTracer) Reset() {
Expand Down

0 comments on commit 4738392

Please sign in to comment.