Do not use global tracer provider (#1835)

Signed-off-by: Kemal Akkoyun <[email protected]> - Use passed down trace provider rather than implicitly depending on the global one. - Make sure the analytics component has proper context propagation.
parca-dev · Jul 17, 2023 · d6e4401 · d6e4401
2 parents d969183 + 23d540a
commit d6e4401
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 14 deletions.
diff --git a/cmd/parca-agent/main.go b/cmd/parca-agent/main.go
@@ -41,9 +41,11 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/collectors"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	promconfig "github.com/prometheus/common/config"
 	"github.com/prometheus/procfs"
 	"github.com/prometheus/prometheus/promql/parser"
 	"github.com/zcalusic/sysinfo"
+	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/otel/trace"
 	"go.uber.org/automaxprocs/maxprocs"
 	"google.golang.org/grpc"
@@ -487,7 +489,14 @@ func run(logger log.Logger, reg *prometheus.Registry, flags flags) error {
 	if !flags.AnalyticsOptOut {
 		logger := log.With(logger, "group", "analytics")
 		c := analytics.NewClient(
-			http.DefaultClient,
+			tp,
+			&http.Client{
+				Transport: otelhttp.NewTransport(
+					promconfig.NewUserAgentRoundTripper(
+						fmt.Sprintf("parca.dev/analytics-client/%s", version),
+						http.DefaultTransport),
+				),
+			},
 			"parca-agent",
 			time.Second*5,
 		)
@@ -672,7 +681,7 @@ func run(logger log.Logger, reg *prometheus.Registry, flags flags) error {
 	if !flags.Debuginfo.UploadDisable {
 		dbginfo = debuginfo.New(
 			log.With(logger, "component", "debuginfo"),
-			tp.Tracer("debuginfo"),
+			tp,
 			reg,
 			ofp,
 			debuginfoClient,

diff --git a/go.mod b/go.mod
@@ -39,6 +39,7 @@ require (
 	go.buf.build/protocolbuffers/go/prometheus/prometheus v1.3.9
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.42.0
 	go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.42.0
+	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0
 	go.opentelemetry.io/otel v1.16.0
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.16.0
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.16.0
@@ -94,6 +95,7 @@ require (
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/efficientgo/core v1.0.0-rc.0.0.20221201130417-ba593f67d2a4 // indirect
 	github.com/emicklei/go-restful/v3 v3.10.1 // indirect
+	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/go-delve/delve v1.20.2 // indirect
 	github.com/go-logfmt/logfmt v0.6.0 // indirect
 	github.com/go-logr/logr v1.2.4 // indirect
@@ -119,6 +121,7 @@ require (
 	github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab // indirect
 	github.com/imdario/mergo v0.3.13 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
+	github.com/jpillora/backoff v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/klauspost/cpuid/v2 v2.2.4 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
@@ -134,6 +137,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/mozillazg/go-httpheader v0.2.1 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect
 	github.com/nanmu42/limitio v1.0.0 // indirect
 	github.com/ncw/swift v1.0.53 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect

diff --git a/go.sum b/go.sum
@@ -180,6 +180,8 @@ github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/envoyproxy/protoc-gen-validate v1.0.1 h1:kt9FtLiooDc0vbwTLhdg3dyNX1K9Qwa1EK9LcD4jVUQ=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
+github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
+github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/flowstack/go-jsonschema v0.1.1/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0=
 github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k=
 github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA=
@@ -366,6 +368,7 @@ github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA=
+github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4=
 github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
 github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
 github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
@@ -442,6 +445,7 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU=
+github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/nanmu42/limitio v1.0.0 h1:dpopBYPwUyLOPv+vsGja0iax+dG0SP9paTEmz+Sy7KU=
 github.com/nanmu42/limitio v1.0.0/go.mod h1:8H40zQ7pqxzbwZ9jxsK2hDoE06TH5ziybtApt1io8So=
 github.com/ncw/swift v1.0.53 h1:luHjjTNtekIEvHg5KdAFIBaH7bWfNkefwFnpDffSIks=
@@ -592,6 +596,8 @@ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.4
 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.42.0/go.mod h1:5z+/ZWJQKXa9YT34fQNx5K8Hd1EoIhvtUygUQPqEOgQ=
 go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.42.0 h1:0vzgiFDsCh/jxRCR1xcRrtMoeCu2itXz/PsXst5P8rI=
 go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.42.0/go.mod h1:y0vOY2OKFMOTvwxKfurStPayUUKGHlNeVqNneHmFXr0=
+go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 h1:pginetY7+onl4qN1vl0xW/V/v6OBZ0vVdH+esuJgvmM=
+go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0/go.mod h1:XiYsayHc36K3EByOO6nbAXnAWbrUxdjUROCEeeROOH8=
 go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s=
 go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4=
 go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.16.0 h1:t4ZwRPU+emrcvM2e9DHd0Fsf0JTPVcbfa/BhTDF03d0=

diff --git a/pkg/analytics/remote_write.go b/pkg/analytics/remote_write.go
@@ -20,13 +20,14 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptrace"
 	"os"
 	"time"
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/golang/snappy"
 	"go.buf.build/protocolbuffers/go/prometheus/prometheus"
-	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -36,6 +37,9 @@ const (
 )
 
 type Client struct {
+	tp     trace.TracerProvider
+	tracer trace.Tracer
+
 	client    *http.Client
 	urlString string
 	userAgent string
@@ -46,6 +50,7 @@ type Client struct {
 }
 
 func NewClient(
+	tp trace.TracerProvider,
 	client *http.Client,
 	userAgent string,
 	timeout time.Duration,
@@ -57,6 +62,9 @@ func NewClient(
 	}
 
 	return &Client{
+		tp:     tp,
+		tracer: tp.Tracer("parca/analytics"),
+
 		client: client,
 
 		urlString: analyticsURL,
@@ -69,6 +77,9 @@ func NewClient(
 }
 
 func (c *Client) Send(ctx context.Context, wreq *prometheus.WriteRequest) error {
+	ctx, span := c.tracer.Start(ctx, "Send", trace.WithSpanKind(trace.SpanKindClient))
+	defer span.End()
+
 	c.pBuf.Reset()
 	err := c.pBuf.Marshal(wreq)
 	if err != nil {
@@ -88,6 +99,9 @@ func (c *Client) Send(ctx context.Context, wreq *prometheus.WriteRequest) error
 // Store sends a batch of samples to the HTTP endpoint, the request is the proto marshaled
 // and encoded bytes from codec.go.
 func (c *Client) sendReq(ctx context.Context, req []byte) error {
+	ctx, span := c.tracer.Start(ctx, "sendReq", trace.WithSpanKind(trace.SpanKindClient))
+	defer span.End()
+
 	httpReq, err := http.NewRequest(http.MethodPost, c.urlString, bytes.NewReader(req))
 	if err != nil {
 		// Errors from NewRequest are from unparsable URLs, so are not
@@ -99,11 +113,10 @@ func (c *Client) sendReq(ctx context.Context, req []byte) error {
 	httpReq.Header.Set("Content-Type", "application/x-protobuf")
 	httpReq.Header.Set("User-Agent", c.userAgent)
 	httpReq.Header.Set("X-Prometheus-Remote-Write-Version", "0.1.0")
+
 	ctx, cancel := context.WithTimeout(ctx, c.timeout)
 	defer cancel()
-
-	ctx, span := otel.Tracer("").Start(ctx, "Remote Store", trace.WithSpanKind(trace.SpanKindClient))
-	defer span.End()
+	ctx = httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithTracerProvider(c.tp)))
 
 	httpResp, err := c.client.Do(httpReq.WithContext(ctx))
 	if err != nil {

diff --git a/pkg/debuginfo/manager.go b/pkg/debuginfo/manager.go
@@ -56,6 +56,7 @@ type Cache[K comparable, V any] interface {
 // Manager is a mechanism for extracting or finding the relevant debug information for the discovered executables.
 type Manager struct {
 	logger  log.Logger
+	tp      trace.TracerProvider
 	tracer  trace.Tracer
 	metrics *metrics
 
@@ -86,7 +87,7 @@ type Manager struct {
 // New creates a new Manager.
 func New(
 	logger log.Logger,
-	tracer trace.Tracer,
+	tp trace.TracerProvider,
 	reg prometheus.Registerer,
 	objFilePool *objectfile.Pool,
 	debuginfoClient debuginfopb.DebuginfoServiceClient,
@@ -105,8 +106,10 @@ func New(
 			5*time.Minute,
 		)
 	}
+	tracer := tp.Tracer("debuginfo")
 	return &Manager{
 		logger:      logger,
+		tp:          tp,
 		tracer:      tracer,
 		metrics:     newMetrics(reg),
 		objFilePool: objFilePool,
@@ -522,14 +525,11 @@ func (di *Manager) uploadViaSignedURL(ctx context.Context, url string, r io.Read
 	ctx, span := di.tracer.Start(ctx, "DebuginfoManager.uploadViaSignedURL")
 	defer span.End()
 
-	// Uses the default tracer provider and propagator that's set in tracer package,
-	// or from the span context passed in.
-	ctx = httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx))
-
 	// Client is closing the reader if the reader is also closer.
 	// We need to wrap the reader to avoid this.
 	// We want to have total control over the reader.
 	r = bufio.NewReader(r)
+	ctx = httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithTracerProvider(di.tp)))
 	req, err := http.NewRequestWithContext(ctx, http.MethodPut, url, r)
 	if err != nil {
 		return fmt.Errorf("create request: %w", err)

diff --git a/pkg/debuginfo/manager_test.go b/pkg/debuginfo/manager_test.go
@@ -66,7 +66,7 @@ func BenchmarkUploadInitiateUploadError(b *testing.B) {
 	}
 	debuginfoManager := New(
 		log.NewNopLogger(),
-		trace.NewNoopTracerProvider().Tracer("test"),
+		trace.NewNoopTracerProvider(),
 		prometheus.NewRegistry(),
 		objFilePool,
 		c,
@@ -149,7 +149,7 @@ func TestUpload(t *testing.T) {
 	// Create a Manager instance.
 	dim := New(
 		log.NewNopLogger(),
-		trace.NewNoopTracerProvider().Tracer("test"),
+		trace.NewNoopTracerProvider(),
 		prometheus.NewRegistry(),
 		objFilePool,
 		c,
@@ -270,7 +270,7 @@ func TestUploadSingleFlight(t *testing.T) {
 	// Create a Manager instance.
 	dim := New(
 		log.NewNopLogger(),
-		trace.NewNoopTracerProvider().Tracer("test"),
+		trace.NewNoopTracerProvider(),
 		prometheus.NewRegistry(),
 		objFilePool,
 		c,