From c8b6653520500700fe14193dc908c8d7632783e3 Mon Sep 17 00:00:00 2001 From: Kyle Nusbaum Date: Fri, 13 Mar 2020 10:39:26 -0500 Subject: [PATCH] contrib: Update non-client contrib packages to measure their spans (#603) This commit marks relevant spans in our integrations to be measured. Previously, choosing which spans are measured was done elsewhere. We are able to do this now that we have support for manually marking spans to be measured (#591). A relevant span is any span that sets a service name, and is likely to have a parent span from the user's code or a different integration. Any spans that will be children of spans from the same integration don't need to be measured. We also don't measure spans that are for integrations for external service clients. This is a very generic class of integrations, and includes any integration for a package that is primarily an API to an external service. This includes SQL integrations, the aws integration, the client portion of the grpc integration, the http client integration, etc. This is actually most of our integrations. The reason for this is that there can be conflicts when measuring client spans that may cause inaccurate metrics. --- contrib/Shopify/sarama/sarama.go | 1 + .../confluentinc/confluent-kafka-go/kafka/kafka.go | 1 + contrib/gin-gonic/gin/gintrace.go | 1 + contrib/go-chi/chi/chi.go | 1 + contrib/google.golang.org/grpc.v12/grpc.go | 1 + contrib/google.golang.org/grpc/client.go | 6 +++--- contrib/google.golang.org/grpc/grpc.go | 10 +++------- contrib/google.golang.org/grpc/server.go | 13 +++++++++---- contrib/google.golang.org/grpc/stats_client.go | 2 +- contrib/google.golang.org/grpc/stats_server.go | 7 ++++--- contrib/google.golang.org/grpc/stats_server_test.go | 1 + contrib/gorilla/mux/mux.go | 1 + contrib/graph-gophers/graphql-go/graphql.go | 2 ++ contrib/julienschmidt/httprouter/httprouter.go | 1 + contrib/labstack/echo/echotrace.go | 1 + contrib/net/http/option.go | 7 ++++--- contrib/twitchtv/twirp/twirp.go | 2 ++ ddtrace/tracer/option.go | 10 ++++++++++ 18 files changed, 47 insertions(+), 21 deletions(-) diff --git a/contrib/Shopify/sarama/sarama.go b/contrib/Shopify/sarama/sarama.go index c2863b801c..933f5532b1 100644 --- a/contrib/Shopify/sarama/sarama.go +++ b/contrib/Shopify/sarama/sarama.go @@ -50,6 +50,7 @@ func WrapPartitionConsumer(pc sarama.PartitionConsumer, opts ...Option) sarama.P tracer.SpanType(ext.SpanTypeMessageConsumer), tracer.Tag("partition", msg.Partition), tracer.Tag("offset", msg.Offset), + tracer.Measured(), } if !math.IsNaN(cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) diff --git a/contrib/confluentinc/confluent-kafka-go/kafka/kafka.go b/contrib/confluentinc/confluent-kafka-go/kafka/kafka.go index 2e1ee886c2..2950d8a9d9 100644 --- a/contrib/confluentinc/confluent-kafka-go/kafka/kafka.go +++ b/contrib/confluentinc/confluent-kafka-go/kafka/kafka.go @@ -93,6 +93,7 @@ func (c *Consumer) startSpan(msg *kafka.Message) ddtrace.Span { tracer.SpanType(ext.SpanTypeMessageConsumer), tracer.Tag("partition", msg.TopicPartition.Partition), tracer.Tag("offset", msg.TopicPartition.Offset), + tracer.Measured(), } if !math.IsNaN(c.cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, c.cfg.analyticsRate)) diff --git a/contrib/gin-gonic/gin/gintrace.go b/contrib/gin-gonic/gin/gintrace.go index ad2a56bef4..f504b9da70 100644 --- a/contrib/gin-gonic/gin/gintrace.go +++ b/contrib/gin-gonic/gin/gintrace.go @@ -33,6 +33,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { tracer.SpanType(ext.SpanTypeWeb), tracer.Tag(ext.HTTPMethod, c.Request.Method), tracer.Tag(ext.HTTPURL, c.Request.URL.Path), + tracer.Measured(), } if !math.IsNaN(cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) diff --git a/contrib/go-chi/chi/chi.go b/contrib/go-chi/chi/chi.go index aed7ae3ddc..f09329c8e5 100644 --- a/contrib/go-chi/chi/chi.go +++ b/contrib/go-chi/chi/chi.go @@ -34,6 +34,7 @@ func Middleware(opts ...Option) func(next http.Handler) http.Handler { tracer.ServiceName(cfg.serviceName), tracer.Tag(ext.HTTPMethod, r.Method), tracer.Tag(ext.HTTPURL, r.URL.Path), + tracer.Measured(), } if !math.IsNaN(cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) diff --git a/contrib/google.golang.org/grpc.v12/grpc.go b/contrib/google.golang.org/grpc.v12/grpc.go index 70fcf3e9b4..9850e7eb2c 100644 --- a/contrib/google.golang.org/grpc.v12/grpc.go +++ b/contrib/google.golang.org/grpc.v12/grpc.go @@ -47,6 +47,7 @@ func startSpanFromContext(ctx context.Context, method, service string, rate floa tracer.ResourceName(method), tracer.Tag(tagMethod, method), tracer.SpanType(ext.AppTypeRPC), + tracer.Measured(), } if !math.IsNaN(rate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, rate)) diff --git a/contrib/google.golang.org/grpc/client.go b/contrib/google.golang.org/grpc/client.go index 375ede1b8a..2a6c18680b 100644 --- a/contrib/google.golang.org/grpc/client.go +++ b/contrib/google.golang.org/grpc/client.go @@ -32,7 +32,7 @@ func (cs *clientStream) RecvMsg(m interface{}) (err error) { cs.method, "grpc.message", cs.cfg.clientServiceName(), - cs.cfg.analyticsRate, + tracer.AnalyticsRate(cs.cfg.analyticsRate), ) if p, ok := peer.FromContext(cs.Context()); ok { setSpanTargetFromPeer(span, *p) @@ -50,7 +50,7 @@ func (cs *clientStream) SendMsg(m interface{}) (err error) { cs.method, "grpc.message", cs.cfg.clientServiceName(), - cs.cfg.analyticsRate, + tracer.AnalyticsRate(cs.cfg.analyticsRate), ) if p, ok := peer.FromContext(cs.Context()); ok { setSpanTargetFromPeer(span, *p) @@ -156,7 +156,7 @@ func doClientRequest( method, "grpc.client", cfg.clientServiceName(), - cfg.analyticsRate, + tracer.AnalyticsRate(cfg.analyticsRate), ) if methodKind != "" { span.SetTag(tagMethodKind, methodKind) diff --git a/contrib/google.golang.org/grpc/grpc.go b/contrib/google.golang.org/grpc/grpc.go index 899286ccf2..7aa0aa8494 100644 --- a/contrib/google.golang.org/grpc/grpc.go +++ b/contrib/google.golang.org/grpc/grpc.go @@ -10,7 +10,6 @@ package grpc // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/google.golang.or import ( "io" - "math" "gopkg.in/DataDog/dd-trace-go.v1/contrib/google.golang.org/internal/grpcutil" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" @@ -24,17 +23,14 @@ import ( ) func startSpanFromContext( - ctx context.Context, method, operation, service string, rate float64, + ctx context.Context, method, operation, service string, opts ...tracer.StartSpanOption, ) (ddtrace.Span, context.Context) { - opts := []ddtrace.StartSpanOption{ + opts = append(opts, tracer.ServiceName(service), tracer.ResourceName(method), tracer.Tag(tagMethodName, method), tracer.SpanType(ext.AppTypeRPC), - } - if !math.IsNaN(rate) { - opts = append(opts, tracer.Tag(ext.EventSampleRate, rate)) - } + ) md, _ := metadata.FromIncomingContext(ctx) // nil is ok if sctx, err := tracer.Extract(grpcutil.MDCarrier(md)); err == nil { opts = append(opts, tracer.ChildOf(sctx)) diff --git a/contrib/google.golang.org/grpc/server.go b/contrib/google.golang.org/grpc/server.go index 5e7ab7d817..da4ee7f145 100644 --- a/contrib/google.golang.org/grpc/server.go +++ b/contrib/google.golang.org/grpc/server.go @@ -7,6 +7,7 @@ package grpc import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" context "golang.org/x/net/context" "google.golang.org/grpc" @@ -38,7 +39,8 @@ func (ss *serverStream) RecvMsg(m interface{}) (err error) { ss.method, "grpc.message", ss.cfg.serverServiceName(), - ss.cfg.analyticsRate, + tracer.AnalyticsRate(ss.cfg.analyticsRate), + tracer.Measured(), ) defer func() { finishWithError(span, err, ss.cfg) }() } @@ -53,7 +55,8 @@ func (ss *serverStream) SendMsg(m interface{}) (err error) { ss.method, "grpc.message", ss.cfg.serverServiceName(), - ss.cfg.analyticsRate, + tracer.AnalyticsRate(ss.cfg.analyticsRate), + tracer.Measured(), ) defer func() { finishWithError(span, err, ss.cfg) }() } @@ -81,7 +84,8 @@ func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor { info.FullMethod, "grpc.server", cfg.serviceName, - cfg.analyticsRate, + tracer.AnalyticsRate(cfg.analyticsRate), + tracer.Measured(), ) switch { case info.IsServerStream && info.IsClientStream: @@ -123,7 +127,8 @@ func UnaryServerInterceptor(opts ...Option) grpc.UnaryServerInterceptor { info.FullMethod, "grpc.server", cfg.serverServiceName(), - cfg.analyticsRate, + tracer.AnalyticsRate(cfg.analyticsRate), + tracer.Measured(), ) span.SetTag(tagMethodKind, methodKindUnary) resp, err := handler(ctx, req) diff --git a/contrib/google.golang.org/grpc/stats_client.go b/contrib/google.golang.org/grpc/stats_client.go index f6e4ee8ccc..cedcd01cac 100644 --- a/contrib/google.golang.org/grpc/stats_client.go +++ b/contrib/google.golang.org/grpc/stats_client.go @@ -36,7 +36,7 @@ func (h *clientStatsHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo) rti.FullMethodName, "grpc.client", h.cfg.clientServiceName(), - h.cfg.analyticsRate, + tracer.AnalyticsRate(h.cfg.analyticsRate), ) ctx = injectSpanIntoContext(ctx) return ctx diff --git a/contrib/google.golang.org/grpc/stats_server.go b/contrib/google.golang.org/grpc/stats_server.go index d3326ceadf..a7c1bee331 100644 --- a/contrib/google.golang.org/grpc/stats_server.go +++ b/contrib/google.golang.org/grpc/stats_server.go @@ -6,10 +6,10 @@ package grpc import ( + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" + context "golang.org/x/net/context" "google.golang.org/grpc/stats" - - "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" ) // NewServerStatsHandler returns a gRPC server stats.Handler to trace RPC calls. @@ -35,7 +35,8 @@ func (h *serverStatsHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo) rti.FullMethodName, "grpc.server", h.cfg.serverServiceName(), - h.cfg.analyticsRate, + tracer.AnalyticsRate(h.cfg.analyticsRate), + tracer.Measured(), ) return ctx } diff --git a/contrib/google.golang.org/grpc/stats_server_test.go b/contrib/google.golang.org/grpc/stats_server_test.go index 96ba80cb05..2ad0c9779e 100644 --- a/contrib/google.golang.org/grpc/stats_server_test.go +++ b/contrib/google.golang.org/grpc/stats_server_test.go @@ -50,6 +50,7 @@ func TestServerStatsHandler(t *testing.T) { "service.name": serviceName, "resource.name": "/grpc.Fixture/Ping", tagMethodName: "/grpc.Fixture/Ping", + "_dd.measured": 1, }, span.Tags()) } diff --git a/contrib/gorilla/mux/mux.go b/contrib/gorilla/mux/mux.go index c7daced416..9f7d996813 100644 --- a/contrib/gorilla/mux/mux.go +++ b/contrib/gorilla/mux/mux.go @@ -82,6 +82,7 @@ func NewRouter(opts ...RouterOption) *Router { if !math.IsNaN(cfg.analyticsRate) { cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } + cfg.spanOpts = append(cfg.spanOpts, tracer.Measured()) return &Router{ Router: mux.NewRouter(), config: cfg, diff --git a/contrib/graph-gophers/graphql-go/graphql.go b/contrib/graph-gophers/graphql-go/graphql.go index d3730627a7..ef7d05caaa 100644 --- a/contrib/graph-gophers/graphql-go/graphql.go +++ b/contrib/graph-gophers/graphql-go/graphql.go @@ -45,6 +45,7 @@ func (t *Tracer) TraceQuery(ctx context.Context, queryString string, operationNa tracer.ServiceName(t.cfg.serviceName), tracer.Tag(tagGraphqlQuery, queryString), tracer.Tag(tagGraphqlOperationName, operationName), + tracer.Measured(), } if !math.IsNaN(t.cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, t.cfg.analyticsRate)) @@ -71,6 +72,7 @@ func (t *Tracer) TraceField(ctx context.Context, label string, typeName string, tracer.ServiceName(t.cfg.serviceName), tracer.Tag(tagGraphqlField, fieldName), tracer.Tag(tagGraphqlType, typeName), + tracer.Measured(), } if !math.IsNaN(t.cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, t.cfg.analyticsRate)) diff --git a/contrib/julienschmidt/httprouter/httprouter.go b/contrib/julienschmidt/httprouter/httprouter.go index d79d6afc3c..c37bcf7d1a 100644 --- a/contrib/julienschmidt/httprouter/httprouter.go +++ b/contrib/julienschmidt/httprouter/httprouter.go @@ -34,6 +34,7 @@ func New(opts ...RouterOption) *Router { if !math.IsNaN(cfg.analyticsRate) { cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } + cfg.spanOpts = append(cfg.spanOpts, tracer.Measured()) return &Router{httprouter.New(), cfg} } diff --git a/contrib/labstack/echo/echotrace.go b/contrib/labstack/echo/echotrace.go index 8df4859193..22da129200 100644 --- a/contrib/labstack/echo/echotrace.go +++ b/contrib/labstack/echo/echotrace.go @@ -34,6 +34,7 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { tracer.SpanType(ext.SpanTypeWeb), tracer.Tag(ext.HTTPMethod, request.Method), tracer.Tag(ext.HTTPURL, request.URL.Path), + tracer.Measured(), } if !math.IsNaN(cfg.analyticsRate) { diff --git a/contrib/net/http/option.go b/contrib/net/http/option.go index b54aa78aff..3f28425290 100644 --- a/contrib/net/http/option.go +++ b/contrib/net/http/option.go @@ -30,8 +30,9 @@ type Option func(*config) func defaults(cfg *config) { cfg.analyticsRate = globalconfig.AnalyticsRate() cfg.serviceName = "http.router" + cfg.spanOpts = []ddtrace.StartSpanOption{tracer.Measured()} if !math.IsNaN(cfg.analyticsRate) { - cfg.spanOpts = []ddtrace.StartSpanOption{tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)} + cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } } @@ -47,7 +48,7 @@ func WithAnalytics(on bool) MuxOption { return func(cfg *config) { if on { cfg.analyticsRate = 1.0 - cfg.spanOpts = []ddtrace.StartSpanOption{tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)} + cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } else { cfg.analyticsRate = math.NaN() } @@ -60,7 +61,7 @@ func WithAnalyticsRate(rate float64) MuxOption { return func(cfg *config) { if rate >= 0.0 && rate <= 1.0 { cfg.analyticsRate = rate - cfg.spanOpts = []ddtrace.StartSpanOption{tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)} + cfg.spanOpts = append(cfg.spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } else { cfg.analyticsRate = math.NaN() } diff --git a/contrib/twitchtv/twirp/twirp.go b/contrib/twitchtv/twirp/twirp.go index cafb8769e7..dae161b840 100644 --- a/contrib/twitchtv/twirp/twirp.go +++ b/contrib/twitchtv/twirp/twirp.go @@ -109,6 +109,7 @@ func WrapServer(h http.Handler, opts ...Option) http.Handler { tracer.ServiceName(cfg.serverServiceName()), tracer.Tag(ext.HTTPMethod, r.Method), tracer.Tag(ext.HTTPURL, r.URL.Path), + tracer.Measured(), } if !math.IsNaN(cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) @@ -155,6 +156,7 @@ func requestReceivedHook(cfg *config) func(context.Context) (context.Context, er opts := []tracer.StartSpanOption{ tracer.SpanType(ext.SpanTypeWeb), tracer.ServiceName(cfg.serverServiceName()), + tracer.Measured(), } if pkg, ok := twirp.PackageName(ctx); ok { opts = append(opts, tracer.Tag("twirp.package", pkg)) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 685cc6ba3f..e66f0e65ec 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -345,6 +345,16 @@ func StartTime(t time.Time) StartSpanOption { } } +// AnalyticsRate sets a custom analytics rate for a span. It decides the percentage +// of events that will be picked up by the App Analytics product. It's represents a +// float64 between 0 and 1 where 0.5 would represent 50% of events. +func AnalyticsRate(rate float64) StartSpanOption { + if math.IsNaN(rate) { + return func(cfg *ddtrace.StartSpanConfig) {} + } + return Tag(ext.EventSampleRate, rate) +} + // FinishOption is a configuration option for FinishSpan. It is aliased in order // to help godoc group all the functions returning it together. It is considered // more correct to refer to it as the type as the origin, ddtrace.FinishOption.