Skip to content

Commit

Permalink
Remove use of oteltest in otelecho (#997)
Browse files Browse the repository at this point in the history
* Refactor to use Noop TracerProvider where possible

* Use tracetest instead of oteltest in test mod

* Fix B3 propagators import
  • Loading branch information
MrAlias authored Aug 18, 2021
1 parent ba1ce0d commit ec88a56
Show file tree
Hide file tree
Showing 9 changed files with 278 additions and 133 deletions.
10 changes: 10 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,16 @@ updates:
schedule:
interval: "weekly"
day: "sunday"
-
package-ecosystem: "gomod"
directory: "/instrumentation/github.com/labstack/echo/otelecho/test"
labels:
- dependencies
- go
- "Skip Changelog"
schedule:
interval: "weekly"
day: "sunday"
-
package-ecosystem: "gomod"
directory: "/instrumentation/github.com/Shopify/sarama/otelsarama"
Expand Down
153 changes: 25 additions & 128 deletions instrumentation/github.com/labstack/echo/otelecho/echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,123 +26,14 @@ import (
"github.com/labstack/echo/v4"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/oteltest"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"

b3prop "go.opentelemetry.io/contrib/propagators/b3"
"go.opentelemetry.io/otel/attribute"
oteltrace "go.opentelemetry.io/otel/trace"
)

func TestChildSpanFromGlobalTracer(t *testing.T) {
otel.SetTracerProvider(oteltest.NewTracerProvider())

router := echo.New()
router.Use(Middleware("foobar"))
router.GET("/user/:id", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
_, ok := span.(*oteltest.Span)
assert.True(t, ok)
return c.NoContent(200)
})

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

router.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Result().StatusCode, "should call the 'user' handler")
}

func TestChildSpanFromCustomTracer(t *testing.T) {
provider := oteltest.NewTracerProvider()

router := echo.New()
router.Use(Middleware("foobar", WithTracerProvider(provider)))
router.GET("/user/:id", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
_, ok := span.(*oteltest.Span)
assert.True(t, ok)
return c.NoContent(200)
})

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

router.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Result().StatusCode, "should call the 'user' handler")
}

func TestTrace200(t *testing.T) {
sr := new(oteltest.SpanRecorder)
provider := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr))

router := echo.New()
router.Use(Middleware("foobar", WithTracerProvider(provider)))
router.GET("/user/:id", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
mspan, ok := span.(*oteltest.Span)
require.True(t, ok)
assert.Equal(t, attribute.StringValue("foobar"), mspan.Attributes()["http.server_name"])
id := c.Param("id")
return c.String(200, id)
})

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

// do and verify the request
router.ServeHTTP(w, r)
response := w.Result()
require.Equal(t, http.StatusOK, response.StatusCode)

// verify traces look good
spans := sr.Completed()
require.Len(t, spans, 1)
span := spans[0]
assert.Equal(t, "/user/:id", span.Name())
assert.Equal(t, oteltrace.SpanKindServer, span.SpanKind())
assert.Equal(t, attribute.StringValue("foobar"), span.Attributes()["http.server_name"])
assert.Equal(t, attribute.IntValue(http.StatusOK), span.Attributes()["http.status_code"])
assert.Equal(t, attribute.StringValue("GET"), span.Attributes()["http.method"])
assert.Equal(t, attribute.StringValue("/user/123"), span.Attributes()["http.target"])
assert.Equal(t, attribute.StringValue("/user/:id"), span.Attributes()["http.route"])
}

func TestError(t *testing.T) {
sr := new(oteltest.SpanRecorder)
provider := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr))

// setup
router := echo.New()
router.Use(Middleware("foobar", WithTracerProvider(provider)))
wantErr := errors.New("oh no")
// configure a handler that returns an error and 5xx status
// code
router.GET("/server_err", func(c echo.Context) error {
return wantErr
})
r := httptest.NewRequest("GET", "/server_err", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, r)
response := w.Result()
assert.Equal(t, http.StatusInternalServerError, response.StatusCode)

