Specify tracing span kind on creation

OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

filters/shedder/admission.go

@@ -456,7 +456,6 @@ func (ac *admissionControl) startSpan(ctx context.Context) (span opentracing.Spa
 	if parent != nil {
 		span = ac.tracer.StartSpan(admissionControlSpanName, opentracing.ChildOf(parent.Context()))
 		ext.Component.Set(span, "skipper")
-		ext.SpanKind.Set(span, "shedder")
 		span.SetTag("mode", ac.mode.String())
 	}
 	return

net/httpclient.go

@@ -381,20 +381,17 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
 }
 
 func (t *Transport) injectSpan(req *http.Request) (*http.Request, opentracing.Span) {
-	parentSpan := opentracing.SpanFromContext(req.Context())
-	var span opentracing.Span
-	if parentSpan != nil {
+	spanOpts := []opentracing.StartSpanOption{opentracing.Tags{
+		string(ext.Component):  t.componentName,
+		string(ext.SpanKind):   "client",
+		string(ext.HTTPMethod): req.Method,
+		string(ext.HTTPUrl):    req.URL.String(),
+	}}
+	if parentSpan := opentracing.SpanFromContext(req.Context()); parentSpan != nil {
 		req = req.WithContext(opentracing.ContextWithSpan(req.Context(), parentSpan))
-		span = t.tracer.StartSpan(t.spanName, opentracing.ChildOf(parentSpan.Context()))
-	} else {
-		span = t.tracer.StartSpan(t.spanName)
+		spanOpts = append(spanOpts, opentracing.ChildOf(parentSpan.Context()))
 	}
-
-	// add Tags
-	ext.Component.Set(span, t.componentName)
-	ext.HTTPUrl.Set(span, req.URL.String())
-	ext.HTTPMethod.Set(span, req.Method)
-	ext.SpanKind.Set(span, "client")
+	span := t.tracer.StartSpan(t.spanName, spanOpts...)
 
 	_ = t.tracer.Inject(span.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header))
 

proxy/proxy.go

@@ -958,14 +958,19 @@ func (p *Proxy) makeBackendRequest(ctx *context, requestContext stdlibcontext.Co
 	if !ok {
 		spanName = "proxy"
 	}
-	ctx.proxySpan = tracing.CreateSpan(spanName, req.Context(), p.tracing.tracer)
+
+	proxySpanOpts := []ot.StartSpanOption{ot.Tags{
+		SpanKindTag:       SpanKindClient,
+		SkipperRouteIDTag: ctx.route.Id,
+	}}
+	if parentSpan := ot.SpanFromContext(req.Context()); parentSpan != nil {
+		proxySpanOpts = append(proxySpanOpts, ot.ChildOf(parentSpan.Context()))
+	}
+	ctx.proxySpan = p.tracing.tracer.StartSpan(spanName, proxySpanOpts...)
 
 	u := cloneURL(req.URL)
 	u.RawQuery = ""
-	p.tracing.
-		setTag(ctx.proxySpan, SpanKindTag, SpanKindClient).
-		setTag(ctx.proxySpan, SkipperRouteIDTag, ctx.route.Id).
-		setTag(ctx.proxySpan, HTTPUrlTag, u.String())
+	p.tracing.setTag(ctx.proxySpan, HTTPUrlTag, u.String())
 	p.setCommonSpanInfo(u, req, ctx.proxySpan)
 
 	carrier := ot.HTTPHeadersCarrier(req.Header)
@@ -1181,10 +1186,16 @@ func (p *Proxy) do(ctx *context, parentSpan ot.Span) (err error) {
 		ctx.ensureDefaultResponse()
 	} else if ctx.route.BackendType == eskip.LoopBackend {
 		loopCTX := ctx.clone()
-		loopSpan := tracing.CreateSpan("loopback", ctx.request.Context(), p.tracing.tracer)
-		p.tracing.
-			setTag(loopSpan, SpanKindTag, SpanKindServer).
-			setTag(loopSpan, SkipperRouteIDTag, ctx.route.Id)
+
+		loopSpanOpts := []ot.StartSpanOption{ot.Tags{
+			SpanKindTag:       SpanKindServer,
+			SkipperRouteIDTag: ctx.route.Id,
+		}}
+		if parentSpan := ot.SpanFromContext(ctx.request.Context()); parentSpan != nil {
+			loopSpanOpts = append(loopSpanOpts, ot.ChildOf(parentSpan.Context()))
+		}
+		loopSpan := p.tracing.tracer.StartSpan("loopback", loopSpanOpts...)
+
 		p.setCommonSpanInfo(ctx.Request().URL, ctx.Request(), loopSpan)
 		ctx.parentSpan = loopSpan
 
@@ -1481,12 +1492,15 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	p.metrics.IncCounter("incoming." + r.Proto)
 	var ctx *context
 
-	var span ot.Span
-	if wireContext, err := p.tracing.tracer.Extract(ot.HTTPHeaders, ot.HTTPHeadersCarrier(r.Header)); err != nil {
-		span = p.tracing.tracer.StartSpan(p.tracing.initialOperationName)
-	} else {
-		span = p.tracing.tracer.StartSpan(p.tracing.initialOperationName, ext.RPCServerOption(wireContext))
+	spanOpts := []ot.StartSpanOption{ot.Tags{
+		SpanKindTag:     SpanKindServer,
+		HTTPRemoteIPTag: stripPort(r.RemoteAddr),
+	}}
+	if wireContext, err := p.tracing.tracer.Extract(ot.HTTPHeaders, ot.HTTPHeadersCarrier(r.Header)); err == nil {
+		spanOpts = append(spanOpts, ext.RPCServerOption(wireContext))
 	}
+	span := p.tracing.tracer.StartSpan(p.tracing.initialOperationName, spanOpts...)
+
 	defer func() {
 		if ctx != nil && ctx.proxySpan != nil {
 			ctx.proxySpan.Finish()
@@ -1533,9 +1547,6 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		r.URL.Path = rfc.PatchPath(r.URL.Path, r.URL.RawPath)
 	}
 
-	p.tracing.
-		setTag(span, SpanKindTag, SpanKindServer).
-		setTag(span, HTTPRemoteIPTag, stripPort(r.RemoteAddr))
 	p.setCommonSpanInfo(r.URL, r, span)
 	r = r.WithContext(ot.ContextWithSpan(r.Context(), span))
 	r = r.WithContext(routing.NewContext(r.Context()))

ratelimit/leakybucket.go

@@ -112,13 +112,12 @@ func (b *ClusterLeakyBucket) getBucketId(label string) string {
 }
 
 func (b *ClusterLeakyBucket) startSpan(ctx context.Context) (span opentracing.Span) {
-	parent := opentracing.SpanFromContext(ctx)
-	if parent != nil {
-		span = b.ringClient.StartSpan(leakyBucketSpanName, opentracing.ChildOf(parent.Context()))
-	} else {
-		span = opentracing.NoopTracer{}.StartSpan("")
+	spanOpts := []opentracing.StartSpanOption{opentracing.Tags{
+		string(ext.Component): "skipper",
+		string(ext.SpanKind):  "client",
+	}}
+	if parent := opentracing.SpanFromContext(ctx); parent != nil {
+		spanOpts = append(spanOpts, opentracing.ChildOf(parent.Context()))
 	}
-	ext.Component.Set(span, "skipper")
-	ext.SpanKind.Set(span, "client")
-	return
+	return b.ringClient.StartSpan(leakyBucketSpanName, spanOpts...)
 }

ratelimit/redis.go

@@ -84,14 +84,14 @@ func parentSpan(ctx context.Context) opentracing.Span {
 	return opentracing.SpanFromContext(ctx)
 }
 
-func (c *clusterLimitRedis) setCommonTags(span opentracing.Span) {
-	if span != nil {
-		ext.Component.Set(span, "skipper")
-		ext.SpanKind.Set(span, "client")
-		span.SetTag("ratelimit_type", c.typ)
-		span.SetTag("group", c.group)
-		span.SetTag("max_hits", c.maxHits)
-		span.SetTag("window", c.window.String())
+func (c *clusterLimitRedis) commonTags() opentracing.Tags {
+	return opentracing.Tags{
+		string(ext.Component): "skipper",
+		string(ext.SpanKind):  "client",
+		"ratelimit_type":      c.typ,
+		"group":               c.group,
+		"max_hits":            c.maxHits,
+		"window":              c.window.String(),
 	}
 }
 
@@ -114,10 +114,9 @@ func (c *clusterLimitRedis) Allow(ctx context.Context, clearText string) bool {
 
 	var span opentracing.Span
 	if parentSpan := parentSpan(ctx); parentSpan != nil {
-		span = c.ringClient.StartSpan(allowSpanName, opentracing.ChildOf(parentSpan.Context()))
+		span = c.ringClient.StartSpan(allowSpanName, opentracing.ChildOf(parentSpan.Context()), c.commonTags())
 		defer span.Finish()
 	}
-	c.setCommonTags(span)
 
 	allow, err := c.allow(ctx, clearText)
 	failed := err != nil
@@ -227,10 +226,9 @@ func (c *clusterLimitRedis) oldest(ctx context.Context, clearText string) (time.
 
 	var span opentracing.Span
 	if parentSpan := parentSpan(ctx); parentSpan != nil {
-		span = c.ringClient.StartSpan(oldestScoreSpanName, opentracing.ChildOf(parentSpan.Context()))
+		span = c.ringClient.StartSpan(oldestScoreSpanName, opentracing.ChildOf(parentSpan.Context()), c.commonTags())
 		defer span.Finish()
 	}
-	c.setCommonTags(span)
 
 	res, err := c.ringClient.ZRangeByScoreWithScoresFirst(ctx, key, 0.0, float64(now.UnixNano()), 0, 1)