Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tracecontextb3 HTTPFormats #1429

Merged
merged 6 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/spoof/spoof.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ import (
"knative.dev/pkg/test/ingress"
"knative.dev/pkg/test/logging"
"knative.dev/pkg/test/zipkin"
"knative.dev/pkg/tracing/b3tracecontext"

"go.opencensus.io/plugin/ochttp"
"go.opencensus.io/plugin/ochttp/propagation/b3"
"go.opencensus.io/trace"
)

Expand Down Expand Up @@ -135,7 +135,7 @@ func New(
// Enable Zipkin tracing
roundTripper := &ochttp.Transport{
Base: transport,
Propagation: &b3.HTTPFormat{},
Propagation: &b3tracecontext.HTTPFormat{},
}

sc := SpoofingClient{
Expand Down
53 changes: 53 additions & 0 deletions tracing/b3tracecontext/b3_traceparent.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
Copyright 2020 The Knative 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 b3tracecontext

import (
"net/http"

"go.opencensus.io/plugin/ochttp/propagation/b3"
"go.opencensus.io/plugin/ochttp/propagation/tracecontext"
"go.opencensus.io/trace"
"go.opencensus.io/trace/propagation"
)

// The propagation.HTTPFormats that this is built on.
var (
b3Format = &b3.HTTPFormat{}
traceContext = &tracecontext.HTTPFormat{}
)

// HTTPFormat is a propagation.HTTPFormat that reads both B3 and TraceContext tracing headers,
// preferring TraceContext. It will write both formats.
type HTTPFormat struct{}
Harwayne marked this conversation as resolved.
Show resolved Hide resolved

var _ propagation.HTTPFormat = (*HTTPFormat)(nil)

func (H *HTTPFormat) SpanContextFromRequest(req *http.Request) (trace.SpanContext, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (H *HTTPFormat) SpanContextFromRequest(req *http.Request) (trace.SpanContext, bool) {
func (h *HTTPFormat) SpanContextFromRequest(req *http.Request) (trace.SpanContext, bool) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed entirely.

Harwayne marked this conversation as resolved.
Show resolved Hide resolved
if sc, ok := traceContext.SpanContextFromRequest(req); ok {
return sc, true
}
if sc, ok := b3Format.SpanContextFromRequest(req); ok {
return sc, true
}
return trace.SpanContext{}, false
}

func (H *HTTPFormat) SpanContextToRequest(sc trace.SpanContext, req *http.Request) {
Harwayne marked this conversation as resolved.
Show resolved Hide resolved
traceContext.SpanContextToRequest(sc, req)
b3Format.SpanContextToRequest(sc, req)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we set both headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

139 changes: 139 additions & 0 deletions tracing/b3tracecontext/b3_traceparent_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
Copyright 2020 The Knative 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 b3tracecontext

import (
"fmt"
"net/http"
"testing"

"github.com/google/go-cmp/cmp"
"go.opencensus.io/plugin/ochttp/propagation/b3"
"go.opencensus.io/plugin/ochttp/propagation/tracecontext"
"go.opencensus.io/trace"
"go.opencensus.io/trace/propagation"

_ "knative.dev/pkg/metrics/testing"
)

var (
sampled trace.TraceOptions = 1
notSampled trace.TraceOptions = 1

traceID = trace.TraceID{99, 108, 97, 101, 114, 115, 105, 103, 104, 116, 101, 100, 110, 101, 115, 115}
spanID = trace.SpanID{107, 110, 97, 116, 105, 118, 101, 0}

tracePropagators = []propagation.HTTPFormat{
&b3.HTTPFormat{},
&tracecontext.HTTPFormat{},
&HTTPFormat{},
}
)

func TestSpanContextFromRequest(t *testing.T) {
testCases := map[string]struct {
sc *trace.SpanContext
}{
"no incoming trace": {},
"not sampled": {
sc: &trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceOptions: notSampled,
},
},
"sampled": {
sc: &trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceOptions: sampled,
},
},
}

for _, tracePropagator := range tracePropagators {
for n, tc := range testCases {
t.Run(fmt.Sprintf("%T-%s", tracePropagator, n), func(t *testing.T) {
r := &http.Request{}
r.Header = http.Header{}
if tc.sc != nil {
tracePropagator.SpanContextToRequest(*tc.sc, r)
}
// Check we extract the correct SpanContext with both the original and the
// b3tracecontext propagators.
for _, extractFormat := range []propagation.HTTPFormat{tracePropagator, &HTTPFormat{}} {
actual, ok := extractFormat.SpanContextFromRequest(r)
if tc.sc == nil {
if ok {
t.Errorf("Expected no span context using %T, found %v", extractFormat, actual)
}
continue
}
if diff := cmp.Diff(*tc.sc, actual); diff != "" {
t.Errorf("Unexpected span context using %T (-want +got): %s", extractFormat, diff)
}
}
})
}
}
}

func TestSpanContextToRequest(t *testing.T) {
testCases := map[string]struct {
sc *trace.SpanContext
}{
"no incoming trace": {},
"not sampled": {
sc: &trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceOptions: notSampled,
},
},
"sampled": {
sc: &trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceOptions: sampled,
},
},
}

for n, tc := range testCases {
t.Run(n, func(t *testing.T) {
r := &http.Request{}
r.Header = http.Header{}
if tc.sc != nil {
// Apply using the b3tracecontext propagator.
(&HTTPFormat{}).SpanContextToRequest(*tc.sc, r)
}
// Verify that we extract the correct SpanContext with all three formats.
for _, tracePropagator := range tracePropagators {
actual, ok := tracePropagator.SpanContextFromRequest(r)
if tc.sc == nil {
if ok {
t.Errorf("Expected no span context using %T, found %v", tracePropagator, actual)
}
continue
}
if diff := cmp.Diff(*tc.sc, actual); diff != "" {
t.Errorf("Unexpected span context using %T (-want +got): %s", tracePropagator, diff)
}
}
})
}
}
2 changes: 2 additions & 0 deletions tracing/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.opencensus.io/plugin/ochttp"
"go.opencensus.io/trace"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/tracing/b3tracecontext"
)

var (
Expand Down Expand Up @@ -50,6 +51,7 @@ func HTTPSpanIgnoringPaths(pathsToIgnore ...string) func(http.Handler) http.Hand
}
return underlyingSampling
},
Propagation: &b3tracecontext.HTTPFormat{},
}
}
}
Loading