// verify the errors and status are correct
spans := sr.Completed()
require.Len(t, spans, 1)
span := spans[0]
assert.Equal(t, "/server_err", span.Name())
assert.Equal(t, attribute.StringValue("foobar"), span.Attributes()["http.server_name"])
assert.Equal(t, attribute.IntValue(http.StatusInternalServerError), span.Attributes()["http.status_code"])
assert.Equal(t, attribute.StringValue("oh no"), span.Attributes()["echo.error"])
// server errors set the status
assert.Equal(t, codes.Error, span.StatusCode())
}

func TestErrorOnlyHandledOnce(t *testing.T) {
router := echo.New()
timesHandlingError := 0
Expand All @@ -164,7 +55,7 @@ func TestGetSpanNotInstrumented(t *testing.T) {
router := echo.New()
router.GET("/ping", func(c echo.Context) error {
// Assert we don't have a span on the context.
span := oteltrace.SpanFromContext(c.Request().Context())
span := trace.SpanFromContext(c.Request().Context())
ok := !span.SpanContext().IsValid()
assert.True(t, ok)
return c.String(200, "ok")
Expand All @@ -177,24 +68,27 @@ func TestGetSpanNotInstrumented(t *testing.T) {
}

func TestPropagationWithGlobalPropagators(t *testing.T) {
sr := new(oteltest.SpanRecorder)
provider := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr))
provider := trace.NewNoopTracerProvider()
otel.SetTextMapPropagator(propagation.TraceContext{})

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

ctx, pspan := provider.Tracer(tracerName).Start(context.Background(), "test")
ctx := context.Background()
sc := trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID{0x01},
SpanID: trace.SpanID{0x01},
})
ctx = trace.ContextWithRemoteSpanContext(ctx, sc)
ctx, _ = provider.Tracer(tracerName).Start(ctx, "test")
otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(r.Header))

router := echo.New()
router.Use(Middleware("foobar", WithTracerProvider(provider)))
router.GET("/user/:id", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
mspan, ok := span.(*oteltest.Span)
require.True(t, ok)
assert.Equal(t, pspan.SpanContext().TraceID(), mspan.SpanContext().TraceID())
assert.Equal(t, pspan.SpanContext().SpanID(), mspan.ParentSpanID())
span := trace.SpanFromContext(c.Request().Context())
assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID())
assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID())
return c.NoContent(200)
})

Expand All @@ -204,25 +98,28 @@ func TestPropagationWithGlobalPropagators(t *testing.T) {
}

func TestPropagationWithCustomPropagators(t *testing.T) {
sr := new(oteltest.SpanRecorder)
provider := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr))
provider := trace.NewNoopTracerProvider()

b3 := b3prop.New()

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

ctx, pspan := provider.Tracer(tracerName).Start(context.Background(), "test")
ctx := context.Background()
sc := trace.NewSpanContext(trace.SpanContextConfig{
TraceID: trace.TraceID{0x01},
SpanID: trace.SpanID{0x01},
})
ctx = trace.ContextWithRemoteSpanContext(ctx, sc)
ctx, _ = provider.Tracer(tracerName).Start(ctx, "test")
b3.Inject(ctx, propagation.HeaderCarrier(r.Header))

router := echo.New()
router.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(b3)))
router.GET("/user/:id", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
mspan, ok := span.(*oteltest.Span)
require.True(t, ok)
assert.Equal(t, pspan.SpanContext().TraceID(), mspan.SpanContext().TraceID())
assert.Equal(t, pspan.SpanContext().SpanID(), mspan.ParentSpanID())
span := trace.SpanFromContext(c.Request().Context())
assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID())
assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID())
return c.NoContent(200)
})

