From b4cf5f96901c871a7aa0d622c2f5d5b3703c68b2 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 11:30:46 -0700 Subject: [PATCH 01/28] add propagation api. --- api/trace/api.go | 22 ++++++++++++++++++++++ api/trace/noop_trace.go | 7 +++++++ experimental/streaming/sdk/trace.go | 4 ++++ sdk/trace/tracer.go | 6 ++++++ 4 files changed, 39 insertions(+) diff --git a/api/trace/api.go b/api/trace/api.go index afb49fa42a5..faa3e43e0b2 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -47,6 +47,9 @@ type Tracer interface { // Note: see https://github.com/opentracing/opentracing-go/issues/127 Inject(context.Context, Span, Injector) + + // Note: see https://github.com/opentracing/opentracing-go/issues/127 + Extract(context.Context, Extractor) (core.SpanContext, tag.Map) } type FinishOptions struct { @@ -109,6 +112,13 @@ type Injector interface { Inject(core.SpanContext, tag.Map) } +type Extractor interface { + // Extract deserializes span context and tag.Map from a carrier associated with the + // extractor. For example in case of http request, span context could be extracted + // from the W3C Trace context header. + Extract() (core.SpanContext, tag.Map) +} + // SpanOption apply changes to SpanOptions. type SpanOption func(*SpanOptions) @@ -169,6 +179,18 @@ func Inject(ctx context.Context, injector Injector) { span.Tracer().Inject(ctx, span, injector) } +// Extract is convenient function to extract remote span context using extractor. +// Extractor is expected to deserialize span context from its carrier. +// An example of a carrier is http request. +func Extract(ctx context.Context, extractor Extractor) { + span := CurrentSpan(ctx) + if span == nil { + return + } + + span.Tracer().Extract(ctx, extractor) +} + // WithStartTime sets the start time of the span to provided time t, when it is started. // In absensce of this option, wall clock time is used as start time. // This option is typically used when starting of the span is delayed. diff --git a/api/trace/noop_trace.go b/api/trace/noop_trace.go index 8a590232300..e2f3b202bba 100644 --- a/api/trace/noop_trace.go +++ b/api/trace/noop_trace.go @@ -17,6 +17,8 @@ package trace import ( "context" + "go.opentelemetry.io/api/tag" + "go.opentelemetry.io/api/core" ) @@ -53,3 +55,8 @@ func (NoopTracer) Start(ctx context.Context, name string, opts ...SpanOption) (c // Inject does nothing. func (NoopTracer) Inject(ctx context.Context, span Span, injector Injector) { } + +// Extract does nothing. +func (noopTracer) Extract(ctx context.Context, extractor Extractor) (core.SpanContext, tag.Map) { + return core.INVALID_SPAN_CONTEXT, nil +} diff --git a/experimental/streaming/sdk/trace.go b/experimental/streaming/sdk/trace.go index 40e2a8c5c41..5d34ebd2584 100644 --- a/experimental/streaming/sdk/trace.go +++ b/experimental/streaming/sdk/trace.go @@ -129,3 +129,7 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...apitrace.SpanOp func (t *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { injector.Inject(span.SpanContext(), tag.FromContext(ctx)) } + +func (t *tracer) Extract(ctx context.Context, extractor apitrace.Extractor) (core.SpanContext, tag.Map) { + return extractor.Extract() +} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index a6307bd46da..5a4624e74bc 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -17,6 +17,8 @@ package trace import ( "context" + "go.opentelemetry.io/api/tag" + "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/tag" apitrace "go.opentelemetry.io/api/trace" @@ -106,3 +108,7 @@ func (tr *tracer) WithComponent(component string) apitrace.Tracer { func (tr *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { injector.Inject(span.SpanContext(), tag.NewEmptyMap()) } + +func (tr *tracer) Extract(ctx context.Context, extractor apitrace.Extractor) (core.SpanContext, tag.Map) { + return extractor.Extract() +} From 067cb3f3847cc27b912f9fc21911c607a2a4f6f3 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 12:44:34 -0700 Subject: [PATCH 02/28] add http propagator interface and w3c propagator implementation. --- propagation/http_textformat_propagator.go | 169 ++++++++++++++++++ .../http_textformat_propagator_test.go | 129 +++++++++++++ propagation/propagator.go | 31 ++++ 3 files changed, 329 insertions(+) create mode 100644 propagation/http_textformat_propagator.go create mode 100644 propagation/http_textformat_propagator_test.go create mode 100644 propagation/propagator.go diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go new file mode 100644 index 00000000000..f5cb9a547d4 --- /dev/null +++ b/propagation/http_textformat_propagator.go @@ -0,0 +1,169 @@ +// Copyright 2018, OpenCensus 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 tracecontext contains HTTP propagator for TraceContext standard. +// See https://github.com/w3c/distributed-tracing for more information. +package propagation // import "go.opentelemetry.io/propagation" + +import ( + "encoding/hex" + "fmt" + "net/http" + "net/textproto" + "strconv" + "strings" + + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/tag" + "go.opentelemetry.io/api/trace" +) + +const ( + supportedVersion = 0 + maxVersion = 254 + traceparentHeader = "traceparent" +) + +type textFormatPropagator struct{} + +var _ HTTPPropagator = textFormatPropagator{} + +func (t textFormatPropagator) Extractor(req *http.Request) trace.Extractor { + return textFormatExtractor{req: req} +} + +func (t textFormatPropagator) Injector(req *http.Request) trace.Injector { + return textFormatInjector{req: req} +} + +// TextFormatPropagator creates a new propagator. The propagator is then used +// to create Injector and Extrator associated with a specific request. Injectors +// and Extractors respectively provides method to inject and extract SpanContext +// into/from the http request. +func TextFormatPropagator() textFormatPropagator { + return textFormatPropagator{} +} + +type textFormatExtractor struct { + req *http.Request +} + +var _ trace.Extractor = textFormatExtractor{} + +// Extract implements Extract method of trace.Extractor interface. It extracts +// W3C TraceContext Header and decodes SpanContext from the Header. +func (f textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { + h, ok := getRequestHeader(f.req, traceparentHeader, false) + if !ok { + return core.INVALID_SPAN_CONTEXT, nil + } + sections := strings.Split(h, "-") + if len(sections) < 4 { + return core.INVALID_SPAN_CONTEXT, nil + } + + if len(sections[0]) != 2 { + return core.INVALID_SPAN_CONTEXT, nil + } + ver, err := hex.DecodeString(sections[0]) + if err != nil { + return core.INVALID_SPAN_CONTEXT, nil + } + version := int(ver[0]) + if version > maxVersion { + return core.INVALID_SPAN_CONTEXT, nil + } + + if version == 0 && len(sections) != 4 { + return core.INVALID_SPAN_CONTEXT, nil + } + + if len(sections[1]) != 32 { + return core.INVALID_SPAN_CONTEXT, nil + } + + result, err := strconv.ParseUint(sections[1][0:16], 16, 64) + if err != nil { + return core.INVALID_SPAN_CONTEXT, nil + } + sc.TraceID.High = result + + result, err = strconv.ParseUint(sections[1][16:32], 16, 64) + if err != nil { + return core.INVALID_SPAN_CONTEXT, nil + } + sc.TraceID.Low = result + + if len(sections[2]) != 16 { + return core.INVALID_SPAN_CONTEXT, nil + } + result, err = strconv.ParseUint(sections[2][0:], 16, 64) + if err != nil { + return core.INVALID_SPAN_CONTEXT, nil + } + sc.SpanID = result + + opts, err := hex.DecodeString(sections[3]) + if err != nil || len(opts) < 1 { + return core.INVALID_SPAN_CONTEXT, nil + } + sc.TraceOptions = opts[0] + + if !sc.IsValid() { + return core.INVALID_SPAN_CONTEXT, nil + } + + // TODO: [rghetia] add tag.Map (distributed context) extraction + return sc, nil +} + +type textFormatInjector struct { + req *http.Request +} + +var _ trace.Injector = textFormatInjector{} + +// Inject implements Inject method of trace.Injector interface. It encodes +// SpanContext into W3C TraceContext Header and injects the header into +// the associated request. +func (f textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { + if sc.IsValid() { + h := fmt.Sprintf("%.2x-%.16x%.16x-%.16x-%.2x", + supportedVersion, + sc.TraceID.High, + sc.TraceID.Low, + sc.SpanID, + sc.TraceOptions) + f.req.Header.Set(traceparentHeader, h) + } + // TODO: [rghetia] add tag.Map (distributed context) injection +} + +// getRequestHeader returns a combined header field according to RFC7230 section 3.2.2. +// If commaSeparated is true, multiple header fields with the same field name using be +// combined using ",". +// If no header was found using the given name, "ok" would be false. +// If more than one headers was found using the given name, while commaSeparated is false, +// "ok" would be false. +func getRequestHeader(req *http.Request, name string, commaSeparated bool) (hdr string, ok bool) { + v := req.Header[textproto.CanonicalMIMEHeaderKey(name)] + switch len(v) { + case 0: + return "", false + case 1: + return v[0], true + default: + return strings.Join(v, ","), commaSeparated + } +} diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go new file mode 100644 index 00000000000..78dab8bbff1 --- /dev/null +++ b/propagation/http_textformat_propagator_test.go @@ -0,0 +1,129 @@ +// Copyright 2018, OpenCensus 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 propagation_test + +import ( + "net/http" + "testing" + + "github.com/google/go-cmp/cmp" + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/propagation" +) + +var ( + traceID = core.TraceID{High: 0x4bf92f3577b34da6, Low: 0xa3ce929d0e0e4736} + spanID = uint64(0x00f067aa0ba902b7) +) + +func TestExtractTraceContextFromHTTPReq(t *testing.T) { + propagator := propagation.TextFormatPropagator() + tests := []struct { + name string + header string + wantSc core.SpanContext + }{ + { + name: "future version", + header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + { + name: "zero trace ID and span ID", + header: "00-00000000000000000000000000000000-0000000000000000-01", + wantSc: core.INVALID_SPAN_CONTEXT, + }, + { + name: "valid header", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + { + name: "missing options", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", + wantSc: core.INVALID_SPAN_CONTEXT, + }, + { + name: "empty options", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", + wantSc: core.INVALID_SPAN_CONTEXT, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com", nil) + req.Header.Set("traceparent", tt.header) + f := propagator.Extractor(req) + + gotSc, _ := f.Extract() + if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} + +func TestInjectTraceContextToHTTPReq(t *testing.T) { + propagator := propagation.TextFormatPropagator() + tests := []struct { + name string + sc core.SpanContext + wantHeader string + }{ + { + name: "valid spancontext, sampled", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + }, + { + name: "valid spancontext, not sampled", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", + }, + { + name: "invalid spancontext", + sc: core.INVALID_SPAN_CONTEXT, + wantHeader: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com", nil) + f := propagator.Injector(req) + f.Inject(tt.sc, nil) + + gotHeader := req.Header.Get("traceparent") + if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} diff --git a/propagation/propagator.go b/propagation/propagator.go new file mode 100644 index 00000000000..b9f12a259ea --- /dev/null +++ b/propagation/propagator.go @@ -0,0 +1,31 @@ +// Copyright 2018, OpenCensus 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 tracecontext contains HTTP propagator for TraceContext standard. +// See https://github.com/w3c/distributed-tracing for more information. +package propagation // import "go.opentelemetry.io/propagation" + +import ( + "net/http" + + "go.opentelemetry.io/api/trace" +) + +// HTTPPropagator is an interface that specifies methods to create Extractor +// and Injector objects for an http request. Typically, an http plugin uses +// this interface to allow user to configure appropriate propagators. +type HTTPPropagator interface { + Extractor(req *http.Request) trace.Extractor + Injector(req *http.Request) trace.Injector +} From fdaf0f0532c833b7321520bf1686dd1772229e5e Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 13:53:45 -0700 Subject: [PATCH 03/28] remove Extract api from trace. --- api/trace/api.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index faa3e43e0b2..39a305bdd02 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -179,18 +179,6 @@ func Inject(ctx context.Context, injector Injector) { span.Tracer().Inject(ctx, span, injector) } -// Extract is convenient function to extract remote span context using extractor. -// Extractor is expected to deserialize span context from its carrier. -// An example of a carrier is http request. -func Extract(ctx context.Context, extractor Extractor) { - span := CurrentSpan(ctx) - if span == nil { - return - } - - span.Tracer().Extract(ctx, extractor) -} - // WithStartTime sets the start time of the span to provided time t, when it is started. // In absensce of this option, wall clock time is used as start time. // This option is typically used when starting of the span is delayed. From 7137e7b1018cd71dc7559814905c0d0af36cf1c2 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 13:58:23 -0700 Subject: [PATCH 04/28] remove Extract interface for tracer. --- api/trace/api.go | 3 --- api/trace/noop_trace.go | 7 ------- experimental/streaming/sdk/trace.go | 4 ---- sdk/trace/tracer.go | 6 ------ 4 files changed, 20 deletions(-) diff --git a/api/trace/api.go b/api/trace/api.go index 39a305bdd02..ab8a97429f0 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -47,9 +47,6 @@ type Tracer interface { // Note: see https://github.com/opentracing/opentracing-go/issues/127 Inject(context.Context, Span, Injector) - - // Note: see https://github.com/opentracing/opentracing-go/issues/127 - Extract(context.Context, Extractor) (core.SpanContext, tag.Map) } type FinishOptions struct { diff --git a/api/trace/noop_trace.go b/api/trace/noop_trace.go index e2f3b202bba..8a590232300 100644 --- a/api/trace/noop_trace.go +++ b/api/trace/noop_trace.go @@ -17,8 +17,6 @@ package trace import ( "context" - "go.opentelemetry.io/api/tag" - "go.opentelemetry.io/api/core" ) @@ -55,8 +53,3 @@ func (NoopTracer) Start(ctx context.Context, name string, opts ...SpanOption) (c // Inject does nothing. func (NoopTracer) Inject(ctx context.Context, span Span, injector Injector) { } - -// Extract does nothing. -func (noopTracer) Extract(ctx context.Context, extractor Extractor) (core.SpanContext, tag.Map) { - return core.INVALID_SPAN_CONTEXT, nil -} diff --git a/experimental/streaming/sdk/trace.go b/experimental/streaming/sdk/trace.go index 5d34ebd2584..40e2a8c5c41 100644 --- a/experimental/streaming/sdk/trace.go +++ b/experimental/streaming/sdk/trace.go @@ -129,7 +129,3 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...apitrace.SpanOp func (t *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { injector.Inject(span.SpanContext(), tag.FromContext(ctx)) } - -func (t *tracer) Extract(ctx context.Context, extractor apitrace.Extractor) (core.SpanContext, tag.Map) { - return extractor.Extract() -} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 5a4624e74bc..a6307bd46da 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -17,8 +17,6 @@ package trace import ( "context" - "go.opentelemetry.io/api/tag" - "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/tag" apitrace "go.opentelemetry.io/api/trace" @@ -108,7 +106,3 @@ func (tr *tracer) WithComponent(component string) apitrace.Tracer { func (tr *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { injector.Inject(span.SpanContext(), tag.NewEmptyMap()) } - -func (tr *tracer) Extract(ctx context.Context, extractor apitrace.Extractor) (core.SpanContext, tag.Map) { - return extractor.Extract() -} From d9c61573fe0baa47776a3249c9d1d78c8dab5009 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 14:00:10 -0700 Subject: [PATCH 05/28] fix copyright. --- propagation/http_textformat_propagator.go | 2 +- propagation/http_textformat_propagator_test.go | 2 +- propagation/propagator.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index f5cb9a547d4..cd1bdad33f6 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -1,4 +1,4 @@ -// Copyright 2018, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go index 78dab8bbff1..a0b510e7f9b 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_textformat_propagator_test.go @@ -1,4 +1,4 @@ -// Copyright 2018, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/propagation/propagator.go b/propagation/propagator.go index b9f12a259ea..122870a8b27 100644 --- a/propagation/propagator.go +++ b/propagation/propagator.go @@ -1,4 +1,4 @@ -// Copyright 2018, OpenCensus Authors +// Copyright 2019, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 1e2f4eac9a22efbd90c5d5724fde6bf68e9c27ab Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 9 Aug 2019 15:02:13 -0700 Subject: [PATCH 06/28] fix variable names and comments. --- propagation/http_textformat_propagator.go | 12 +++++++----- propagation/http_textformat_propagator_test.go | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index cd1bdad33f6..57b40eeea4b 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -50,7 +50,9 @@ func (t textFormatPropagator) Injector(req *http.Request) trace.Injector { // TextFormatPropagator creates a new propagator. The propagator is then used // to create Injector and Extrator associated with a specific request. Injectors // and Extractors respectively provides method to inject and extract SpanContext -// into/from the http request. +// into/from the http request. Inject method encodes SpanContext into W3C +// TraceContext Header and injects the header in the request. Extract extracts +// the header and decodes SpanContext. func TextFormatPropagator() textFormatPropagator { return textFormatPropagator{} } @@ -63,8 +65,8 @@ var _ trace.Extractor = textFormatExtractor{} // Extract implements Extract method of trace.Extractor interface. It extracts // W3C TraceContext Header and decodes SpanContext from the Header. -func (f textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { - h, ok := getRequestHeader(f.req, traceparentHeader, false) +func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { + h, ok := getRequestHeader(tfe.req, traceparentHeader, false) if !ok { return core.INVALID_SPAN_CONTEXT, nil } @@ -137,7 +139,7 @@ var _ trace.Injector = textFormatInjector{} // Inject implements Inject method of trace.Injector interface. It encodes // SpanContext into W3C TraceContext Header and injects the header into // the associated request. -func (f textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { +func (tfi textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { if sc.IsValid() { h := fmt.Sprintf("%.2x-%.16x%.16x-%.16x-%.2x", supportedVersion, @@ -145,7 +147,7 @@ func (f textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { sc.TraceID.Low, sc.SpanID, sc.TraceOptions) - f.req.Header.Set(traceparentHeader, h) + tfi.req.Header.Set(traceparentHeader, h) } // TODO: [rghetia] add tag.Map (distributed context) injection } diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go index a0b510e7f9b..5e95bef3d67 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_textformat_propagator_test.go @@ -74,9 +74,9 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) req.Header.Set("traceparent", tt.header) - f := propagator.Extractor(req) + e := propagator.Extractor(req) - gotSc, _ := f.Extract() + gotSc, _ := e.Extract() if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) } @@ -117,8 +117,8 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) - f := propagator.Injector(req) - f.Inject(tt.sc, nil) + i := propagator.Injector(req) + i.Inject(tt.sc, nil) gotHeader := req.Header.Get("traceparent") if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { From 8215b36da2090ce080da9327e740622e4b1f3f72 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Tue, 13 Aug 2019 10:31:08 -0700 Subject: [PATCH 07/28] replace INVALID_SPAN_CONTEXT with EmptySpanContext function. --- propagation/http_textformat_propagator.go | 26 +++++++++---------- .../http_textformat_propagator_test.go | 8 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index 57b40eeea4b..234bd1ced4d 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -68,62 +68,62 @@ var _ trace.Extractor = textFormatExtractor{} func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { h, ok := getRequestHeader(tfe.req, traceparentHeader, false) if !ok { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } sections := strings.Split(h, "-") if len(sections) < 4 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } if len(sections[0]) != 2 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } ver, err := hex.DecodeString(sections[0]) if err != nil { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } version := int(ver[0]) if version > maxVersion { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } if version == 0 && len(sections) != 4 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } if len(sections[1]) != 32 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } result, err := strconv.ParseUint(sections[1][0:16], 16, 64) if err != nil { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } sc.TraceID.High = result result, err = strconv.ParseUint(sections[1][16:32], 16, 64) if err != nil { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } sc.TraceID.Low = result if len(sections[2]) != 16 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } result, err = strconv.ParseUint(sections[2][0:], 16, 64) if err != nil { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } sc.SpanID = result opts, err := hex.DecodeString(sections[3]) if err != nil || len(opts) < 1 { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } sc.TraceOptions = opts[0] if !sc.IsValid() { - return core.INVALID_SPAN_CONTEXT, nil + return core.EmptySpanContext(), nil } // TODO: [rghetia] add tag.Map (distributed context) extraction diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go index 5e95bef3d67..d35f1696d89 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_textformat_propagator_test.go @@ -47,7 +47,7 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { { name: "zero trace ID and span ID", header: "00-00000000000000000000000000000000-0000000000000000-01", - wantSc: core.INVALID_SPAN_CONTEXT, + wantSc: core.EmptySpanContext(), }, { name: "valid header", @@ -61,12 +61,12 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { { name: "missing options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", - wantSc: core.INVALID_SPAN_CONTEXT, + wantSc: core.EmptySpanContext(), }, { name: "empty options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", - wantSc: core.INVALID_SPAN_CONTEXT, + wantSc: core.EmptySpanContext(), }, } @@ -110,7 +110,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { }, { name: "invalid spancontext", - sc: core.INVALID_SPAN_CONTEXT, + sc: core.EmptySpanContext(), wantHeader: "", }, } From ae02d90119216c46ecef27fdbef7b957e7f46433 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Tue, 13 Aug 2019 10:28:40 -0700 Subject: [PATCH 08/28] move inject/extract out of trace. --- api/propagation/propagator.go | 50 +++++++++++++++++++++++ api/trace/api.go | 29 ------------- api/trace/noop_trace.go | 4 -- example/http/client/client.go | 2 +- experimental/streaming/sdk/trace.go | 5 --- plugin/httptrace/api.go | 4 +- propagation/http_textformat_propagator.go | 12 +++--- propagation/propagator.go | 31 -------------- sdk/trace/tracer.go | 3 -- 9 files changed, 59 insertions(+), 81 deletions(-) create mode 100644 api/propagation/propagator.go delete mode 100644 propagation/propagator.go diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go new file mode 100644 index 00000000000..a29c240e7ed --- /dev/null +++ b/api/propagation/propagator.go @@ -0,0 +1,50 @@ +// Copyright 2019, 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 tracecontext contains HTTP propagator for TraceContext standard. +// See https://github.com/w3c/distributed-tracing for more information. +package propagation // import "go.opentelemetry.io/api/propagation" + +import ( + "net/http" + + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/tag" +) + +// TextFormatPropagator is an interface that specifies methods to create Injector +// and Extractor objects. Injector object implements Inject method to inject +// SpanContext and tag.Map as a text format into carrier like HTTP request. Similarly, +// Extractor object implements Extract method to extract SpanContext encoded in text +// format from a carrier like HTTP request. +// Typically, a plugin for transport like HTTP uses this interface to allow user +// to configure appropriate propagators. +type TextFormatPropagator interface { + Extractor(req *http.Request) Extractor + Injector(req *http.Request) Injector +} + +type Injector interface { + // Inject serializes span context and tag.Map and inserts them in to + // carrier associated with the injector. For example in case of http request, + // span context could be added to the request (carrier) as W3C Trace context header. + Inject(core.SpanContext, tag.Map) +} + +type Extractor interface { + // Extract de-serializes span context and tag.Map from a carrier associated with the + // extractor. For example in case of http request, span context could be extracted + // from the W3C Trace context header. + Extract() (core.SpanContext, tag.Map) +} diff --git a/api/trace/api.go b/api/trace/api.go index ab8a97429f0..68cba1ea990 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -44,9 +44,6 @@ type Tracer interface { // WithResources attaches resource attributes to the Tracer. WithResources(res ...core.KeyValue) Tracer - - // Note: see https://github.com/opentracing/opentracing-go/issues/127 - Inject(context.Context, Span, Injector) } type FinishOptions struct { @@ -102,20 +99,6 @@ type Span interface { ModifyAttributes(...tag.Mutator) } -type Injector interface { - // Inject serializes span context and tag.Map and inserts them in to - // carrier associated with the injector. For example in case of http request, - // span context could added to the request (carrier) as W3C Trace context header. - Inject(core.SpanContext, tag.Map) -} - -type Extractor interface { - // Extract deserializes span context and tag.Map from a carrier associated with the - // extractor. For example in case of http request, span context could be extracted - // from the W3C Trace context header. - Extract() (core.SpanContext, tag.Map) -} - // SpanOption apply changes to SpanOptions. type SpanOption func(*SpanOptions) @@ -164,18 +147,6 @@ func Start(ctx context.Context, name string, opts ...SpanOption) (context.Contex return GlobalTracer().Start(ctx, name, opts...) } -// Inject is convenient function to inject current span context using injector. -// Injector is expected to serialize span context and inject it in to a carrier. -// An example of a carrier is http request. -func Inject(ctx context.Context, injector Injector) { - span := CurrentSpan(ctx) - if span == nil { - return - } - - span.Tracer().Inject(ctx, span, injector) -} - // WithStartTime sets the start time of the span to provided time t, when it is started. // In absensce of this option, wall clock time is used as start time. // This option is typically used when starting of the span is delayed. diff --git a/api/trace/noop_trace.go b/api/trace/noop_trace.go index 8a590232300..b99c0773d45 100644 --- a/api/trace/noop_trace.go +++ b/api/trace/noop_trace.go @@ -49,7 +49,3 @@ func (NoopTracer) Start(ctx context.Context, name string, opts ...SpanOption) (c span := NoopSpan{} return SetCurrentSpan(ctx, span), span } - -// Inject does nothing. -func (NoopTracer) Inject(ctx context.Context, span Span, injector Injector) { -} diff --git a/example/http/client/client.go b/example/http/client/client.go index 0dc8e0ea8b9..5b61a815f95 100644 --- a/example/http/client/client.go +++ b/example/http/client/client.go @@ -52,7 +52,7 @@ func main() { ctx, req, inj := httptrace.W3C(ctx, req) - trace.Inject(ctx, inj) + inj.Inject(trace.CurrentSpan(ctx).SpanContext(), nil) res, err := client.Do(req) if err != nil { diff --git a/experimental/streaming/sdk/trace.go b/experimental/streaming/sdk/trace.go index 40e2a8c5c41..4371389f702 100644 --- a/experimental/streaming/sdk/trace.go +++ b/experimental/streaming/sdk/trace.go @@ -20,7 +20,6 @@ import ( "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/key" - "go.opentelemetry.io/api/tag" "go.opentelemetry.io/api/trace" apitrace "go.opentelemetry.io/api/trace" "go.opentelemetry.io/experimental/streaming/exporter/observer" @@ -125,7 +124,3 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...apitrace.SpanOp } return trace.SetCurrentSpan(ctx, span), span } - -func (t *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { - injector.Inject(span.SpanContext(), tag.FromContext(ctx)) -} diff --git a/plugin/httptrace/api.go b/plugin/httptrace/api.go index da3a0d40131..f1e0a8f4178 100644 --- a/plugin/httptrace/api.go +++ b/plugin/httptrace/api.go @@ -19,11 +19,11 @@ import ( "net/http" "net/http/httptrace" - "go.opentelemetry.io/api/trace" + "go.opentelemetry.io/api/propagation" ) // Client -func W3C(ctx context.Context, req *http.Request) (context.Context, *http.Request, trace.Injector) { +func W3C(ctx context.Context, req *http.Request) (context.Context, *http.Request, propagation.Injector) { t := newClientTracer(ctx) t.GetConn = t.getConn diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index 234bd1ced4d..9a92bc6306d 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -25,8 +25,8 @@ import ( "strings" "go.opentelemetry.io/api/core" + apipropagation "go.opentelemetry.io/api/propagation" "go.opentelemetry.io/api/tag" - "go.opentelemetry.io/api/trace" ) const ( @@ -37,13 +37,13 @@ const ( type textFormatPropagator struct{} -var _ HTTPPropagator = textFormatPropagator{} +var _ apipropagation.TextFormatPropagator = textFormatPropagator{} -func (t textFormatPropagator) Extractor(req *http.Request) trace.Extractor { +func (t textFormatPropagator) Extractor(req *http.Request) apipropagation.Extractor { return textFormatExtractor{req: req} } -func (t textFormatPropagator) Injector(req *http.Request) trace.Injector { +func (t textFormatPropagator) Injector(req *http.Request) apipropagation.Injector { return textFormatInjector{req: req} } @@ -61,7 +61,7 @@ type textFormatExtractor struct { req *http.Request } -var _ trace.Extractor = textFormatExtractor{} +var _ apipropagation.Extractor = textFormatExtractor{} // Extract implements Extract method of trace.Extractor interface. It extracts // W3C TraceContext Header and decodes SpanContext from the Header. @@ -134,7 +134,7 @@ type textFormatInjector struct { req *http.Request } -var _ trace.Injector = textFormatInjector{} +var _ apipropagation.Injector = textFormatInjector{} // Inject implements Inject method of trace.Injector interface. It encodes // SpanContext into W3C TraceContext Header and injects the header into diff --git a/propagation/propagator.go b/propagation/propagator.go deleted file mode 100644 index 122870a8b27..00000000000 --- a/propagation/propagator.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2019, 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 tracecontext contains HTTP propagator for TraceContext standard. -// See https://github.com/w3c/distributed-tracing for more information. -package propagation // import "go.opentelemetry.io/propagation" - -import ( - "net/http" - - "go.opentelemetry.io/api/trace" -) - -// HTTPPropagator is an interface that specifies methods to create Extractor -// and Injector objects for an http request. Typically, an http plugin uses -// this interface to allow user to configure appropriate propagators. -type HTTPPropagator interface { - Extractor(req *http.Request) trace.Extractor - Injector(req *http.Request) trace.Injector -} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index a6307bd46da..7deaf1d5c7d 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -103,6 +103,3 @@ func (tr *tracer) WithComponent(component string) apitrace.Tracer { return tr } -func (tr *tracer) Inject(ctx context.Context, span apitrace.Span, injector apitrace.Injector) { - injector.Inject(span.SpanContext(), tag.NewEmptyMap()) -} From ddeb734fb6e5d6e56d0fe3c6b5db6104234ce614 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 23 Aug 2019 13:49:24 -0700 Subject: [PATCH 09/28] fix tag.Map. --- propagation/http_textformat_propagator.go | 28 +++++++++---------- .../http_textformat_propagator_test.go | 4 ++- sdk/trace/tracer.go | 2 -- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index 9a92bc6306d..a102473d1f1 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -68,66 +68,66 @@ var _ apipropagation.Extractor = textFormatExtractor{} func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { h, ok := getRequestHeader(tfe.req, traceparentHeader, false) if !ok { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } sections := strings.Split(h, "-") if len(sections) < 4 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } if len(sections[0]) != 2 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } ver, err := hex.DecodeString(sections[0]) if err != nil { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } version := int(ver[0]) if version > maxVersion { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } if version == 0 && len(sections) != 4 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } if len(sections[1]) != 32 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } result, err := strconv.ParseUint(sections[1][0:16], 16, 64) if err != nil { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } sc.TraceID.High = result result, err = strconv.ParseUint(sections[1][16:32], 16, 64) if err != nil { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } sc.TraceID.Low = result if len(sections[2]) != 16 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } result, err = strconv.ParseUint(sections[2][0:], 16, 64) if err != nil { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } sc.SpanID = result opts, err := hex.DecodeString(sections[3]) if err != nil || len(opts) < 1 { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } sc.TraceOptions = opts[0] if !sc.IsValid() { - return core.EmptySpanContext(), nil + return core.EmptySpanContext(), tag.NewEmptyMap() } // TODO: [rghetia] add tag.Map (distributed context) extraction - return sc, nil + return sc, tag.NewEmptyMap() } type textFormatInjector struct { diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go index d35f1696d89..cba63806cb1 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_textformat_propagator_test.go @@ -18,6 +18,8 @@ import ( "net/http" "testing" + "go.opentelemetry.io/api/tag" + "github.com/google/go-cmp/cmp" "go.opentelemetry.io/api/core" "go.opentelemetry.io/propagation" @@ -118,7 +120,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) i := propagator.Injector(req) - i.Inject(tt.sc, nil) + i.Inject(tt.sc, tag.NewEmptyMap()) gotHeader := req.Header.Get("traceparent") if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 7deaf1d5c7d..74fb643e5a4 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -18,7 +18,6 @@ import ( "context" "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/tag" apitrace "go.opentelemetry.io/api/trace" ) @@ -102,4 +101,3 @@ func (tr *tracer) WithComponent(component string) apitrace.Tracer { tr.component = component return tr } - From ba6cda45303092e5380b6815168022bedf31ae1d Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 23 Aug 2019 21:50:19 -0700 Subject: [PATCH 10/28] make carrier as interface instead of http.Request. --- api/propagation/propagator.go | 6 +- propagation/http_textformat_propagator.go | 56 ++++++++++++++----- .../http_textformat_propagator_test.go | 52 ++++++++++++++++- 3 files changed, 93 insertions(+), 21 deletions(-) diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index a29c240e7ed..07ac781050a 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -17,8 +17,6 @@ package propagation // import "go.opentelemetry.io/api/propagation" import ( - "net/http" - "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/tag" ) @@ -31,8 +29,8 @@ import ( // Typically, a plugin for transport like HTTP uses this interface to allow user // to configure appropriate propagators. type TextFormatPropagator interface { - Extractor(req *http.Request) Extractor - Injector(req *http.Request) Injector + Extractor(carrier interface{}) Extractor + Injector(carrier interface{}) Injector } type Injector interface { diff --git a/propagation/http_textformat_propagator.go b/propagation/http_textformat_propagator.go index a102473d1f1..fcd95206f4e 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_textformat_propagator.go @@ -35,26 +35,46 @@ const ( traceparentHeader = "traceparent" ) -type textFormatPropagator struct{} +type httpTraceContextPropagator struct{} -var _ apipropagation.TextFormatPropagator = textFormatPropagator{} +var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} -func (t textFormatPropagator) Extractor(req *http.Request) apipropagation.Extractor { - return textFormatExtractor{req: req} +// Extractor implements Extractor method of TextFormatPropagator interface. +// +// It creates Extractor object and binds carrier to the object. The carrier +// is expected to be *http.Request. If the carrier type is not *http.Request +// then an empty extractor is returned which will extract nothing. +func (t httpTraceContextPropagator) Extractor(carrier interface{}) apipropagation.Extractor { + req, ok := carrier.(*http.Request) + if ok { + return textFormatExtractor{req: req} + } + return textFormatExtractor{} } -func (t textFormatPropagator) Injector(req *http.Request) apipropagation.Injector { - return textFormatInjector{req: req} +// Injector implements Injector method of TextFormatPropagator interface. +// +// It creates Injector object and binds carrier to the object. The carrier +// is expected to be of type *http.Request. If the carrier type is not *http.Request +// then an empty injector is returned which will inject nothing. +func (t httpTraceContextPropagator) Injector(carrier interface{}) apipropagation.Injector { + req, ok := carrier.(*http.Request) + if ok { + return textFormatInjector{req: req} + } + return textFormatInjector{} } -// TextFormatPropagator creates a new propagator. The propagator is then used -// to create Injector and Extrator associated with a specific request. Injectors -// and Extractors respectively provides method to inject and extract SpanContext -// into/from the http request. Inject method encodes SpanContext into W3C -// TraceContext Header and injects the header in the request. Extract extracts -// the header and decodes SpanContext. -func TextFormatPropagator() textFormatPropagator { - return textFormatPropagator{} +// HttpTraceContextPropagator creates a new propagator that propagates SpanContext +// in W3C TraceContext format. +// +// The propagator is then used to create Injector and Extractor associated with a +// specific request. Injectors and Extractors respectively provides method to +// inject and extract SpanContext into/from the http request. Inject method encodes +// SpanContext into W3C TraceContext Header and injects the header in the request. +// Extract extracts the header and decodes SpanContext. +func HttpTraceContextPropagator() httpTraceContextPropagator { + return httpTraceContextPropagator{} } type textFormatExtractor struct { @@ -66,6 +86,9 @@ var _ apipropagation.Extractor = textFormatExtractor{} // Extract implements Extract method of trace.Extractor interface. It extracts // W3C TraceContext Header and decodes SpanContext from the Header. func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { + if tfe.req == nil { + return core.EmptySpanContext(), tag.NewEmptyMap() + } h, ok := getRequestHeader(tfe.req, traceparentHeader, false) if !ok { return core.EmptySpanContext(), tag.NewEmptyMap() @@ -136,10 +159,13 @@ type textFormatInjector struct { var _ apipropagation.Injector = textFormatInjector{} -// Inject implements Inject method of trace.Injector interface. It encodes +// Inject implements Inject method of propagation.Injector interface. It encodes // SpanContext into W3C TraceContext Header and injects the header into // the associated request. func (tfi textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { + if tfi.req == nil { + return + } if sc.IsValid() { h := fmt.Sprintf("%.2x-%.16x%.16x-%.16x-%.2x", supportedVersion, diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_textformat_propagator_test.go index cba63806cb1..943e9b85074 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_textformat_propagator_test.go @@ -31,7 +31,7 @@ var ( ) func TestExtractTraceContextFromHTTPReq(t *testing.T) { - propagator := propagation.TextFormatPropagator() + propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string header string @@ -86,8 +86,33 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { } } +func TestExtractTraceContextFromInvalidCarrier(t *testing.T) { + propagator := propagation.HttpTraceContextPropagator() + tests := []struct { + name string + header string + wantSc core.SpanContext + }{ + { + name: "valid header on invalid carrier", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.EmptySpanContext(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := propagator.Extractor(tt.header) + gotSc, _ := e.Extract() + if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} + func TestInjectTraceContextToHTTPReq(t *testing.T) { - propagator := propagation.TextFormatPropagator() + propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string sc core.SpanContext @@ -129,3 +154,26 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { }) } } + +func TestInjectTraceContextToInvalidCarrier(t *testing.T) { + propagator := propagation.HttpTraceContextPropagator() + tests := []struct { + name string + sc core.SpanContext + }{ + { + name: "valid spancontext to invalid carrier", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := propagator.Injector("") + i.Inject(tt.sc, tag.NewEmptyMap()) + }) + } +} From 0c8430479092ae1567992ea825cbba288b7f5ef3 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Sun, 25 Aug 2019 23:31:52 -0700 Subject: [PATCH 11/28] rename structs and update doc comments.. --- api/propagation/propagator.go | 22 +++++--- ...or.go => http_trace_context_propagator.go} | 50 +++++++++---------- ... => http_trace_context_propagator_test.go} | 10 ++-- 3 files changed, 44 insertions(+), 38 deletions(-) rename propagation/{http_textformat_propagator.go => http_trace_context_propagator.go} (75%) rename propagation/{http_textformat_propagator_test.go => http_trace_context_propagator_test.go} (95%) diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index 07ac781050a..0dac763c695 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -21,16 +21,22 @@ import ( "go.opentelemetry.io/api/tag" ) -// TextFormatPropagator is an interface that specifies methods to create Injector -// and Extractor objects. Injector object implements Inject method to inject -// SpanContext and tag.Map as a text format into carrier like HTTP request. Similarly, -// Extractor object implements Extract method to extract SpanContext encoded in text -// format from a carrier like HTTP request. +// TextFormatPropagator is an interface that specifies methods to create CarrierInjector +// and CarrierExtractor objects. These methods bind given carrier to the created object. +// CarrierInjector object implements Inject method to inject +// SpanContext and tag.Map as a text format into carrier like HTTP request. +// Similarly, CarrierExtractor object implements Extract method to extract SpanContext +// encoded in text format from a carrier like HTTP request. // Typically, a plugin for transport like HTTP uses this interface to allow user -// to configure appropriate propagators. +// to configure appropriate text format propagators. type TextFormatPropagator interface { - Extractor(carrier interface{}) Extractor - Injector(carrier interface{}) Injector + // CarrierExtractor creates an object that implements Extractor interface. + // It binds provided carrier to the object. + CarrierExtractor(carrier interface{}) Extractor + + // CarrierInjector creates an object that implements Injector interface. + // It binds provided carrier to the object. + CarrierInjector(carrier interface{}) Injector } type Injector interface { diff --git a/propagation/http_textformat_propagator.go b/propagation/http_trace_context_propagator.go similarity index 75% rename from propagation/http_textformat_propagator.go rename to propagation/http_trace_context_propagator.go index fcd95206f4e..24372ccf70d 100644 --- a/propagation/http_textformat_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -39,57 +39,57 @@ type httpTraceContextPropagator struct{} var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} -// Extractor implements Extractor method of TextFormatPropagator interface. +// CarrierExtractor implements CarrierExtractor method of TextFormatPropagator interface. // -// It creates Extractor object and binds carrier to the object. The carrier +// It creates CarrierExtractor object and binds carrier to the object. The carrier // is expected to be *http.Request. If the carrier type is not *http.Request -// then an empty extractor is returned which will extract nothing. -func (t httpTraceContextPropagator) Extractor(carrier interface{}) apipropagation.Extractor { +// then an empty extractor. Extract method on empty extractor does nothing. +func (hp httpTraceContextPropagator) CarrierExtractor(carrier interface{}) apipropagation.Extractor { req, ok := carrier.(*http.Request) if ok { - return textFormatExtractor{req: req} + return traceContextExtractor{req: req} } - return textFormatExtractor{} + return traceContextExtractor{} } -// Injector implements Injector method of TextFormatPropagator interface. +// CarrierInjector implements CarrierInjector method of TextFormatPropagator interface. // -// It creates Injector object and binds carrier to the object. The carrier +// It creates CarrierInjector object and binds carrier to the object. The carrier // is expected to be of type *http.Request. If the carrier type is not *http.Request -// then an empty injector is returned which will inject nothing. -func (t httpTraceContextPropagator) Injector(carrier interface{}) apipropagation.Injector { +// then an empty injector is returned. Inject method on empty injector does nothing. +func (hp httpTraceContextPropagator) CarrierInjector(carrier interface{}) apipropagation.Injector { req, ok := carrier.(*http.Request) if ok { - return textFormatInjector{req: req} + return traceContextInjector{req: req} } - return textFormatInjector{} + return traceContextInjector{} } // HttpTraceContextPropagator creates a new propagator that propagates SpanContext // in W3C TraceContext format. // -// The propagator is then used to create Injector and Extractor associated with a +// The propagator is then used to create CarrierInjector and CarrierExtractor associated with a // specific request. Injectors and Extractors respectively provides method to // inject and extract SpanContext into/from the http request. Inject method encodes // SpanContext into W3C TraceContext Header and injects the header in the request. -// Extract extracts the header and decodes SpanContext. +// Extract method extracts the header and decodes SpanContext. func HttpTraceContextPropagator() httpTraceContextPropagator { return httpTraceContextPropagator{} } -type textFormatExtractor struct { +type traceContextExtractor struct { req *http.Request } -var _ apipropagation.Extractor = textFormatExtractor{} +var _ apipropagation.Extractor = traceContextExtractor{} -// Extract implements Extract method of trace.Extractor interface. It extracts +// Extract implements Extract method of propagation.Extractor interface. It extracts // W3C TraceContext Header and decodes SpanContext from the Header. -func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { - if tfe.req == nil { +func (tce traceContextExtractor) Extract() (sc core.SpanContext, tm tag.Map) { + if tce.req == nil { return core.EmptySpanContext(), tag.NewEmptyMap() } - h, ok := getRequestHeader(tfe.req, traceparentHeader, false) + h, ok := getRequestHeader(tce.req, traceparentHeader, false) if !ok { return core.EmptySpanContext(), tag.NewEmptyMap() } @@ -153,17 +153,17 @@ func (tfe textFormatExtractor) Extract() (sc core.SpanContext, tm tag.Map) { return sc, tag.NewEmptyMap() } -type textFormatInjector struct { +type traceContextInjector struct { req *http.Request } -var _ apipropagation.Injector = textFormatInjector{} +var _ apipropagation.Injector = traceContextInjector{} // Inject implements Inject method of propagation.Injector interface. It encodes // SpanContext into W3C TraceContext Header and injects the header into // the associated request. -func (tfi textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { - if tfi.req == nil { +func (tci traceContextInjector) Inject(sc core.SpanContext, tm tag.Map) { + if tci.req == nil { return } if sc.IsValid() { @@ -173,7 +173,7 @@ func (tfi textFormatInjector) Inject(sc core.SpanContext, tm tag.Map) { sc.TraceID.Low, sc.SpanID, sc.TraceOptions) - tfi.req.Header.Set(traceparentHeader, h) + tci.req.Header.Set(traceparentHeader, h) } // TODO: [rghetia] add tag.Map (distributed context) injection } diff --git a/propagation/http_textformat_propagator_test.go b/propagation/http_trace_context_propagator_test.go similarity index 95% rename from propagation/http_textformat_propagator_test.go rename to propagation/http_trace_context_propagator_test.go index 943e9b85074..36653dccbc6 100644 --- a/propagation/http_textformat_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -76,7 +76,7 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) req.Header.Set("traceparent", tt.header) - e := propagator.Extractor(req) + e := propagator.CarrierExtractor(req) gotSc, _ := e.Extract() if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { @@ -102,7 +102,7 @@ func TestExtractTraceContextFromInvalidCarrier(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - e := propagator.Extractor(tt.header) + e := propagator.CarrierExtractor(tt.header) gotSc, _ := e.Extract() if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) @@ -144,7 +144,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) - i := propagator.Injector(req) + i := propagator.CarrierInjector(req) i.Inject(tt.sc, tag.NewEmptyMap()) gotHeader := req.Header.Get("traceparent") @@ -162,7 +162,7 @@ func TestInjectTraceContextToInvalidCarrier(t *testing.T) { sc core.SpanContext }{ { - name: "valid spancontext to invalid carrier", + name: "valid spancontext to invalid carrier does nothing.", sc: core.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -172,7 +172,7 @@ func TestInjectTraceContextToInvalidCarrier(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - i := propagator.Injector("") + i := propagator.CarrierInjector("") i.Inject(tt.sc, tag.NewEmptyMap()) }) } From 034c2fcc865297042952de114d33dfd7bdd3c1b4 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Mon, 26 Aug 2019 10:36:37 -0700 Subject: [PATCH 12/28] add doc.go --- api/propagation/doc.go | 16 ++++++++++++++++ api/propagation/propagator.go | 4 +--- propagation/doc.go | 16 ++++++++++++++++ propagation/http_trace_context_propagator.go | 4 +--- .../http_trace_context_propagator_test.go | 4 ++-- 5 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 api/propagation/doc.go create mode 100644 propagation/doc.go diff --git a/api/propagation/doc.go b/api/propagation/doc.go new file mode 100644 index 00000000000..93217e6bbd6 --- /dev/null +++ b/api/propagation/doc.go @@ -0,0 +1,16 @@ +// Copyright 2019, 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 propagation contains interface definition for Binary and TextFormat propagators. +package propagation // import "go.opentelemetry.io/api/propagation" diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index 0dac763c695..d6879608481 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package tracecontext contains HTTP propagator for TraceContext standard. -// See https://github.com/w3c/distributed-tracing for more information. -package propagation // import "go.opentelemetry.io/api/propagation" +package propagation import ( "go.opentelemetry.io/api/core" diff --git a/propagation/doc.go b/propagation/doc.go new file mode 100644 index 00000000000..72296b3b6fc --- /dev/null +++ b/propagation/doc.go @@ -0,0 +1,16 @@ +// Copyright 2019, 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 propagation contains propagators for different format and carriers. +package propagation // import "go.opentelemetry.io/propagation" diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 24372ccf70d..c9adc5858d1 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package tracecontext contains HTTP propagator for TraceContext standard. -// See https://github.com/w3c/distributed-tracing for more information. -package propagation // import "go.opentelemetry.io/propagation" +package propagation import ( "encoding/hex" diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 36653dccbc6..db2ca545130 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -18,10 +18,10 @@ import ( "net/http" "testing" - "go.opentelemetry.io/api/tag" - "github.com/google/go-cmp/cmp" + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/tag" "go.opentelemetry.io/propagation" ) From 2b279afc65a1076b802a08213b818060a61f1e08 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Tue, 27 Aug 2019 12:16:35 -0700 Subject: [PATCH 13/28] update doc. --- api/propagation/propagator.go | 21 +++++--- propagation/http_trace_context_propagator.go | 52 +++++++++++--------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index d6879608481..511116578d9 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -19,12 +19,17 @@ import ( "go.opentelemetry.io/api/tag" ) -// TextFormatPropagator is an interface that specifies methods to create CarrierInjector -// and CarrierExtractor objects. These methods bind given carrier to the created object. -// CarrierInjector object implements Inject method to inject -// SpanContext and tag.Map as a text format into carrier like HTTP request. -// Similarly, CarrierExtractor object implements Extract method to extract SpanContext -// encoded in text format from a carrier like HTTP request. +// TextFormatPropagator is an interface that specifies methods to create objects +// that encodes/decodes into/from text format representation of SpanContext and tag.Map. +// +// CarrierInjector method creates an Injector object and binds the carrier to the object. +// Injector object provides Inject method to inject SpanContext and tag.Map after serializing into +// a text format associated with the propagator. +// +// Similarly, CarrierExtractor method creates an Extractor object and binds the carrier to the +// object. Extractor object provides Extract method to extract text formatted de-serialized +// SpanContext and tag.Map +// // Typically, a plugin for transport like HTTP uses this interface to allow user // to configure appropriate text format propagators. type TextFormatPropagator interface { @@ -38,14 +43,14 @@ type TextFormatPropagator interface { } type Injector interface { - // Inject serializes span context and tag.Map and inserts them in to + // Inject encodes span context and tag.Map and inserts them in to // carrier associated with the injector. For example in case of http request, // span context could be added to the request (carrier) as W3C Trace context header. Inject(core.SpanContext, tag.Map) } type Extractor interface { - // Extract de-serializes span context and tag.Map from a carrier associated with the + // Extract decodes span context and tag.Map from a carrier associated with the // extractor. For example in case of http request, span context could be extracted // from the W3C Trace context header. Extract() (core.SpanContext, tag.Map) diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index c9adc5858d1..4077edf6906 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -37,7 +37,7 @@ type httpTraceContextPropagator struct{} var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} -// CarrierExtractor implements CarrierExtractor method of TextFormatPropagator interface. +// CarrierExtractor implements TextFormatPropagator interface. // // It creates CarrierExtractor object and binds carrier to the object. The carrier // is expected to be *http.Request. If the carrier type is not *http.Request @@ -50,7 +50,7 @@ func (hp httpTraceContextPropagator) CarrierExtractor(carrier interface{}) apipr return traceContextExtractor{} } -// CarrierInjector implements CarrierInjector method of TextFormatPropagator interface. +// CarrierInjector implements TextFormatPropagator interface. // // It creates CarrierInjector object and binds carrier to the object. The carrier // is expected to be of type *http.Request. If the carrier type is not *http.Request @@ -63,14 +63,14 @@ func (hp httpTraceContextPropagator) CarrierInjector(carrier interface{}) apipro return traceContextInjector{} } -// HttpTraceContextPropagator creates a new propagator that propagates SpanContext +// HttpTraceContextPropagator creates a new text format propagator that propagates SpanContext // in W3C TraceContext format. // // The propagator is then used to create CarrierInjector and CarrierExtractor associated with a // specific request. Injectors and Extractors respectively provides method to // inject and extract SpanContext into/from the http request. Inject method encodes -// SpanContext into W3C TraceContext Header and injects the header in the request. -// Extract method extracts the header and decodes SpanContext. +// SpanContext and tag.Map into W3C TraceContext Header and injects the header in the request. +// Extract method extracts the header and decodes SpanContext and tag.Map. func HttpTraceContextPropagator() httpTraceContextPropagator { return httpTraceContextPropagator{} } @@ -81,84 +81,90 @@ type traceContextExtractor struct { var _ apipropagation.Extractor = traceContextExtractor{} -// Extract implements Extract method of propagation.Extractor interface. It extracts -// W3C TraceContext Header and decodes SpanContext from the Header. +// Extract implements Extractor interface. +// +// It extracts W3C TraceContext Header and then decodes SpanContext and tag.Map from the Header. func (tce traceContextExtractor) Extract() (sc core.SpanContext, tm tag.Map) { if tce.req == nil { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } h, ok := getRequestHeader(tce.req, traceparentHeader, false) if !ok { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } sections := strings.Split(h, "-") if len(sections) < 4 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } if len(sections[0]) != 2 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } ver, err := hex.DecodeString(sections[0]) if err != nil { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } version := int(ver[0]) if version > maxVersion { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } if version == 0 && len(sections) != 4 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } if len(sections[1]) != 32 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } result, err := strconv.ParseUint(sections[1][0:16], 16, 64) if err != nil { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } sc.TraceID.High = result result, err = strconv.ParseUint(sections[1][16:32], 16, 64) if err != nil { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } sc.TraceID.Low = result if len(sections[2]) != 16 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } result, err = strconv.ParseUint(sections[2][0:], 16, 64) if err != nil { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } sc.SpanID = result opts, err := hex.DecodeString(sections[3]) if err != nil || len(opts) < 1 { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } sc.TraceOptions = opts[0] if !sc.IsValid() { - return core.EmptySpanContext(), tag.NewEmptyMap() + return noExtract() } // TODO: [rghetia] add tag.Map (distributed context) extraction return sc, tag.NewEmptyMap() } +func noExtract() (core.SpanContext, tag.Map) { + return core.EmptySpanContext(), tag.NewEmptyMap() +} + type traceContextInjector struct { req *http.Request } var _ apipropagation.Injector = traceContextInjector{} -// Inject implements Inject method of propagation.Injector interface. It encodes -// SpanContext into W3C TraceContext Header and injects the header into +// Inject implements Injector interface. +// +// It encodes SpanContext and tag.Map into W3C TraceContext Header and injects the header into // the associated request. func (tci traceContextInjector) Inject(sc core.SpanContext, tm tag.Map) { if tci.req == nil { From 8d3cced7d4ebd868fe858229490580c9668d3ca7 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Tue, 27 Aug 2019 16:40:42 -0700 Subject: [PATCH 14/28] add noop propagator. --- api/propagation/noop_propagator.go | 53 ++++++++++++++++++++ propagation/http_trace_context_propagator.go | 20 ++++---- 2 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 api/propagation/noop_propagator.go diff --git a/api/propagation/noop_propagator.go b/api/propagation/noop_propagator.go new file mode 100644 index 00000000000..802064b7ec4 --- /dev/null +++ b/api/propagation/noop_propagator.go @@ -0,0 +1,53 @@ +// Copyright 2019, 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 propagation + +import ( + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/tag" +) + +// NoopTextFormatPropagator implements TextFormatPropagator that does nothing. +type NoopTextFormatPropagator struct{} + +var _ TextFormatPropagator = NoopTextFormatPropagator{} + +// CarrierExtractor returns NoopExtractor +func (ntp NoopTextFormatPropagator) CarrierExtractor(carrier interface{}) Extractor { + return NoopExtractor{} +} + +// CarrierInjector returns NoopInjector +func (ntp NoopTextFormatPropagator) CarrierInjector(carrier interface{}) Injector { + return NoopInjector{} +} + +// NoopInjector implements Injector interface that does nothing. +type NoopInjector struct{} + +var _ Injector = NoopInjector{} + +func (ni NoopInjector) Inject(sc core.SpanContext, tm tag.Map) { +} + +// NoopExtractor implements Extractor interface that does nothing. +type NoopExtractor struct{} + +var _ Extractor = NoopExtractor{} + +// Extract method always returns Invalid SpanContext and empty tag.Map +func (ne NoopExtractor) Extract() (core.SpanContext, tag.Map) { + return core.EmptySpanContext(), tag.NewEmptyMap() +} diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 4077edf6906..cef01470361 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -40,27 +40,27 @@ var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} // CarrierExtractor implements TextFormatPropagator interface. // // It creates CarrierExtractor object and binds carrier to the object. The carrier -// is expected to be *http.Request. If the carrier type is not *http.Request -// then an empty extractor. Extract method on empty extractor does nothing. +// is expected to be *http.Request. If the carrier is nil or its type is not *http.Request +// then a NoopExtractor is returned. func (hp httpTraceContextPropagator) CarrierExtractor(carrier interface{}) apipropagation.Extractor { - req, ok := carrier.(*http.Request) - if ok { + req, _ := carrier.(*http.Request) + if req != nil { return traceContextExtractor{req: req} } - return traceContextExtractor{} + return apipropagation.NoopExtractor{} } // CarrierInjector implements TextFormatPropagator interface. // // It creates CarrierInjector object and binds carrier to the object. The carrier -// is expected to be of type *http.Request. If the carrier type is not *http.Request -// then an empty injector is returned. Inject method on empty injector does nothing. +// is expected to be of type *http.Request. If the carrier is nil or its type is not *http.Request +// then a NoopInjector is returned. func (hp httpTraceContextPropagator) CarrierInjector(carrier interface{}) apipropagation.Injector { - req, ok := carrier.(*http.Request) - if ok { + req, _ := carrier.(*http.Request) + if req != nil { return traceContextInjector{req: req} } - return traceContextInjector{} + return apipropagation.NoopInjector{} } // HttpTraceContextPropagator creates a new text format propagator that propagates SpanContext From a55726d5a794ae50cbead1849cc9ae0f45289d21 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Tue, 3 Sep 2019 13:26:43 -0700 Subject: [PATCH 15/28] add new propagation api with Supplier interface. - added Default Tracer which simply propagates SpanContext. - added CopyOfRemote option to simply create remote span. --- api/propagation/propagator.go | 33 ++++++ api/trace/api.go | 16 ++- api/trace/default_span.go | 85 ++++++++++++++ api/trace/default_tracer.go | 57 +++++++++ propagation/http_trace_context_propagator.go | 89 +++++++++++++++ .../http_trace_context_propagator_test.go | 108 ++++++++++++++++++ sdk/trace/span.go | 10 ++ 7 files changed, 394 insertions(+), 4 deletions(-) create mode 100644 api/trace/default_span.go create mode 100644 api/trace/default_tracer.go diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index 511116578d9..eb5d1d442bd 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -15,6 +15,8 @@ package propagation import ( + "context" + "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/tag" ) @@ -55,3 +57,34 @@ type Extractor interface { // from the W3C Trace context header. Extract() (core.SpanContext, tag.Map) } + +// Propagator is an interface that specifies methods to inject and extract SpanContext +// into/from a carrier using Supplier interface. +// For example, HTTP Trace Context Propagator would encode SpanContext into W3C Trace +// Context Header and set the header into HttpRequest. +type Propagator interface { + // Inject method retrieves current SpanContext from the ctx, encodes it into propagator + // specific format and then injects the encoded SpanContext using supplier into a carrier + // associated with the supplier. + Inject(ctx context.Context, supplier Supplier) + + // Extract method retrieves SpanContext using supplier from the associated carrier with + // the supplier. It decodes the SpanContext and it creates remote span using registered + // tracer. + Extract(ctx context.Context, supplier Supplier) context.Context + + // GetAllKeys returns all the keys that this propagator injects/extracts into/from a + // carrier. The use cases for this are + // * allow pre-allocation of fields, especially in systems like gRPC Metadata + // * allow a single-pass over an iterator (ex OpenTracing has no getter in TextMap) + GetAllKeys() []string +} + +// Supplier is an interface that specifies methods to retrieve and store +// value for a key to an associated carrier. +// Get method retrieves the value for a given key. +// Set method stores the value for a given key. +type Supplier interface { + Get(key string) string + Set(key string, value string) +} diff --git a/api/trace/api.go b/api/trace/api.go index 68cba1ea990..e9e185a9365 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -105,10 +105,11 @@ type SpanOption func(*SpanOptions) // SpanOptions provides options to set properties of span at the time of starting // a new span. type SpanOptions struct { - Attributes []core.KeyValue - StartTime time.Time - Reference Reference - RecordEvent bool + Attributes []core.KeyValue + StartTime time.Time + Reference Reference + RecordEvent bool + RemoteSpanContext core.SpanContext } // Reference is used to establish relationship between newly created span and the @@ -173,6 +174,13 @@ func WithRecordEvents() SpanOption { } } +// CopyOfRemote allows to create an inactive span mirroring remote span. +func CopyOfRemote(sc core.SpanContext) SpanOption { + return func(o *SpanOptions) { + o.RemoteSpanContext = sc + } +} + // ChildOf. TODO: do we need this?. func ChildOf(sc core.SpanContext) SpanOption { return func(o *SpanOptions) { diff --git a/api/trace/default_span.go b/api/trace/default_span.go new file mode 100644 index 00000000000..a7043548c5b --- /dev/null +++ b/api/trace/default_span.go @@ -0,0 +1,85 @@ +// Copyright 2019, 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 trace + +import ( + "context" + + "google.golang.org/grpc/codes" + + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/event" + "go.opentelemetry.io/api/tag" +) + +type DefaultSpan struct { + sc core.SpanContext +} + +var _ Span = (*DefaultSpan)(nil) + +// SpancContext returns an invalid span context. +func (ds *DefaultSpan) SpanContext() core.SpanContext { + if ds == nil { + core.EmptySpanContext() + } + return ds.sc +} + +// IsRecordingEvents always returns false for DefaultSpan. +func (ds *DefaultSpan) IsRecordingEvents() bool { + return false +} + +// SetStatus does nothing. +func (ds *DefaultSpan) SetStatus(status codes.Code) { +} + +// SetError does nothing. +func (ds *DefaultSpan) SetError(v bool) { +} + +// SetAttribute does nothing. +func (ds *DefaultSpan) SetAttribute(attribute core.KeyValue) { +} + +// SetAttributes does nothing. +func (ds *DefaultSpan) SetAttributes(attributes ...core.KeyValue) { +} + +// ModifyAttribute does nothing. +func (ds *DefaultSpan) ModifyAttribute(mutator tag.Mutator) { +} + +// ModifyAttributes does nothing. +func (ds *DefaultSpan) ModifyAttributes(mutators ...tag.Mutator) { +} + +// Finish does nothing. +func (ds *DefaultSpan) Finish() { +} + +// Tracer returns noop implementation of Tracer. +func (ds *DefaultSpan) Tracer() Tracer { + return NoopTracer{} +} + +// AddEvent does nothing. +func (ds *DefaultSpan) AddEvent(ctx context.Context, event event.Event) { +} + +// Event does nothing. +func (ds *DefaultSpan) Event(ctx context.Context, msg string, attrs ...core.KeyValue) { +} diff --git a/api/trace/default_tracer.go b/api/trace/default_tracer.go new file mode 100644 index 00000000000..2a2419f6754 --- /dev/null +++ b/api/trace/default_tracer.go @@ -0,0 +1,57 @@ +// Copyright 2019, 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 trace + +import ( + "context" + "go.opentelemetry.io/api/core" +) + +type DefaultTracer struct{} + +var _ Tracer = DefaultTracer{} + +// WithResources does nothing and returns noop implementation of Tracer. +func (t DefaultTracer) WithResources(attributes ...core.KeyValue) Tracer { + return t +} + +// WithComponent does nothing and returns noop implementation of Tracer. +func (t DefaultTracer) WithComponent(name string) Tracer { + return t +} + +// WithService does nothing and returns noop implementation of Tracer. +func (t DefaultTracer) WithService(name string) Tracer { + return t +} + +// WithSpan wraps around execution of func with noop span. +func (t DefaultTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { + return body(ctx) +} + +// Start starts a noop span. +func (DefaultTracer) Start(ctx context.Context, name string, o ...SpanOption) (context.Context, Span) { + var opts SpanOptions + for _, op := range o { + op(&opts) + } + if !opts.RemoteSpanContext.IsValid() { + return ctx, NoopSpan{} + } + span := &DefaultSpan{sc:opts.RemoteSpanContext} + return SetCurrentSpan(ctx, span), span +} diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index cef01470361..63da84020bd 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -15,6 +15,7 @@ package propagation import ( + "context" "encoding/hex" "fmt" "net/http" @@ -22,6 +23,8 @@ import ( "strconv" "strings" + "go.opentelemetry.io/api/trace" + "go.opentelemetry.io/api/core" apipropagation "go.opentelemetry.io/api/propagation" "go.opentelemetry.io/api/tag" @@ -36,6 +39,7 @@ const ( type httpTraceContextPropagator struct{} var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} +var _ apipropagation.Propagator = httpTraceContextPropagator{} // CarrierExtractor implements TextFormatPropagator interface. // @@ -63,6 +67,91 @@ func (hp httpTraceContextPropagator) CarrierInjector(carrier interface{}) apipro return apipropagation.NoopInjector{} } +func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipropagation.Supplier) { + sc := trace.CurrentSpan(ctx).SpanContext() + if sc.IsValid() { + h := fmt.Sprintf("%.2x-%.16x%.16x-%.16x-%.2x", + supportedVersion, + sc.TraceID.High, + sc.TraceID.Low, + sc.SpanID, + sc.TraceOptions) + supplier.Set(traceparentHeader, h) + } +} + +func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipropagation.Supplier) context.Context { + h := supplier.Get(traceparentHeader) + if h == "" { + return ctx + } + + sections := strings.Split(h, "-") + if len(sections) < 4 { + return ctx + } + + if len(sections[0]) != 2 { + return ctx + } + ver, err := hex.DecodeString(sections[0]) + if err != nil { + return ctx + } + version := int(ver[0]) + if version > maxVersion { + return ctx + } + + if version == 0 && len(sections) != 4 { + return ctx + } + + if len(sections[1]) != 32 { + return ctx + } + + result, err := strconv.ParseUint(sections[1][0:16], 16, 64) + if err != nil { + return ctx + } + var sc core.SpanContext + + sc.TraceID.High = result + + result, err = strconv.ParseUint(sections[1][16:32], 16, 64) + if err != nil { + return ctx + } + sc.TraceID.Low = result + + if len(sections[2]) != 16 { + return ctx + } + result, err = strconv.ParseUint(sections[2][0:], 16, 64) + if err != nil { + return ctx + } + sc.SpanID = result + + opts, err := hex.DecodeString(sections[3]) + if err != nil || len(opts) < 1 { + return ctx + } + sc.TraceOptions = opts[0] + + if !sc.IsValid() { + return ctx + } + + ctx, _ = trace.GlobalTracer().Start(ctx, "remote", trace.CopyOfRemote(sc)) + return ctx +} + +func (hp httpTraceContextPropagator) GetAllKeys() []string { + return nil +} + // HttpTraceContextPropagator creates a new text format propagator that propagates SpanContext // in W3C TraceContext format. // diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index db2ca545130..4624f6b6b83 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -15,9 +15,12 @@ package propagation_test import ( + "context" "net/http" "testing" + "go.opentelemetry.io/api/trace" + "github.com/google/go-cmp/cmp" "go.opentelemetry.io/api/core" @@ -86,6 +89,65 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { } } +func TestExtractTraceContextFromHTTPReq2(t *testing.T) { + trace.SetGlobalTracer(trace.DefaultTracer{}) + propagator := propagation.HttpTraceContextPropagator() + tests := []struct { + name string + header string + wantSc core.SpanContext + }{ + { + name: "future version", + header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + { + name: "zero trace ID and span ID", + header: "00-00000000000000000000000000000000-0000000000000000-01", + wantSc: core.EmptySpanContext(), + }, + { + name: "valid header", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + { + name: "missing options", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", + wantSc: core.EmptySpanContext(), + }, + { + name: "empty options", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", + wantSc: core.EmptySpanContext(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com", nil) + req.Header.Set("traceparent", tt.header) + + ctx := context.Background() + ctx = propagator.Extract(ctx, req.Header) + span := trace.CurrentSpan(ctx) + gotSc := span.SpanContext() + if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} + func TestExtractTraceContextFromInvalidCarrier(t *testing.T) { propagator := propagation.HttpTraceContextPropagator() tests := []struct { @@ -155,6 +217,52 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { } } +func TestInjectTraceContextToHTTPReq2(t *testing.T) { + trace.SetGlobalTracer(trace.DefaultTracer{}) + propagator := propagation.HttpTraceContextPropagator() + tests := []struct { + name string + sc core.SpanContext + wantHeader string + }{ + { + name: "valid spancontext, sampled", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + }, + { + name: "valid spancontext, not sampled", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", + }, + { + name: "invalid spancontext", + sc: core.EmptySpanContext(), + wantHeader: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com", nil) + ctx := context.Background() + ctx, _ = trace.GlobalTracer().Start(ctx, "inject", trace.CopyOfRemote(tt.sc)) + propagator.Inject(ctx, req.Header) + + gotHeader := req.Header.Get("traceparent") + if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} + func TestInjectTraceContextToInvalidCarrier(t *testing.T) { propagator := propagation.HttpTraceContextPropagator() tests := []struct { diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 38e6fd94931..3a8315a28d4 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -38,6 +38,9 @@ type span struct { mu sync.Mutex // protects the contents of *data (but not the pointer value.) spanContext core.SpanContext + // remote is true if span was created to mirror a remote span. + remote bool + // lruAttributes are capped at configured limit. When the capacity is reached an oldest entry // is removed to create room for a new entry. lruAttributes *lruMap @@ -297,6 +300,13 @@ func (s *span) addChild() { func startSpanInternal(name string, parent core.SpanContext, remoteParent bool, o apitrace.SpanOptions) *span { var noParent bool span := &span{} + + if o.RemoteSpanContext.IsValid() { + span.remote = true + span.spanContext = o.RemoteSpanContext + return span + } + span.spanContext = parent cfg := config.Load().(*Config) From 8fe108597fe6dad854cfdd04e5bd0c61a671418b Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 4 Sep 2019 16:37:02 -0700 Subject: [PATCH 16/28] remove old propagator. --- api/propagation/noop_propagator.go | 37 ++--- api/propagation/propagator.go | 40 ----- api/trace/default_tracer.go | 3 +- example/http/client/client.go | 9 +- example/http/server/server.go | 5 +- plugin/httptrace/api.go | 6 +- plugin/httptrace/httptrace.go | 45 ++--- propagation/http_trace_context_propagator.go | 155 ------------------ .../http_trace_context_propagator_test.go | 149 ----------------- 9 files changed, 35 insertions(+), 414 deletions(-) diff --git a/api/propagation/noop_propagator.go b/api/propagation/noop_propagator.go index 802064b7ec4..5d855ade93e 100644 --- a/api/propagation/noop_propagator.go +++ b/api/propagation/noop_propagator.go @@ -15,39 +15,24 @@ package propagation import ( - "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/tag" + "context" ) // NoopTextFormatPropagator implements TextFormatPropagator that does nothing. type NoopTextFormatPropagator struct{} -var _ TextFormatPropagator = NoopTextFormatPropagator{} +var _ Propagator = NoopTextFormatPropagator{} -// CarrierExtractor returns NoopExtractor -func (ntp NoopTextFormatPropagator) CarrierExtractor(carrier interface{}) Extractor { - return NoopExtractor{} +// Inject does nothing. +func (np NoopTextFormatPropagator) Inject(ctx context.Context, supplier Supplier) { } -// CarrierInjector returns NoopInjector -func (ntp NoopTextFormatPropagator) CarrierInjector(carrier interface{}) Injector { - return NoopInjector{} +// Extract does nothing. +func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) context.Context { + return ctx } -// NoopInjector implements Injector interface that does nothing. -type NoopInjector struct{} - -var _ Injector = NoopInjector{} - -func (ni NoopInjector) Inject(sc core.SpanContext, tm tag.Map) { -} - -// NoopExtractor implements Extractor interface that does nothing. -type NoopExtractor struct{} - -var _ Extractor = NoopExtractor{} - -// Extract method always returns Invalid SpanContext and empty tag.Map -func (ne NoopExtractor) Extract() (core.SpanContext, tag.Map) { - return core.EmptySpanContext(), tag.NewEmptyMap() -} +// GetAllKeys returns empty list of strings. +func (np NoopTextFormatPropagator) GetAllKeys() []string { + return []string{} +} \ No newline at end of file diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index eb5d1d442bd..16be9352b91 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -16,48 +16,8 @@ package propagation import ( "context" - - "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/tag" ) -// TextFormatPropagator is an interface that specifies methods to create objects -// that encodes/decodes into/from text format representation of SpanContext and tag.Map. -// -// CarrierInjector method creates an Injector object and binds the carrier to the object. -// Injector object provides Inject method to inject SpanContext and tag.Map after serializing into -// a text format associated with the propagator. -// -// Similarly, CarrierExtractor method creates an Extractor object and binds the carrier to the -// object. Extractor object provides Extract method to extract text formatted de-serialized -// SpanContext and tag.Map -// -// Typically, a plugin for transport like HTTP uses this interface to allow user -// to configure appropriate text format propagators. -type TextFormatPropagator interface { - // CarrierExtractor creates an object that implements Extractor interface. - // It binds provided carrier to the object. - CarrierExtractor(carrier interface{}) Extractor - - // CarrierInjector creates an object that implements Injector interface. - // It binds provided carrier to the object. - CarrierInjector(carrier interface{}) Injector -} - -type Injector interface { - // Inject encodes span context and tag.Map and inserts them in to - // carrier associated with the injector. For example in case of http request, - // span context could be added to the request (carrier) as W3C Trace context header. - Inject(core.SpanContext, tag.Map) -} - -type Extractor interface { - // Extract decodes span context and tag.Map from a carrier associated with the - // extractor. For example in case of http request, span context could be extracted - // from the W3C Trace context header. - Extract() (core.SpanContext, tag.Map) -} - // Propagator is an interface that specifies methods to inject and extract SpanContext // into/from a carrier using Supplier interface. // For example, HTTP Trace Context Propagator would encode SpanContext into W3C Trace diff --git a/api/trace/default_tracer.go b/api/trace/default_tracer.go index 2a2419f6754..3ac98fa0149 100644 --- a/api/trace/default_tracer.go +++ b/api/trace/default_tracer.go @@ -16,6 +16,7 @@ package trace import ( "context" + "go.opentelemetry.io/api/core" ) @@ -52,6 +53,6 @@ func (DefaultTracer) Start(ctx context.Context, name string, o ...SpanOption) (c if !opts.RemoteSpanContext.IsValid() { return ctx, NoopSpan{} } - span := &DefaultSpan{sc:opts.RemoteSpanContext} + span := &DefaultSpan{sc: opts.RemoteSpanContext} return SetCurrentSpan(ctx, span), span } diff --git a/example/http/client/client.go b/example/http/client/client.go index 5b61a815f95..8fe9d7a5f34 100644 --- a/example/http/client/client.go +++ b/example/http/client/client.go @@ -17,6 +17,7 @@ package main import ( "context" "fmt" + "go.opentelemetry.io/plugin/httptrace" "io/ioutil" "net/http" @@ -25,7 +26,7 @@ import ( "go.opentelemetry.io/api/key" "go.opentelemetry.io/api/tag" "go.opentelemetry.io/api/trace" - "go.opentelemetry.io/plugin/httptrace" + "go.opentelemetry.io/propagation" ) var ( @@ -50,9 +51,9 @@ func main() { func(ctx context.Context) error { req, _ := http.NewRequest("GET", "http://localhost:7777/hello", nil) - ctx, req, inj := httptrace.W3C(ctx, req) - - inj.Inject(trace.CurrentSpan(ctx).SpanContext(), nil) + propagator := propagation.HttpTraceContextPropagator() + ctx, req = httptrace.W3C(ctx, req) + propagator.Inject(ctx, req.Header) res, err := client.Do(req) if err != nil { diff --git a/example/http/server/server.go b/example/http/server/server.go index dbab3b10113..db4a152e4fb 100644 --- a/example/http/server/server.go +++ b/example/http/server/server.go @@ -35,9 +35,9 @@ var ( func main() { helloHandler := func(w http.ResponseWriter, req *http.Request) { - attrs, tags, spanCtx := httptrace.Extract(req) + attrs, tags, ctx := httptrace.Extract(req.Context(), req) - req = req.WithContext(tag.WithMap(req.Context(), tag.NewMap(tag.MapUpdate{ + req = req.WithContext(tag.WithMap(ctx, tag.NewMap(tag.MapUpdate{ MultiKV: tags, }))) @@ -45,7 +45,6 @@ func main() { req.Context(), "hello", trace.WithAttributes(attrs...), - trace.ChildOf(spanCtx), ) defer span.Finish() diff --git a/plugin/httptrace/api.go b/plugin/httptrace/api.go index f1e0a8f4178..3fb84c30ef0 100644 --- a/plugin/httptrace/api.go +++ b/plugin/httptrace/api.go @@ -18,12 +18,10 @@ import ( "context" "net/http" "net/http/httptrace" - - "go.opentelemetry.io/api/propagation" ) // Client -func W3C(ctx context.Context, req *http.Request) (context.Context, *http.Request, propagation.Injector) { +func W3C(ctx context.Context, req *http.Request) (context.Context, *http.Request) { t := newClientTracer(ctx) t.GetConn = t.getConn @@ -45,5 +43,5 @@ func W3C(ctx context.Context, req *http.Request) (context.Context, *http.Request ctx = httptrace.WithClientTrace(ctx, &t.ClientTrace) req = req.WithContext(ctx) - return ctx, req, hinjector{req} + return ctx, req } diff --git a/plugin/httptrace/httptrace.go b/plugin/httptrace/httptrace.go index c2287c297a3..0cca193f443 100644 --- a/plugin/httptrace/httptrace.go +++ b/plugin/httptrace/httptrace.go @@ -15,15 +15,17 @@ package httptrace import ( + "context" "encoding/binary" "net/http" + "go.opentelemetry.io/api/trace" + "github.com/lightstep/tracecontext.go" - "github.com/lightstep/tracecontext.go/tracestate" "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/key" - "go.opentelemetry.io/api/tag" + "go.opentelemetry.io/propagation" ) const ( @@ -34,46 +36,25 @@ var ( HostKey = key.New("http.host") URLKey = key.New("http.url") - encoding = binary.BigEndian + encoding = binary.BigEndian + propagator = propagation.HttpTraceContextPropagator() ) // Returns the Attributes, Context Tags, and SpanContext that were encoded by Inject. -func Extract(req *http.Request) ([]core.KeyValue, []core.KeyValue, core.SpanContext) { - tc, err := tracecontext.FromHeaders(req.Header) - - if err != nil { - return nil, nil, core.SpanContext{} - } - - var sc core.SpanContext - sc.SpanID = encoding.Uint64(tc.TraceParent.SpanID[0:8]) - sc.TraceID.High = encoding.Uint64(tc.TraceParent.TraceID[0:8]) - sc.TraceID.Low = encoding.Uint64(tc.TraceParent.TraceID[8:16]) +func Extract(ctx context.Context, req *http.Request) ([]core.KeyValue, []core.KeyValue, context.Context) { + ctx = propagator.Extract(ctx, req.Header) attrs := []core.KeyValue{ URLKey.String(req.URL.String()), // Etc. } - var tags []core.KeyValue - - for _, ts := range tc.TraceState { - if ts.Vendor != Vendor { - continue - } - // TODO: max-hops, type conversion questions answered, - // case-conversion questions. - tags = append(tags, key.New(ts.Tenant).String(ts.Value)) - } - - return attrs, tags, sc -} - -type hinjector struct { - *http.Request + return attrs, nil, ctx } -func (h hinjector) Inject(sc core.SpanContext, tags tag.Map) { +func Inject(ctx context.Context, req *http.Request) { + propagator.Inject(ctx, req.Header) + sc := trace.CurrentSpan(ctx).SpanContext() var tc tracecontext.TraceContext var sid [8]byte var tid [16]byte @@ -97,5 +78,5 @@ func (h hinjector) Inject(sc core.SpanContext, tags tag.Map) { return true }) - tc.SetHeaders(h.Header) + tc.SetHeaders(req.Header) } diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 63da84020bd..971bfa4b853 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -18,8 +18,6 @@ import ( "context" "encoding/hex" "fmt" - "net/http" - "net/textproto" "strconv" "strings" @@ -27,7 +25,6 @@ import ( "go.opentelemetry.io/api/core" apipropagation "go.opentelemetry.io/api/propagation" - "go.opentelemetry.io/api/tag" ) const ( @@ -38,35 +35,8 @@ const ( type httpTraceContextPropagator struct{} -var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} var _ apipropagation.Propagator = httpTraceContextPropagator{} -// CarrierExtractor implements TextFormatPropagator interface. -// -// It creates CarrierExtractor object and binds carrier to the object. The carrier -// is expected to be *http.Request. If the carrier is nil or its type is not *http.Request -// then a NoopExtractor is returned. -func (hp httpTraceContextPropagator) CarrierExtractor(carrier interface{}) apipropagation.Extractor { - req, _ := carrier.(*http.Request) - if req != nil { - return traceContextExtractor{req: req} - } - return apipropagation.NoopExtractor{} -} - -// CarrierInjector implements TextFormatPropagator interface. -// -// It creates CarrierInjector object and binds carrier to the object. The carrier -// is expected to be of type *http.Request. If the carrier is nil or its type is not *http.Request -// then a NoopInjector is returned. -func (hp httpTraceContextPropagator) CarrierInjector(carrier interface{}) apipropagation.Injector { - req, _ := carrier.(*http.Request) - if req != nil { - return traceContextInjector{req: req} - } - return apipropagation.NoopInjector{} -} - func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipropagation.Supplier) { sc := trace.CurrentSpan(ctx).SpanContext() if sc.IsValid() { @@ -163,128 +133,3 @@ func (hp httpTraceContextPropagator) GetAllKeys() []string { func HttpTraceContextPropagator() httpTraceContextPropagator { return httpTraceContextPropagator{} } - -type traceContextExtractor struct { - req *http.Request -} - -var _ apipropagation.Extractor = traceContextExtractor{} - -// Extract implements Extractor interface. -// -// It extracts W3C TraceContext Header and then decodes SpanContext and tag.Map from the Header. -func (tce traceContextExtractor) Extract() (sc core.SpanContext, tm tag.Map) { - if tce.req == nil { - return noExtract() - } - h, ok := getRequestHeader(tce.req, traceparentHeader, false) - if !ok { - return noExtract() - } - sections := strings.Split(h, "-") - if len(sections) < 4 { - return noExtract() - } - - if len(sections[0]) != 2 { - return noExtract() - } - ver, err := hex.DecodeString(sections[0]) - if err != nil { - return noExtract() - } - version := int(ver[0]) - if version > maxVersion { - return noExtract() - } - - if version == 0 && len(sections) != 4 { - return noExtract() - } - - if len(sections[1]) != 32 { - return noExtract() - } - - result, err := strconv.ParseUint(sections[1][0:16], 16, 64) - if err != nil { - return noExtract() - } - sc.TraceID.High = result - - result, err = strconv.ParseUint(sections[1][16:32], 16, 64) - if err != nil { - return noExtract() - } - sc.TraceID.Low = result - - if len(sections[2]) != 16 { - return noExtract() - } - result, err = strconv.ParseUint(sections[2][0:], 16, 64) - if err != nil { - return noExtract() - } - sc.SpanID = result - - opts, err := hex.DecodeString(sections[3]) - if err != nil || len(opts) < 1 { - return noExtract() - } - sc.TraceOptions = opts[0] - - if !sc.IsValid() { - return noExtract() - } - - // TODO: [rghetia] add tag.Map (distributed context) extraction - return sc, tag.NewEmptyMap() -} - -func noExtract() (core.SpanContext, tag.Map) { - return core.EmptySpanContext(), tag.NewEmptyMap() -} - -type traceContextInjector struct { - req *http.Request -} - -var _ apipropagation.Injector = traceContextInjector{} - -// Inject implements Injector interface. -// -// It encodes SpanContext and tag.Map into W3C TraceContext Header and injects the header into -// the associated request. -func (tci traceContextInjector) Inject(sc core.SpanContext, tm tag.Map) { - if tci.req == nil { - return - } - if sc.IsValid() { - h := fmt.Sprintf("%.2x-%.16x%.16x-%.16x-%.2x", - supportedVersion, - sc.TraceID.High, - sc.TraceID.Low, - sc.SpanID, - sc.TraceOptions) - tci.req.Header.Set(traceparentHeader, h) - } - // TODO: [rghetia] add tag.Map (distributed context) injection -} - -// getRequestHeader returns a combined header field according to RFC7230 section 3.2.2. -// If commaSeparated is true, multiple header fields with the same field name using be -// combined using ",". -// If no header was found using the given name, "ok" would be false. -// If more than one headers was found using the given name, while commaSeparated is false, -// "ok" would be false. -func getRequestHeader(req *http.Request, name string, commaSeparated bool) (hdr string, ok bool) { - v := req.Header[textproto.CanonicalMIMEHeaderKey(name)] - switch len(v) { - case 0: - return "", false - case 1: - return v[0], true - default: - return strings.Join(v, ","), commaSeparated - } -} diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 4624f6b6b83..4120540e384 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -24,7 +24,6 @@ import ( "github.com/google/go-cmp/cmp" "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/tag" "go.opentelemetry.io/propagation" ) @@ -34,62 +33,6 @@ var ( ) func TestExtractTraceContextFromHTTPReq(t *testing.T) { - propagator := propagation.HttpTraceContextPropagator() - tests := []struct { - name string - header string - wantSc core.SpanContext - }{ - { - name: "future version", - header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - wantSc: core.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceOptions: core.TraceOptionSampled, - }, - }, - { - name: "zero trace ID and span ID", - header: "00-00000000000000000000000000000000-0000000000000000-01", - wantSc: core.EmptySpanContext(), - }, - { - name: "valid header", - header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - wantSc: core.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceOptions: core.TraceOptionSampled, - }, - }, - { - name: "missing options", - header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", - wantSc: core.EmptySpanContext(), - }, - { - name: "empty options", - header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", - wantSc: core.EmptySpanContext(), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest("GET", "http://example.com", nil) - req.Header.Set("traceparent", tt.header) - e := propagator.CarrierExtractor(req) - - gotSc, _ := e.Extract() - if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { - t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) - } - }) - } -} - -func TestExtractTraceContextFromHTTPReq2(t *testing.T) { trace.SetGlobalTracer(trace.DefaultTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { @@ -148,76 +91,7 @@ func TestExtractTraceContextFromHTTPReq2(t *testing.T) { } } -func TestExtractTraceContextFromInvalidCarrier(t *testing.T) { - propagator := propagation.HttpTraceContextPropagator() - tests := []struct { - name string - header string - wantSc core.SpanContext - }{ - { - name: "valid header on invalid carrier", - header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - wantSc: core.EmptySpanContext(), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - e := propagator.CarrierExtractor(tt.header) - gotSc, _ := e.Extract() - if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { - t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) - } - }) - } -} - func TestInjectTraceContextToHTTPReq(t *testing.T) { - propagator := propagation.HttpTraceContextPropagator() - tests := []struct { - name string - sc core.SpanContext - wantHeader string - }{ - { - name: "valid spancontext, sampled", - sc: core.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceOptions: core.TraceOptionSampled, - }, - wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", - }, - { - name: "valid spancontext, not sampled", - sc: core.SpanContext{ - TraceID: traceID, - SpanID: spanID, - }, - wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", - }, - { - name: "invalid spancontext", - sc: core.EmptySpanContext(), - wantHeader: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - req, _ := http.NewRequest("GET", "http://example.com", nil) - i := propagator.CarrierInjector(req) - i.Inject(tt.sc, tag.NewEmptyMap()) - - gotHeader := req.Header.Get("traceparent") - if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { - t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) - } - }) - } -} - -func TestInjectTraceContextToHTTPReq2(t *testing.T) { trace.SetGlobalTracer(trace.DefaultTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { @@ -262,26 +136,3 @@ func TestInjectTraceContextToHTTPReq2(t *testing.T) { }) } } - -func TestInjectTraceContextToInvalidCarrier(t *testing.T) { - propagator := propagation.HttpTraceContextPropagator() - tests := []struct { - name string - sc core.SpanContext - }{ - { - name: "valid spancontext to invalid carrier does nothing.", - sc: core.SpanContext{ - TraceID: traceID, - SpanID: spanID, - TraceOptions: core.TraceOptionSampled, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - i := propagator.CarrierInjector("") - i.Inject(tt.sc, tag.NewEmptyMap()) - }) - } -} From db8f60b03e9dad8c3747ec6c17c2b2d47367fc05 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 4 Sep 2019 16:37:45 -0700 Subject: [PATCH 17/28] rename propagator to TextFormatPropagator. --- api/propagation/noop_propagator.go | 2 +- api/propagation/propagator.go | 6 +++--- propagation/http_trace_context_propagator.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/propagation/noop_propagator.go b/api/propagation/noop_propagator.go index 5d855ade93e..19da3c02d4c 100644 --- a/api/propagation/noop_propagator.go +++ b/api/propagation/noop_propagator.go @@ -21,7 +21,7 @@ import ( // NoopTextFormatPropagator implements TextFormatPropagator that does nothing. type NoopTextFormatPropagator struct{} -var _ Propagator = NoopTextFormatPropagator{} +var _ TextFormatPropagator = NoopTextFormatPropagator{} // Inject does nothing. func (np NoopTextFormatPropagator) Inject(ctx context.Context, supplier Supplier) { diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index 16be9352b91..735de1dce29 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -18,11 +18,11 @@ import ( "context" ) -// Propagator is an interface that specifies methods to inject and extract SpanContext +// TextFormatPropagator is an interface that specifies methods to inject and extract SpanContext // into/from a carrier using Supplier interface. -// For example, HTTP Trace Context Propagator would encode SpanContext into W3C Trace +// For example, HTTP Trace Context propagator would encode SpanContext into W3C Trace // Context Header and set the header into HttpRequest. -type Propagator interface { +type TextFormatPropagator interface { // Inject method retrieves current SpanContext from the ctx, encodes it into propagator // specific format and then injects the encoded SpanContext using supplier into a carrier // associated with the supplier. diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 971bfa4b853..5a0773928d6 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -35,7 +35,7 @@ const ( type httpTraceContextPropagator struct{} -var _ apipropagation.Propagator = httpTraceContextPropagator{} +var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipropagation.Supplier) { sc := trace.CurrentSpan(ctx).SpanContext() From 9c3ecb68a8965f946384a6d80428b56005df9075 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 4 Sep 2019 21:38:50 -0700 Subject: [PATCH 18/28] rename default tracer/span as pass_through tracer/span. --- .../{default_span.go => pass_through_span.go} | 32 +++++++++---------- ...fault_tracer.go => pass_through_tracer.go} | 22 +++++++------ .../http_trace_context_propagator_test.go | 4 +-- 3 files changed, 31 insertions(+), 27 deletions(-) rename api/trace/{default_span.go => pass_through_span.go} (58%) rename api/trace/{default_tracer.go => pass_through_tracer.go} (57%) diff --git a/api/trace/default_span.go b/api/trace/pass_through_span.go similarity index 58% rename from api/trace/default_span.go rename to api/trace/pass_through_span.go index a7043548c5b..6cf14ded974 100644 --- a/api/trace/default_span.go +++ b/api/trace/pass_through_span.go @@ -24,62 +24,62 @@ import ( "go.opentelemetry.io/api/tag" ) -type DefaultSpan struct { +type PassThroughSpan struct { sc core.SpanContext } -var _ Span = (*DefaultSpan)(nil) +var _ Span = (*PassThroughSpan)(nil) // SpancContext returns an invalid span context. -func (ds *DefaultSpan) SpanContext() core.SpanContext { +func (ds *PassThroughSpan) SpanContext() core.SpanContext { if ds == nil { core.EmptySpanContext() } return ds.sc } -// IsRecordingEvents always returns false for DefaultSpan. -func (ds *DefaultSpan) IsRecordingEvents() bool { +// IsRecordingEvents always returns false for PassThroughSpan. +func (ds *PassThroughSpan) IsRecordingEvents() bool { return false } // SetStatus does nothing. -func (ds *DefaultSpan) SetStatus(status codes.Code) { +func (ds *PassThroughSpan) SetStatus(status codes.Code) { } // SetError does nothing. -func (ds *DefaultSpan) SetError(v bool) { +func (ds *PassThroughSpan) SetError(v bool) { } // SetAttribute does nothing. -func (ds *DefaultSpan) SetAttribute(attribute core.KeyValue) { +func (ds *PassThroughSpan) SetAttribute(attribute core.KeyValue) { } // SetAttributes does nothing. -func (ds *DefaultSpan) SetAttributes(attributes ...core.KeyValue) { +func (ds *PassThroughSpan) SetAttributes(attributes ...core.KeyValue) { } // ModifyAttribute does nothing. -func (ds *DefaultSpan) ModifyAttribute(mutator tag.Mutator) { +func (ds *PassThroughSpan) ModifyAttribute(mutator tag.Mutator) { } // ModifyAttributes does nothing. -func (ds *DefaultSpan) ModifyAttributes(mutators ...tag.Mutator) { +func (ds *PassThroughSpan) ModifyAttributes(mutators ...tag.Mutator) { } // Finish does nothing. -func (ds *DefaultSpan) Finish() { +func (ds *PassThroughSpan) Finish() { } // Tracer returns noop implementation of Tracer. -func (ds *DefaultSpan) Tracer() Tracer { - return NoopTracer{} +func (ds *PassThroughSpan) Tracer() Tracer { + return PassThroughTracer{} } // AddEvent does nothing. -func (ds *DefaultSpan) AddEvent(ctx context.Context, event event.Event) { +func (ds *PassThroughSpan) AddEvent(ctx context.Context, event event.Event) { } // Event does nothing. -func (ds *DefaultSpan) Event(ctx context.Context, msg string, attrs ...core.KeyValue) { +func (ds *PassThroughSpan) Event(ctx context.Context, msg string, attrs ...core.KeyValue) { } diff --git a/api/trace/default_tracer.go b/api/trace/pass_through_tracer.go similarity index 57% rename from api/trace/default_tracer.go rename to api/trace/pass_through_tracer.go index 3ac98fa0149..70c7783d8a1 100644 --- a/api/trace/default_tracer.go +++ b/api/trace/pass_through_tracer.go @@ -20,32 +20,36 @@ import ( "go.opentelemetry.io/api/core" ) -type DefaultTracer struct{} +// PassThroughTracer is equivalent of noop tracer except that +// it facilitates forwarding incoming trace to downstream services. +// It does require to use appropriate propagators. +type PassThroughTracer struct{} -var _ Tracer = DefaultTracer{} +var _ Tracer = PassThroughTracer{} // WithResources does nothing and returns noop implementation of Tracer. -func (t DefaultTracer) WithResources(attributes ...core.KeyValue) Tracer { +func (t PassThroughTracer) WithResources(attributes ...core.KeyValue) Tracer { return t } // WithComponent does nothing and returns noop implementation of Tracer. -func (t DefaultTracer) WithComponent(name string) Tracer { +func (t PassThroughTracer) WithComponent(name string) Tracer { return t } // WithService does nothing and returns noop implementation of Tracer. -func (t DefaultTracer) WithService(name string) Tracer { +func (t PassThroughTracer) WithService(name string) Tracer { return t } // WithSpan wraps around execution of func with noop span. -func (t DefaultTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { +func (t PassThroughTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { return body(ctx) } -// Start starts a noop span. -func (DefaultTracer) Start(ctx context.Context, name string, o ...SpanOption) (context.Context, Span) { +// Start starts a PassThroughSpan. It simply creates a copy of remote span. +// If RemoteSpanContext is not provided then it returns a NoopSpan. +func (PassThroughTracer) Start(ctx context.Context, name string, o ...SpanOption) (context.Context, Span) { var opts SpanOptions for _, op := range o { op(&opts) @@ -53,6 +57,6 @@ func (DefaultTracer) Start(ctx context.Context, name string, o ...SpanOption) (c if !opts.RemoteSpanContext.IsValid() { return ctx, NoopSpan{} } - span := &DefaultSpan{sc: opts.RemoteSpanContext} + span := &PassThroughSpan{sc: opts.RemoteSpanContext} return SetCurrentSpan(ctx, span), span } diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 4120540e384..3bd9255e415 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -33,7 +33,7 @@ var ( ) func TestExtractTraceContextFromHTTPReq(t *testing.T) { - trace.SetGlobalTracer(trace.DefaultTracer{}) + trace.SetGlobalTracer(trace.PassThroughTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string @@ -92,7 +92,7 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { } func TestInjectTraceContextToHTTPReq(t *testing.T) { - trace.SetGlobalTracer(trace.DefaultTracer{}) + trace.SetGlobalTracer(trace.PassThroughTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string From e5e1a933f02efb879b211569f51b9a81b01d0402 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 4 Sep 2019 22:19:14 -0700 Subject: [PATCH 19/28] add test for pass through tracer. --- api/trace/pass_through_tracer_test.go | 57 +++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 api/trace/pass_through_tracer_test.go diff --git a/api/trace/pass_through_tracer_test.go b/api/trace/pass_through_tracer_test.go new file mode 100644 index 00000000000..b728f1a15ec --- /dev/null +++ b/api/trace/pass_through_tracer_test.go @@ -0,0 +1,57 @@ +// Copyright 2019, 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 trace_test + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/trace" +) + +func TestStartWithRemoteSpanContext(t *testing.T) { + trace.SetGlobalTracer(trace.PassThroughTracer{}) + sc := core.SpanContext{ + TraceID: core.TraceID{ + High: 0x0102030405060708, + Low: 0x090a0b0c0d0e0f10, + }, + SpanID: 0x0102030405060708, + TraceOptions: 0x00, + } + ctx, span := trace.GlobalTracer().Start(context.Background(), "foo", trace.CopyOfRemote(sc)) + if _, ok := span.(*trace.PassThroughSpan); !ok { + t.Errorf("Want PassThroughSpan but got %T\n", span) + } + currSpanCtx := trace.CurrentSpan(ctx).SpanContext() + if diff := cmp.Diff(currSpanCtx, sc); diff != "" { + t.Errorf("Want copy of span context but got %v\n", currSpanCtx) + } +} + +func TestStartWithoutRemoteSpanContext(t *testing.T) { + trace.SetGlobalTracer(trace.PassThroughTracer{}) + ctx, span := trace.GlobalTracer().Start(context.Background(), "foo") + if _, ok := span.(trace.NoopSpan); !ok { + t.Errorf("Want NoopSpan but got %T\n", span) + } + currSpanCtx := trace.CurrentSpan(ctx).SpanContext() + if diff := cmp.Diff(currSpanCtx, core.EmptySpanContext()); diff != "" { + t.Errorf("Want Invalid span context but got %v\n", currSpanCtx) + } +} From ecd6ba75f22e541c71d178d84d95aa7dc7c83b6c Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 4 Sep 2019 22:36:04 -0700 Subject: [PATCH 20/28] add missing interface to pass through tracer. --- api/trace/pass_through_span.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/api/trace/pass_through_span.go b/api/trace/pass_through_span.go index 6cf14ded974..e47343fd911 100644 --- a/api/trace/pass_through_span.go +++ b/api/trace/pass_through_span.go @@ -20,7 +20,6 @@ import ( "google.golang.org/grpc/codes" "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/event" "go.opentelemetry.io/api/tag" ) @@ -68,7 +67,11 @@ func (ds *PassThroughSpan) ModifyAttributes(mutators ...tag.Mutator) { } // Finish does nothing. -func (ds *PassThroughSpan) Finish() { +func (ds *PassThroughSpan) Finish(options ...FinishOption) { +} + +// SetName does nothing. +func (ds *PassThroughSpan) SetName(name string) { } // Tracer returns noop implementation of Tracer. @@ -77,9 +80,5 @@ func (ds *PassThroughSpan) Tracer() Tracer { } // AddEvent does nothing. -func (ds *PassThroughSpan) AddEvent(ctx context.Context, event event.Event) { -} - -// Event does nothing. -func (ds *PassThroughSpan) Event(ctx context.Context, msg string, attrs ...core.KeyValue) { +func (ds *PassThroughSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyValue) { } From 2a5fc9e2582c569c38120b9e023852cb47b958ae Mon Sep 17 00:00:00 2001 From: rahulpa Date: Mon, 9 Sep 2019 11:49:21 -0700 Subject: [PATCH 21/28] return SpanContext instead of contex.Context from Extract interface. - also remove PassThroughTracer --- api/propagation/noop_propagator.go | 6 +- api/propagation/propagator.go | 10 +- api/trace/api.go | 16 +--- api/trace/pass_through_tracer.go | 62 ------------- api/trace/pass_through_tracer_test.go | 57 ------------ example/http/server/server.go | 5 +- .../trace/mock_span.go | 40 ++++---- internal/trace/mock_tracer.go | 91 +++++++++++++++++++ plugin/httptrace/httptrace.go | 6 +- propagation/http_trace_context_propagator.go | 31 +++---- .../http_trace_context_propagator_test.go | 21 +++-- sdk/trace/span.go | 10 -- 12 files changed, 160 insertions(+), 195 deletions(-) delete mode 100644 api/trace/pass_through_tracer.go delete mode 100644 api/trace/pass_through_tracer_test.go rename api/trace/pass_through_span.go => internal/trace/mock_span.go (55%) create mode 100644 internal/trace/mock_tracer.go diff --git a/api/propagation/noop_propagator.go b/api/propagation/noop_propagator.go index 19da3c02d4c..fd3ef263414 100644 --- a/api/propagation/noop_propagator.go +++ b/api/propagation/noop_propagator.go @@ -16,6 +16,8 @@ package propagation import ( "context" + + "go.opentelemetry.io/api/core" ) // NoopTextFormatPropagator implements TextFormatPropagator that does nothing. @@ -28,8 +30,8 @@ func (np NoopTextFormatPropagator) Inject(ctx context.Context, supplier Supplier } // Extract does nothing. -func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) context.Context { - return ctx +func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) core.SpanContext { + return core.EmptySpanContext() } // GetAllKeys returns empty list of strings. diff --git a/api/propagation/propagator.go b/api/propagation/propagator.go index 735de1dce29..f52a51ed92a 100644 --- a/api/propagation/propagator.go +++ b/api/propagation/propagator.go @@ -16,6 +16,8 @@ package propagation import ( "context" + + "go.opentelemetry.io/api/core" ) // TextFormatPropagator is an interface that specifies methods to inject and extract SpanContext @@ -28,10 +30,10 @@ type TextFormatPropagator interface { // associated with the supplier. Inject(ctx context.Context, supplier Supplier) - // Extract method retrieves SpanContext using supplier from the associated carrier with - // the supplier. It decodes the SpanContext and it creates remote span using registered - // tracer. - Extract(ctx context.Context, supplier Supplier) context.Context + // Extract method retrieves encoded SpanContext using supplier from the associated carrier. + // It decodes the SpanContext and returns it. If no SpanContext was retrieved OR + // if the retrieved SpanContext is invalid then an empty SpanContext is returned. + Extract(ctx context.Context, supplier Supplier) core.SpanContext // GetAllKeys returns all the keys that this propagator injects/extracts into/from a // carrier. The use cases for this are diff --git a/api/trace/api.go b/api/trace/api.go index e9e185a9365..68cba1ea990 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -105,11 +105,10 @@ type SpanOption func(*SpanOptions) // SpanOptions provides options to set properties of span at the time of starting // a new span. type SpanOptions struct { - Attributes []core.KeyValue - StartTime time.Time - Reference Reference - RecordEvent bool - RemoteSpanContext core.SpanContext + Attributes []core.KeyValue + StartTime time.Time + Reference Reference + RecordEvent bool } // Reference is used to establish relationship between newly created span and the @@ -174,13 +173,6 @@ func WithRecordEvents() SpanOption { } } -// CopyOfRemote allows to create an inactive span mirroring remote span. -func CopyOfRemote(sc core.SpanContext) SpanOption { - return func(o *SpanOptions) { - o.RemoteSpanContext = sc - } -} - // ChildOf. TODO: do we need this?. func ChildOf(sc core.SpanContext) SpanOption { return func(o *SpanOptions) { diff --git a/api/trace/pass_through_tracer.go b/api/trace/pass_through_tracer.go deleted file mode 100644 index 70c7783d8a1..00000000000 --- a/api/trace/pass_through_tracer.go +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2019, 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 trace - -import ( - "context" - - "go.opentelemetry.io/api/core" -) - -// PassThroughTracer is equivalent of noop tracer except that -// it facilitates forwarding incoming trace to downstream services. -// It does require to use appropriate propagators. -type PassThroughTracer struct{} - -var _ Tracer = PassThroughTracer{} - -// WithResources does nothing and returns noop implementation of Tracer. -func (t PassThroughTracer) WithResources(attributes ...core.KeyValue) Tracer { - return t -} - -// WithComponent does nothing and returns noop implementation of Tracer. -func (t PassThroughTracer) WithComponent(name string) Tracer { - return t -} - -// WithService does nothing and returns noop implementation of Tracer. -func (t PassThroughTracer) WithService(name string) Tracer { - return t -} - -// WithSpan wraps around execution of func with noop span. -func (t PassThroughTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { - return body(ctx) -} - -// Start starts a PassThroughSpan. It simply creates a copy of remote span. -// If RemoteSpanContext is not provided then it returns a NoopSpan. -func (PassThroughTracer) Start(ctx context.Context, name string, o ...SpanOption) (context.Context, Span) { - var opts SpanOptions - for _, op := range o { - op(&opts) - } - if !opts.RemoteSpanContext.IsValid() { - return ctx, NoopSpan{} - } - span := &PassThroughSpan{sc: opts.RemoteSpanContext} - return SetCurrentSpan(ctx, span), span -} diff --git a/api/trace/pass_through_tracer_test.go b/api/trace/pass_through_tracer_test.go deleted file mode 100644 index b728f1a15ec..00000000000 --- a/api/trace/pass_through_tracer_test.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2019, 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 trace_test - -import ( - "context" - "testing" - - "github.com/google/go-cmp/cmp" - - "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/trace" -) - -func TestStartWithRemoteSpanContext(t *testing.T) { - trace.SetGlobalTracer(trace.PassThroughTracer{}) - sc := core.SpanContext{ - TraceID: core.TraceID{ - High: 0x0102030405060708, - Low: 0x090a0b0c0d0e0f10, - }, - SpanID: 0x0102030405060708, - TraceOptions: 0x00, - } - ctx, span := trace.GlobalTracer().Start(context.Background(), "foo", trace.CopyOfRemote(sc)) - if _, ok := span.(*trace.PassThroughSpan); !ok { - t.Errorf("Want PassThroughSpan but got %T\n", span) - } - currSpanCtx := trace.CurrentSpan(ctx).SpanContext() - if diff := cmp.Diff(currSpanCtx, sc); diff != "" { - t.Errorf("Want copy of span context but got %v\n", currSpanCtx) - } -} - -func TestStartWithoutRemoteSpanContext(t *testing.T) { - trace.SetGlobalTracer(trace.PassThroughTracer{}) - ctx, span := trace.GlobalTracer().Start(context.Background(), "foo") - if _, ok := span.(trace.NoopSpan); !ok { - t.Errorf("Want NoopSpan but got %T\n", span) - } - currSpanCtx := trace.CurrentSpan(ctx).SpanContext() - if diff := cmp.Diff(currSpanCtx, core.EmptySpanContext()); diff != "" { - t.Errorf("Want Invalid span context but got %v\n", currSpanCtx) - } -} diff --git a/example/http/server/server.go b/example/http/server/server.go index db4a152e4fb..0a3d099de3d 100644 --- a/example/http/server/server.go +++ b/example/http/server/server.go @@ -35,9 +35,9 @@ var ( func main() { helloHandler := func(w http.ResponseWriter, req *http.Request) { - attrs, tags, ctx := httptrace.Extract(req.Context(), req) + attrs, tags, spanCtx := httptrace.Extract(req.Context(), req) - req = req.WithContext(tag.WithMap(ctx, tag.NewMap(tag.MapUpdate{ + req = req.WithContext(tag.WithMap(req.Context(), tag.NewMap(tag.MapUpdate{ MultiKV: tags, }))) @@ -45,6 +45,7 @@ func main() { req.Context(), "hello", trace.WithAttributes(attrs...), + trace.ChildOf(spanCtx), ) defer span.Finish() diff --git a/api/trace/pass_through_span.go b/internal/trace/mock_span.go similarity index 55% rename from api/trace/pass_through_span.go rename to internal/trace/mock_span.go index e47343fd911..e458a04745a 100644 --- a/api/trace/pass_through_span.go +++ b/internal/trace/mock_span.go @@ -21,64 +21,66 @@ import ( "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/tag" + apitrace "go.opentelemetry.io/api/trace" ) -type PassThroughSpan struct { - sc core.SpanContext +type MockSpan struct { + sc core.SpanContext + tracer apitrace.Tracer } -var _ Span = (*PassThroughSpan)(nil) +var _ apitrace.Span = (*MockSpan)(nil) // SpancContext returns an invalid span context. -func (ds *PassThroughSpan) SpanContext() core.SpanContext { - if ds == nil { +func (ms *MockSpan) SpanContext() core.SpanContext { + if ms == nil { core.EmptySpanContext() } - return ds.sc + return ms.sc } -// IsRecordingEvents always returns false for PassThroughSpan. -func (ds *PassThroughSpan) IsRecordingEvents() bool { +// IsRecordingEvents always returns false for MockSpan. +func (ms *MockSpan) IsRecordingEvents() bool { return false } // SetStatus does nothing. -func (ds *PassThroughSpan) SetStatus(status codes.Code) { +func (ms *MockSpan) SetStatus(status codes.Code) { } // SetError does nothing. -func (ds *PassThroughSpan) SetError(v bool) { +func (ms *MockSpan) SetError(v bool) { } // SetAttribute does nothing. -func (ds *PassThroughSpan) SetAttribute(attribute core.KeyValue) { +func (ms *MockSpan) SetAttribute(attribute core.KeyValue) { } // SetAttributes does nothing. -func (ds *PassThroughSpan) SetAttributes(attributes ...core.KeyValue) { +func (ms *MockSpan) SetAttributes(attributes ...core.KeyValue) { } // ModifyAttribute does nothing. -func (ds *PassThroughSpan) ModifyAttribute(mutator tag.Mutator) { +func (ms *MockSpan) ModifyAttribute(mutator tag.Mutator) { } // ModifyAttributes does nothing. -func (ds *PassThroughSpan) ModifyAttributes(mutators ...tag.Mutator) { +func (ms *MockSpan) ModifyAttributes(mutators ...tag.Mutator) { } // Finish does nothing. -func (ds *PassThroughSpan) Finish(options ...FinishOption) { +func (ms *MockSpan) Finish(options ...apitrace.FinishOption) { } // SetName does nothing. -func (ds *PassThroughSpan) SetName(name string) { +func (ms *MockSpan) SetName(name string) { } // Tracer returns noop implementation of Tracer. -func (ds *PassThroughSpan) Tracer() Tracer { - return PassThroughTracer{} +func (ms *MockSpan) Tracer() apitrace.Tracer { + return ms.tracer } // AddEvent does nothing. -func (ds *PassThroughSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyValue) { +func (ms *MockSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyValue) { } diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go new file mode 100644 index 00000000000..8df6f307183 --- /dev/null +++ b/internal/trace/mock_tracer.go @@ -0,0 +1,91 @@ +// Copyright 2019, 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 trace + +import ( + "context" + "math/rand" + "sync/atomic" + + "go.opentelemetry.io/api/core" + apitrace "go.opentelemetry.io/api/trace" +) + +// MockTracer is a simple tracer used for testing purpose only. +// It only supports ChildOf option. SpanId is atomically increased every time a +// new span is created. +type MockTracer struct { + // Sampled specifies if the new span should be sampled or not. + Sampled bool + + // StartSpanId is used to initialize spanId. It is incremented by one + // every time a new span is created. + StartSpanId *uint64 +} + +var _ apitrace.Tracer = (*MockTracer)(nil) + +// WithResources does nothing and returns noop implementation of Tracer. +func (mt *MockTracer) WithResources(attributes ...core.KeyValue) apitrace.Tracer { + return mt +} + +// WithComponent does nothing and returns noop implementation of Tracer. +func (mt *MockTracer) WithComponent(name string) apitrace.Tracer { + return mt +} + +// WithService does nothing and returns noop implementation of Tracer. +func (mt *MockTracer) WithService(name string) apitrace.Tracer { + return mt +} + +// WithSpan wraps around execution of func with noop span. +func (mt *MockTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { + return body(ctx) +} + +// Start starts a MockSpan. It creates a new Span based on Reference SpanContext option. +// TracdID is used from Reference Span Context and SpanID is assigned. +// If Reference SpanContext option is not specified then random TraceID is used. +// No other options are supported. +func (mt *MockTracer) Start(ctx context.Context, name string, o ...apitrace.SpanOption) (context.Context, apitrace.Span) { + var opts apitrace.SpanOptions + for _, op := range o { + op(&opts) + } + var span *MockSpan + var sc core.SpanContext + if !opts.Reference.SpanContext.IsValid() { + sc = core.SpanContext{ + TraceID: core.TraceID{ + High: rand.Uint64(), + Low: rand.Uint64(), + }, + } + if mt.Sampled { + sc.TraceOptions = core.TraceOptionSampled + } + } else { + sc = opts.Reference.SpanContext + } + sc.SpanID = atomic.AddUint64(mt.StartSpanId, 1) + span = &MockSpan{ + sc: sc, + tracer: mt, + } + + return apitrace.SetCurrentSpan(ctx, span), span +} diff --git a/plugin/httptrace/httptrace.go b/plugin/httptrace/httptrace.go index 0cca193f443..f6c52646f71 100644 --- a/plugin/httptrace/httptrace.go +++ b/plugin/httptrace/httptrace.go @@ -41,15 +41,15 @@ var ( ) // Returns the Attributes, Context Tags, and SpanContext that were encoded by Inject. -func Extract(ctx context.Context, req *http.Request) ([]core.KeyValue, []core.KeyValue, context.Context) { - ctx = propagator.Extract(ctx, req.Header) +func Extract(ctx context.Context, req *http.Request) ([]core.KeyValue, []core.KeyValue, core.SpanContext) { + sc := propagator.Extract(ctx, req.Header) attrs := []core.KeyValue{ URLKey.String(req.URL.String()), // Etc. } - return attrs, nil, ctx + return attrs, nil, sc } func Inject(ctx context.Context, req *http.Request) { diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 5a0773928d6..c53535347ac 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -50,40 +50,40 @@ func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipro } } -func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipropagation.Supplier) context.Context { +func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipropagation.Supplier) core.SpanContext { h := supplier.Get(traceparentHeader) if h == "" { - return ctx + return core.EmptySpanContext() } sections := strings.Split(h, "-") if len(sections) < 4 { - return ctx + return core.EmptySpanContext() } if len(sections[0]) != 2 { - return ctx + return core.EmptySpanContext() } ver, err := hex.DecodeString(sections[0]) if err != nil { - return ctx + return core.EmptySpanContext() } version := int(ver[0]) if version > maxVersion { - return ctx + return core.EmptySpanContext() } if version == 0 && len(sections) != 4 { - return ctx + return core.EmptySpanContext() } if len(sections[1]) != 32 { - return ctx + return core.EmptySpanContext() } result, err := strconv.ParseUint(sections[1][0:16], 16, 64) if err != nil { - return ctx + return core.EmptySpanContext() } var sc core.SpanContext @@ -91,31 +91,30 @@ func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipr result, err = strconv.ParseUint(sections[1][16:32], 16, 64) if err != nil { - return ctx + return core.EmptySpanContext() } sc.TraceID.Low = result if len(sections[2]) != 16 { - return ctx + return core.EmptySpanContext() } result, err = strconv.ParseUint(sections[2][0:], 16, 64) if err != nil { - return ctx + return core.EmptySpanContext() } sc.SpanID = result opts, err := hex.DecodeString(sections[3]) if err != nil || len(opts) < 1 { - return ctx + return core.EmptySpanContext() } sc.TraceOptions = opts[0] if !sc.IsValid() { - return ctx + return core.EmptySpanContext() } - ctx, _ = trace.GlobalTracer().Start(ctx, "remote", trace.CopyOfRemote(sc)) - return ctx + return sc } func (hp httpTraceContextPropagator) GetAllKeys() []string { diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 3bd9255e415..566a839a348 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "go.opentelemetry.io/api/core" + mocktrace "go.opentelemetry.io/internal/trace" "go.opentelemetry.io/propagation" ) @@ -33,7 +34,7 @@ var ( ) func TestExtractTraceContextFromHTTPReq(t *testing.T) { - trace.SetGlobalTracer(trace.PassThroughTracer{}) + trace.SetGlobalTracer(&mocktrace.MockTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string @@ -81,9 +82,7 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { req.Header.Set("traceparent", tt.header) ctx := context.Background() - ctx = propagator.Extract(ctx, req.Header) - span := trace.CurrentSpan(ctx) - gotSc := span.SpanContext() + gotSc := propagator.Extract(ctx, req.Header) if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) } @@ -92,7 +91,11 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { } func TestInjectTraceContextToHTTPReq(t *testing.T) { - trace.SetGlobalTracer(trace.PassThroughTracer{}) + var id uint64 + trace.SetGlobalTracer(&mocktrace.MockTracer{ + Sampled: false, + StartSpanId: &id, + }) propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string @@ -106,7 +109,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { SpanID: spanID, TraceOptions: core.TraceOptionSampled, }, - wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000001-01", }, { name: "valid spancontext, not sampled", @@ -114,7 +117,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { TraceID: traceID, SpanID: spanID, }, - wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000002-00", }, { name: "invalid spancontext", @@ -126,7 +129,9 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := context.Background() - ctx, _ = trace.GlobalTracer().Start(ctx, "inject", trace.CopyOfRemote(tt.sc)) + if tt.sc.IsValid() { + ctx, _ = trace.GlobalTracer().Start(ctx, "inject", trace.ChildOf(tt.sc)) + } propagator.Inject(ctx, req.Header) gotHeader := req.Header.Get("traceparent") diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 3a8315a28d4..38e6fd94931 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -38,9 +38,6 @@ type span struct { mu sync.Mutex // protects the contents of *data (but not the pointer value.) spanContext core.SpanContext - // remote is true if span was created to mirror a remote span. - remote bool - // lruAttributes are capped at configured limit. When the capacity is reached an oldest entry // is removed to create room for a new entry. lruAttributes *lruMap @@ -300,13 +297,6 @@ func (s *span) addChild() { func startSpanInternal(name string, parent core.SpanContext, remoteParent bool, o apitrace.SpanOptions) *span { var noParent bool span := &span{} - - if o.RemoteSpanContext.IsValid() { - span.remote = true - span.spanContext = o.RemoteSpanContext - return span - } - span.spanContext = parent cfg := config.Load().(*Config) From c8a05ec0d210dcc5b03a6913f4a0a4eeb7dd2b52 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Wed, 18 Sep 2019 10:41:44 -0700 Subject: [PATCH 22/28] fix review comments. --- api/propagation/noop_propagator.go | 4 +-- example/http/client/client.go | 4 +-- internal/trace/mock_span.go | 6 ++-- internal/trace/mock_tracer.go | 8 ++--- plugin/httptrace/httptrace.go | 31 ------------------- propagation/http_trace_context_propagator.go | 10 ++---- .../http_trace_context_propagator_test.go | 15 +++++++-- 7 files changed, 25 insertions(+), 53 deletions(-) diff --git a/api/propagation/noop_propagator.go b/api/propagation/noop_propagator.go index fd3ef263414..dc185c80c92 100644 --- a/api/propagation/noop_propagator.go +++ b/api/propagation/noop_propagator.go @@ -29,7 +29,7 @@ var _ TextFormatPropagator = NoopTextFormatPropagator{} func (np NoopTextFormatPropagator) Inject(ctx context.Context, supplier Supplier) { } -// Extract does nothing. +// Extract does nothing and returns an empty SpanContext func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) core.SpanContext { return core.EmptySpanContext() } @@ -37,4 +37,4 @@ func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplie // GetAllKeys returns empty list of strings. func (np NoopTextFormatPropagator) GetAllKeys() []string { return []string{} -} \ No newline at end of file +} diff --git a/example/http/client/client.go b/example/http/client/client.go index 8fe9d7a5f34..6be1d347412 100644 --- a/example/http/client/client.go +++ b/example/http/client/client.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/api/key" "go.opentelemetry.io/api/tag" "go.opentelemetry.io/api/trace" - "go.opentelemetry.io/propagation" ) var ( @@ -51,9 +50,8 @@ func main() { func(ctx context.Context) error { req, _ := http.NewRequest("GET", "http://localhost:7777/hello", nil) - propagator := propagation.HttpTraceContextPropagator() ctx, req = httptrace.W3C(ctx, req) - propagator.Inject(ctx, req.Header) + httptrace.Inject(ctx, req) res, err := client.Do(req) if err != nil { diff --git a/internal/trace/mock_span.go b/internal/trace/mock_span.go index e458a04745a..84ded071c16 100644 --- a/internal/trace/mock_span.go +++ b/internal/trace/mock_span.go @@ -24,6 +24,7 @@ import ( apitrace "go.opentelemetry.io/api/trace" ) +// MockSpan is a mock span used in association with MockTracer for testing purpose only. type MockSpan struct { sc core.SpanContext tracer apitrace.Tracer @@ -31,7 +32,8 @@ type MockSpan struct { var _ apitrace.Span = (*MockSpan)(nil) -// SpancContext returns an invalid span context. +// SpanContext returns associated core.SpanContext. If the receiver is nil it returns +// an empty core.SpanContext func (ms *MockSpan) SpanContext() core.SpanContext { if ms == nil { core.EmptySpanContext() @@ -76,7 +78,7 @@ func (ms *MockSpan) Finish(options ...apitrace.FinishOption) { func (ms *MockSpan) SetName(name string) { } -// Tracer returns noop implementation of Tracer. +// Tracer returns MockTracer implementation of Tracer. func (ms *MockSpan) Tracer() apitrace.Tracer { return ms.tracer } diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go index 8df6f307183..101748bd2a0 100644 --- a/internal/trace/mock_tracer.go +++ b/internal/trace/mock_tracer.go @@ -37,22 +37,22 @@ type MockTracer struct { var _ apitrace.Tracer = (*MockTracer)(nil) -// WithResources does nothing and returns noop implementation of Tracer. +// WithResources does nothing and returns MockTracer implementation of Tracer. func (mt *MockTracer) WithResources(attributes ...core.KeyValue) apitrace.Tracer { return mt } -// WithComponent does nothing and returns noop implementation of Tracer. +// WithComponent does nothing and returns MockTracer implementation of Tracer. func (mt *MockTracer) WithComponent(name string) apitrace.Tracer { return mt } -// WithService does nothing and returns noop implementation of Tracer. +// WithService does nothing and returns MockTracer implementation of Tracer. func (mt *MockTracer) WithService(name string) apitrace.Tracer { return mt } -// WithSpan wraps around execution of func with noop span. +// WithSpan does nothing except executing the body. func (mt *MockTracer) WithSpan(ctx context.Context, name string, body func(context.Context) error) error { return body(ctx) } diff --git a/plugin/httptrace/httptrace.go b/plugin/httptrace/httptrace.go index f6c52646f71..86a64c5cd1f 100644 --- a/plugin/httptrace/httptrace.go +++ b/plugin/httptrace/httptrace.go @@ -16,13 +16,8 @@ package httptrace import ( "context" - "encoding/binary" "net/http" - "go.opentelemetry.io/api/trace" - - "github.com/lightstep/tracecontext.go" - "go.opentelemetry.io/api/core" "go.opentelemetry.io/api/key" "go.opentelemetry.io/propagation" @@ -36,7 +31,6 @@ var ( HostKey = key.New("http.host") URLKey = key.New("http.url") - encoding = binary.BigEndian propagator = propagation.HttpTraceContextPropagator() ) @@ -54,29 +48,4 @@ func Extract(ctx context.Context, req *http.Request) ([]core.KeyValue, []core.Ke func Inject(ctx context.Context, req *http.Request) { propagator.Inject(ctx, req.Header) - sc := trace.CurrentSpan(ctx).SpanContext() - var tc tracecontext.TraceContext - var sid [8]byte - var tid [16]byte - - encoding.PutUint64(sid[0:8], sc.SpanID) - encoding.PutUint64(tid[0:8], sc.TraceID.High) - encoding.PutUint64(tid[8:16], sc.TraceID.Low) - - tc.TraceParent.Version = tracecontext.Version - tc.TraceParent.TraceID = tid - tc.TraceParent.SpanID = sid - tc.TraceParent.Flags.Recorded = true // Note: not implemented. - - tags.Foreach(func(kv core.KeyValue) bool { - // TODO: implement MaxHops - tc.TraceState = append(tc.TraceState, tracestate.Member{ - Vendor: Vendor, - Tenant: kv.Key.Name, - Value: kv.Value.Emit(), - }) - return true - }) - - tc.SetHeaders(req.Header) } diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index c53535347ac..3f92725ebc7 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -118,17 +118,11 @@ func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipr } func (hp httpTraceContextPropagator) GetAllKeys() []string { - return nil + return []string{traceparentHeader} } // HttpTraceContextPropagator creates a new text format propagator that propagates SpanContext // in W3C TraceContext format. -// -// The propagator is then used to create CarrierInjector and CarrierExtractor associated with a -// specific request. Injectors and Extractors respectively provides method to -// inject and extract SpanContext into/from the http request. Inject method encodes -// SpanContext and tag.Map into W3C TraceContext Header and injects the header in the request. -// Extract method extracts the header and decodes SpanContext and tag.Map. -func HttpTraceContextPropagator() httpTraceContextPropagator { +func HttpTraceContextPropagator() apipropagation.TextFormatPropagator { return httpTraceContextPropagator{} } diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 566a839a348..fc2fe34311d 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -92,10 +92,10 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { func TestInjectTraceContextToHTTPReq(t *testing.T) { var id uint64 - trace.SetGlobalTracer(&mocktrace.MockTracer{ + mockTracer := &mocktrace.MockTracer{ Sampled: false, StartSpanId: &id, - }) + } propagator := propagation.HttpTraceContextPropagator() tests := []struct { name string @@ -130,7 +130,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := context.Background() if tt.sc.IsValid() { - ctx, _ = trace.GlobalTracer().Start(ctx, "inject", trace.ChildOf(tt.sc)) + ctx, _ = mockTracer.Start(ctx, "inject", trace.ChildOf(tt.sc)) } propagator.Inject(ctx, req.Header) @@ -141,3 +141,12 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { }) } } + +func TestHttpTraceContextPropagator_GetAllKeys(t *testing.T) { + propagator := propagation.HttpTraceContextPropagator() + want := []string{"traceparent"} + got := propagator.GetAllKeys() + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("GetAllKeys: -got +want %s", diff) + } +} \ No newline at end of file From d759fec9fd06214c955985cc6071c28e00348004 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Thu, 19 Sep 2019 12:23:35 -0700 Subject: [PATCH 23/28] add more test cases for traceContext extraction. --- propagation/http_trace_context_propagator.go | 14 +- .../http_trace_context_propagator_test.go | 127 ++++++++++++++++-- 2 files changed, 129 insertions(+), 12 deletions(-) diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 3f92725ebc7..e00031df5d4 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -18,6 +18,7 @@ import ( "context" "encoding/hex" "fmt" + "regexp" "strconv" "strings" @@ -36,6 +37,7 @@ const ( type httpTraceContextPropagator struct{} var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} +var traceCtxRegExp = regexp.MustCompile("^[0-9a-f]{2}-[a-f0-9]{32}-[a-f0-9]{16}-[a-f0-9]{2}") func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipropagation.Supplier) { sc := trace.CurrentSpan(ctx).SpanContext() @@ -56,6 +58,11 @@ func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipr return core.EmptySpanContext() } + if !traceCtxRegExp.MatchString(h) { + fmt.Printf("header does not match regex %s\n", h) + return core.EmptySpanContext() + } + sections := strings.Split(h, "-") if len(sections) < 4 { return core.EmptySpanContext() @@ -104,11 +111,14 @@ func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipr } sc.SpanID = result + if len(sections[3]) != 2 { + return core.EmptySpanContext() + } opts, err := hex.DecodeString(sections[3]) - if err != nil || len(opts) < 1 { + if err != nil || len(opts) < 1 || (version == 0 && opts[0] > 2) { return core.EmptySpanContext() } - sc.TraceOptions = opts[0] + sc.TraceOptions = opts[0] &^ core.TraceOptionUnused if !sc.IsValid() { return core.EmptySpanContext() diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index fc2fe34311d..ea48ddca926 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -33,7 +33,7 @@ var ( spanID = uint64(0x00f067aa0ba902b7) ) -func TestExtractTraceContextFromHTTPReq(t *testing.T) { +func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { trace.SetGlobalTracer(&mocktrace.MockTracer{}) propagator := propagation.HttpTraceContextPropagator() tests := []struct { @@ -41,6 +41,23 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { header string wantSc core.SpanContext }{ + { + name: "valid header", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + }, + { + name: "valid header and sampled", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, { name: "future version", header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", @@ -51,28 +68,118 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { }, }, { - name: "zero trace ID and span ID", - header: "00-00000000000000000000000000000000-0000000000000000-01", - wantSc: core.EmptySpanContext(), + name: "future options with sampled bit set", + header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, }, { - name: "valid header", - header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + name: "future options with sampled bit cleared", + header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-08", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + }, + }, + { + name: "future additional data", + header: "02-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09-XYZxsf09", wantSc: core.SpanContext{ TraceID: traceID, SpanID: spanID, TraceOptions: core.TraceOptionSampled, }, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com", nil) + req.Header.Set("traceparent", tt.header) + + ctx := context.Background() + gotSc := propagator.Extract(ctx, req.Header) + if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { + t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) + } + }) + } +} + +func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { + trace.SetGlobalTracer(&mocktrace.MockTracer{}) + propagator := propagation.HttpTraceContextPropagator() + wantSc := core.EmptySpanContext() + tests := []struct { + name string + header string + }{ + { + name: "wrong version length", + header: "0000-00000000000000000000000000000000-0000000000000000-01", + }, + { + name: "wrong trace ID length", + header: "00-ab00000000000000000000000000000000-cd00000000000000-01", + }, + { + name: "wrong span ID length", + header: "00-ab000000000000000000000000000000-cd0000000000000000-01", + }, + { + name: "wrong trace flag length", + header: "00-ab000000000000000000000000000000-cd00000000000000-0100", + }, + { + name: "bogus version length", + header: "qw-00000000000000000000000000000000-0000000000000000-01", + }, + { + name: "bogus trace ID length", + header: "00-qw000000000000000000000000000000-cd00000000000000-01", + }, + { + name: "bogus span ID length", + header: "00-ab000000000000000000000000000000-qw00000000000000-01", + }, + { + name: "bogus trace flag length", + header: "00-ab000000000000000000000000000000-cd00000000000000-qw", + }, + { + name: "upper case version length", + header: "A0-00000000000000000000000000000000-0000000000000000-01", + }, + { + name: "upper case trace ID length", + header: "00-AB000000000000000000000000000000-cd00000000000000-01", + }, + { + name: "upper case span ID length", + header: "00-ab000000000000000000000000000000-CD00000000000000-01", + }, + { + name: "upper case trace flag length", + header: "00-ab000000000000000000000000000000-cd00000000000000-A1", + }, + { + name: "zero trace ID and span ID", + header: "00-00000000000000000000000000000000-0000000000000000-01", + }, + { + name: "trace-flag unused bits set", + header: "00-ab000000000000000000000000000000-cd00000000000000-09", + }, { name: "missing options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7", - wantSc: core.EmptySpanContext(), }, { name: "empty options", header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-", - wantSc: core.EmptySpanContext(), }, } @@ -83,7 +190,7 @@ func TestExtractTraceContextFromHTTPReq(t *testing.T) { ctx := context.Background() gotSc := propagator.Extract(ctx, req.Header) - if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { + if diff := cmp.Diff(gotSc, wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) } }) @@ -149,4 +256,4 @@ func TestHttpTraceContextPropagator_GetAllKeys(t *testing.T) { if diff := cmp.Diff(got, want); diff != "" { t.Errorf("GetAllKeys: -got +want %s", diff) } -} \ No newline at end of file +} From 418b0f43d558222b5743d3ffb353dac24b88f954 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Thu, 19 Sep 2019 22:13:57 -0700 Subject: [PATCH 24/28] remove tidy temporarily from circle-ci target to avoid build failure. --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 1c5b8d8c463..29fff1c613d 100644 --- a/Makefile +++ b/Makefile @@ -33,6 +33,9 @@ precommit: $(TOOLS_DIR)/golangci-lint $(TOOLS_DIR)/misspell $(TOOLS_DIR)/string PATH="$(abspath $(TOOLS_DIR)):$${PATH}" go generate ./... $(TOOLS_DIR)/golangci-lint run --fix # TODO: Fix this on windows. $(TOOLS_DIR)/misspell -w $(ALL_DOCS) + +.PHONY: mod-tidy +mod-tidy: for dir in $(ALL_GO_MOD_DIRS); do \ echo "go mod tidy in $${dir}"; \ (cd "$${dir}" && go mod tidy); \ From f271dfc554dc9ca0ea6dfbadf188d2ca1ed7c5d8 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Fri, 20 Sep 2019 13:46:33 -0700 Subject: [PATCH 25/28] allow header ending in dash '-'. --- propagation/http_trace_context_propagator.go | 4 +-- .../http_trace_context_propagator_test.go | 28 +++++++++++++++---- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index e00031df5d4..9fa755aea00 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -37,7 +37,7 @@ const ( type httpTraceContextPropagator struct{} var _ apipropagation.TextFormatPropagator = httpTraceContextPropagator{} -var traceCtxRegExp = regexp.MustCompile("^[0-9a-f]{2}-[a-f0-9]{32}-[a-f0-9]{16}-[a-f0-9]{2}") +var traceCtxRegExp = regexp.MustCompile("^[0-9a-f]{2}-[a-f0-9]{32}-[a-f0-9]{16}-[a-f0-9]{2}-?") func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipropagation.Supplier) { sc := trace.CurrentSpan(ctx).SpanContext() @@ -58,8 +58,8 @@ func (hp httpTraceContextPropagator) Extract(ctx context.Context, supplier apipr return core.EmptySpanContext() } + h = strings.Trim(h, "-") if !traceCtxRegExp.MatchString(h) { - fmt.Printf("header does not match regex %s\n", h) return core.EmptySpanContext() } diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index ea48ddca926..27b4cf61de5 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -93,6 +93,24 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { TraceOptions: core.TraceOptionSampled, }, }, + { + name: "valid header ending in dash", + header: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01-", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, + { + name: "future valid header ending in dash", + header: "01-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-09-", + wantSc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: core.TraceOptionSampled, + }, + }, } for _, tt := range tests { @@ -146,23 +164,23 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { header: "00-ab000000000000000000000000000000-qw00000000000000-01", }, { - name: "bogus trace flag length", + name: "bogus trace flag", header: "00-ab000000000000000000000000000000-cd00000000000000-qw", }, { - name: "upper case version length", + name: "upper case version", header: "A0-00000000000000000000000000000000-0000000000000000-01", }, { - name: "upper case trace ID length", + name: "upper case trace ID", header: "00-AB000000000000000000000000000000-cd00000000000000-01", }, { - name: "upper case span ID length", + name: "upper case span ID", header: "00-ab000000000000000000000000000000-CD00000000000000-01", }, { - name: "upper case trace flag length", + name: "upper case trace flag", header: "00-ab000000000000000000000000000000-cd00000000000000-A1", }, { From 4ca80c37cc057147432d1debfeef3acc62752d6f Mon Sep 17 00:00:00 2001 From: rahulpa Date: Sun, 22 Sep 2019 22:13:45 -0700 Subject: [PATCH 26/28] add inject test for non-zero value other than 01 for traceoption --- propagation/http_trace_context_propagator.go | 2 +- propagation/http_trace_context_propagator_test.go | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/propagation/http_trace_context_propagator.go b/propagation/http_trace_context_propagator.go index 9fa755aea00..7ce6f614aeb 100644 --- a/propagation/http_trace_context_propagator.go +++ b/propagation/http_trace_context_propagator.go @@ -47,7 +47,7 @@ func (hp httpTraceContextPropagator) Inject(ctx context.Context, supplier apipro sc.TraceID.High, sc.TraceID.Low, sc.SpanID, - sc.TraceOptions) + sc.TraceOptions&core.TraceOptionSampled) supplier.Set(traceparentHeader, h) } } diff --git a/propagation/http_trace_context_propagator_test.go b/propagation/http_trace_context_propagator_test.go index 27b4cf61de5..65be3b95b33 100644 --- a/propagation/http_trace_context_propagator_test.go +++ b/propagation/http_trace_context_propagator_test.go @@ -152,15 +152,15 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { header: "00-ab000000000000000000000000000000-cd00000000000000-0100", }, { - name: "bogus version length", + name: "bogus version", header: "qw-00000000000000000000000000000000-0000000000000000-01", }, { - name: "bogus trace ID length", + name: "bogus trace ID", header: "00-qw000000000000000000000000000000-cd00000000000000-01", }, { - name: "bogus span ID length", + name: "bogus span ID", header: "00-ab000000000000000000000000000000-qw00000000000000-01", }, { @@ -244,6 +244,15 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { }, wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000002-00", }, + { + name: "valid spancontext, with unsupported bit set in traceoption", + sc: core.SpanContext{ + TraceID: traceID, + SpanID: spanID, + TraceOptions: 0xff, + }, + wantHeader: "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000003-01", + }, { name: "invalid spancontext", sc: core.EmptySpanContext(), From 064af5ede56f0b39318c821fa24ca22d282be97d Mon Sep 17 00:00:00 2001 From: rahulpa Date: Mon, 23 Sep 2019 09:10:42 -0700 Subject: [PATCH 27/28] add AddLink and Link interface to MockSpan --- internal/trace/mock_span.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/trace/mock_span.go b/internal/trace/mock_span.go index 84ded071c16..04858b51d99 100644 --- a/internal/trace/mock_span.go +++ b/internal/trace/mock_span.go @@ -86,3 +86,11 @@ func (ms *MockSpan) Tracer() apitrace.Tracer { // AddEvent does nothing. func (ms *MockSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyValue) { } + +// AddLink does nothing. +func (ms *MockSpan) AddLink(link apitrace.Link) { +} + +// Link does nothing. +func (ms *MockSpan) Link(sc core.SpanContext, attrs ...core.KeyValue) { +} From ed41e82645238e359acfa26f70a5d20ca20d4386 Mon Sep 17 00:00:00 2001 From: rahulpa Date: Mon, 23 Sep 2019 10:03:43 -0700 Subject: [PATCH 28/28] fix running go mod tidy on every build. --- Makefile | 3 --- example/basic/go.sum | 1 - example/http/go.sum | 2 -- go.mod | 1 - go.sum | 2 -- 5 files changed, 9 deletions(-) diff --git a/Makefile b/Makefile index 29fff1c613d..1c5b8d8c463 100644 --- a/Makefile +++ b/Makefile @@ -33,9 +33,6 @@ precommit: $(TOOLS_DIR)/golangci-lint $(TOOLS_DIR)/misspell $(TOOLS_DIR)/string PATH="$(abspath $(TOOLS_DIR)):$${PATH}" go generate ./... $(TOOLS_DIR)/golangci-lint run --fix # TODO: Fix this on windows. $(TOOLS_DIR)/misspell -w $(ALL_DOCS) - -.PHONY: mod-tidy -mod-tidy: for dir in $(ALL_GO_MOD_DIRS); do \ echo "go mod tidy in $${dir}"; \ (cd "$${dir}" && go mod tidy); \ diff --git a/example/basic/go.sum b/example/basic/go.sum index 8c722a62d3b..cf930d5b072 100644 --- a/example/basic/go.sum +++ b/example/basic/go.sum @@ -77,7 +77,6 @@ github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgo github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac/go.mod h1:Frd2bnT3w5FB5q49ENTfVlztJES+1k/7lyWX2+9gq/M= github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/magiconair/properties v1.7.6/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= diff --git a/example/http/go.sum b/example/http/go.sum index 640c478f697..8af8206fe58 100644 --- a/example/http/go.sum +++ b/example/http/go.sum @@ -81,8 +81,6 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac h1:+2b6iGRJe3hvV/yVXrd41yVEjxuFHxasJqDhkIjS4gk= -github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac/go.mod h1:Frd2bnT3w5FB5q49ENTfVlztJES+1k/7lyWX2+9gq/M= github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/magiconair/properties v1.7.6/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= diff --git a/go.mod b/go.mod index 052874d78bf..638e0be25a7 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/golangci/golangci-lint v1.17.1 github.com/google/go-cmp v0.3.0 github.com/hashicorp/golang-lru v0.5.3 - github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 google.golang.org/api v0.9.0 google.golang.org/grpc v1.22.1 diff --git a/go.sum b/go.sum index b03d2e062f0..d1d45805931 100644 --- a/go.sum +++ b/go.sum @@ -123,8 +123,6 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac h1:+2b6iGRJe3hvV/yVXrd41yVEjxuFHxasJqDhkIjS4gk= -github.com/lightstep/tracecontext.go v0.0.0-20181129014701-1757c391b1ac/go.mod h1:Frd2bnT3w5FB5q49ENTfVlztJES+1k/7lyWX2+9gq/M= github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/magiconair/properties v1.7.6 h1:U+1DqNen04MdEPgFiIwdOUiqZ8qPa37xgogX/sd3+54= github.com/magiconair/properties v1.7.6/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=