Expand All @@ -241,7 +138,7 @@ func TestSkipper(t *testing.T) {
router := echo.New()
router.Use(Middleware("foobar", WithSkipper(skipper)))
router.GET("/ping", func(c echo.Context) error {
span := oteltrace.SpanFromContext(c.Request().Context())
span := trace.SpanFromContext(c.Request().Context())
assert.False(t, span.SpanContext().HasSpanID())
assert.False(t, span.SpanContext().HasTraceID())
return c.NoContent(200)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ go.opentelemetry.io/otel v1.0.0-RC2 h1:SHhxSjB+omnGZPgGlKe+QMp3MyazcOHdQ8qwo89oK
go.opentelemetry.io/otel v1.0.0-RC2/go.mod h1:w1thVQ7qbAy8MHb0IFj8a5Q2QU0l2ksf8u/CN8m3NOM=
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.0.0-RC2 h1:crksoFyTPDDywRJDUW36OZma+C3HhcYwQLPUZZMXFO0=
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.0.0-RC2/go.mod h1:6kVxj1C/f3irP/IeeZNbcEwbg3rwnM6a7bCrcGbIJeI=
go.opentelemetry.io/otel/oteltest v1.0.0-RC2 h1:xNKqMhlZYkASSyvF4JwObZFMq0jhFN3c3SP+2rCzVPk=
go.opentelemetry.io/otel/oteltest v1.0.0-RC2/go.mod h1:kiQ4tw5tAL4JLTbcOYwK1CWI1HkT5aiLzHovgOVnz/A=
go.opentelemetry.io/otel/sdk v1.0.0-RC2 h1:ROuteeSCBaZNjiT9JcFzZepmInDvLktR28Y6qKo8bCs=
go.opentelemetry.io/otel/sdk v1.0.0-RC2/go.mod h1:fgwHyiDn4e5k40TD9VX243rOxXR+jzsWBZYA2P5jpEw=
go.opentelemetry.io/otel/trace v1.0.0-RC2 h1:dunAP0qDULMIT82atj34m5RgvsIK6LcsXf1c/MsYg1w=
Expand Down
1 change: 0 additions & 1 deletion instrumentation/github.com/labstack/echo/otelecho/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@ require (
go.opentelemetry.io/contrib v0.22.0
go.opentelemetry.io/contrib/propagators/b3 v0.22.0
go.opentelemetry.io/otel v1.0.0-RC2
go.opentelemetry.io/otel/oteltest v1.0.0-RC2
go.opentelemetry.io/otel/trace v1.0.0-RC2
)
2 changes: 0 additions & 2 deletions instrumentation/github.com/labstack/echo/otelecho/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ github.com/valyala/fasttemplate v1.2.1 h1:TVEnxayobAdVkhQfrfes2IzOB6o+z4roRkPF52
github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ=
go.opentelemetry.io/otel v1.0.0-RC2 h1:SHhxSjB+omnGZPgGlKe+QMp3MyazcOHdQ8qwo89oKbg=
go.opentelemetry.io/otel v1.0.0-RC2/go.mod h1:w1thVQ7qbAy8MHb0IFj8a5Q2QU0l2ksf8u/CN8m3NOM=
go.opentelemetry.io/otel/oteltest v1.0.0-RC2 h1:xNKqMhlZYkASSyvF4JwObZFMq0jhFN3c3SP+2rCzVPk=
go.opentelemetry.io/otel/oteltest v1.0.0-RC2/go.mod h1:kiQ4tw5tAL4JLTbcOYwK1CWI1HkT5aiLzHovgOVnz/A=
go.opentelemetry.io/otel/trace v1.0.0-RC2 h1:dunAP0qDULMIT82atj34m5RgvsIK6LcsXf1c/MsYg1w=
go.opentelemetry.io/otel/trace v1.0.0-RC2/go.mod h1:JPQ+z6nNw9mqEGT8o3eoPTdnNI+Aj5JcxEsVGREIAy4=
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2 h1:It14KIkyBFYkHkwZ7k45minvA9aorojkyjGk9KJ5B/w=
Expand Down
22 changes: 22 additions & 0 deletions instrumentation/github.com/labstack/echo/otelecho/test/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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 test validates the otelecho instrumentation with the default SDK.
This package is in a separate module from the instrumentation it tests to
isolate the dependency of the default SDK and not impose this as a transitive
dependency for users.
*/
package test
Loading

0 comments on commit ec88a56

Please sign in to comment.