fix: allow sending otel tracing to non honeycomb backend (#1219)

<!--
Thank you for contributing to the project! 💜
Please make sure to:
- Chat with us first if this is a big change
  - Open a new issue (or comment on an existing one)
- We want to make sure you don't spend time implementing something we
might have to say No to
- Add unit tests
- Mention any relevant issues in the PR description (e.g. "Fixes #123")

Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

## Which problem is this PR solving?
`OtelTracing.APIKey` is specifically for setting a honeycomb api key and
header when setting up a tracer. When it's not set and
`OtelTracing.Enabled` is set true, Refinery should still set up a tracer
that can be used to send Refinery traces to non-honeycomb backend.

## Short description of the changes

- Only set APIKey headers when APIKey is not empty
- add some basic instrumentation

app/app_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/klauspost/compress/zstd"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel/trace/noop"
 	"gopkg.in/alexcesaro/statsd.v2"
 
 	"github.com/honeycombio/libhoney-go"
@@ -192,6 +193,7 @@ func newStartedApp(
 		&inject.Object{Value: transmit.NewDefaultTransmission(upstreamClient, metricsr, "upstream"), Name: "upstreamTransmission"},
 		&inject.Object{Value: transmit.NewDefaultTransmission(peerClient, metricsr, "peer"), Name: "peerTransmission"},
 		&inject.Object{Value: shrdr},
+		&inject.Object{Value: noop.NewTracerProvider().Tracer("test"), Name: "tracer"},
 		&inject.Object{Value: collector},
 		&inject.Object{Value: metricsr, Name: "metrics"},
 		&inject.Object{Value: metricsr, Name: "genericMetrics"},

collect/collect.go

@@ -1,6 +1,7 @@
 package collect
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
@@ -9,9 +10,12 @@ import (
 	"sync"
 	"time"
 
+	"go.opentelemetry.io/otel/trace"
+
 	"github.com/honeycombio/refinery/collect/cache"
 	"github.com/honeycombio/refinery/config"
 	"github.com/honeycombio/refinery/generics"
+	"github.com/honeycombio/refinery/internal/otelutil"
 	"github.com/honeycombio/refinery/logger"
 	"github.com/honeycombio/refinery/metrics"
 	"github.com/honeycombio/refinery/sample"
@@ -50,6 +54,7 @@ const (
 type InMemCollector struct {
 	Config         config.Config          `inject:""`
 	Logger         logger.Logger          `inject:""`
+	Tracer         trace.Tracer           `inject:"tracer"`
 	Transmission   transmit.Transmission  `inject:"upstreamTransmission"`
 	Metrics        metrics.Metrics        `inject:"genericMetrics"`
 	SamplerFactory *sample.SamplerFactory `inject:""`
@@ -381,7 +386,9 @@ func (i *InMemCollector) sendTracesInCache(now time.Time) {
 // processSpan does all the stuff necessary to take an incoming span and add it
 // to (or create a new placeholder for) a trace.
 func (i *InMemCollector) processSpan(sp *types.Span) {
+	ctx, span := otelutil.StartSpan(context.Background(), i.Tracer, "processSpan")
 	defer func() {
+		span.End()
 		i.Metrics.Increment("span_processed")
 		i.Metrics.Down("spans_waiting")
 	}()
@@ -394,7 +401,7 @@ func (i *InMemCollector) processSpan(sp *types.Span) {
 			// bump the count of records on this trace -- if the root span isn't
 			// the last late span, then it won't be perfect, but it will be better than
 			// having none at all
-			i.dealWithSentTrace(sr, sentReason, sp)
+			i.dealWithSentTrace(ctx, sr, sentReason, sp)
 			return
 		}
 		// trace hasn't already been sent (or this span is really old); let's
@@ -427,13 +434,13 @@ func (i *InMemCollector) processSpan(sp *types.Span) {
 	if trace.Sent {
 		if sr, reason, found := i.sampleTraceCache.Check(sp); found {
 			i.Metrics.Increment("trace_sent_cache_hit")
-			i.dealWithSentTrace(sr, reason, sp)
+			i.dealWithSentTrace(ctx, sr, reason, sp)
 			return
 		}
 		// trace has already been sent, but this is not in the sent cache.
 		// we will just use the default late span reason as the sent reason which is
 		// set inside the dealWithSentTrace function
-		i.dealWithSentTrace(cache.NewKeptTraceCacheEntry(trace), "", sp)
+		i.dealWithSentTrace(ctx, cache.NewKeptTraceCacheEntry(trace), "", sp)
 	}
 
 	// great! trace is live. add the span.
@@ -495,7 +502,14 @@ func (i *InMemCollector) ProcessSpanImmediately(sp *types.Span, keep bool, sampl
 // dealWithSentTrace handles a span that has arrived after the sampling decision
 // on the trace has already been made, and it obeys that decision by either
 // sending the span immediately or dropping it.
-func (i *InMemCollector) dealWithSentTrace(tr cache.TraceSentRecord, sentReason string, sp *types.Span) {
+func (i *InMemCollector) dealWithSentTrace(ctx context.Context, tr cache.TraceSentRecord, sentReason string, sp *types.Span) {
+	ctx, span := otelutil.StartSpanMulti(ctx, i.Tracer, "dealWithSentTrace", map[string]interface{}{
+		"trace_id":    sp.TraceID,
+		"sent_reason": sentReason,
+		"hostname":    i.hostname,
+	})
+	defer span.End()
+
 	if i.Config.GetAddRuleReasonToTrace() {
 		var metaReason string
 		if len(sentReason) > 0 {
@@ -512,6 +526,10 @@ func (i *InMemCollector) dealWithSentTrace(tr cache.TraceSentRecord, sentReason
 	}
 	isDryRun := i.Config.GetIsDryRun()
 	keep := tr.Kept()
+	otelutil.AddSpanFields(span, map[string]interface{}{
+		"keep":      keep,
+		"is_dryrun": isDryRun,
+	})
 
 	if isDryRun {
 		// if dry run mode is enabled, we keep all traces and mark the spans with the sampling decision
@@ -528,7 +546,8 @@ func (i *InMemCollector) dealWithSentTrace(tr cache.TraceSentRecord, sentReason
 		i.Logger.Debug().WithField("trace_id", sp.TraceID).Logf("Sending span because of previous decision to send trace")
 		mergeTraceAndSpanSampleRates(sp, tr.Rate(), isDryRun)
 		// if this span is a late root span, possibly update it with our current span count
-		if i.isRootSpan(sp) {
+		isRootSpan := i.isRootSpan(sp)
+		if isRootSpan {
 			if i.Config.GetAddCountsToRoot() {
 				sp.Data["meta.span_event_count"] = int64(tr.SpanEventCount())
 				sp.Data["meta.span_link_count"] = int64(tr.SpanLinkCount())
@@ -537,8 +556,8 @@ func (i *InMemCollector) dealWithSentTrace(tr cache.TraceSentRecord, sentReason
 			} else if i.Config.GetAddSpanCountToRoot() {
 				sp.Data["meta.span_count"] = int64(tr.DescendantCount())
 			}
-
 		}
+		otelutil.AddSpanField(span, "is_root_span", isRootSpan)
 		i.Metrics.Increment(TraceSendLateSpan)
 		i.addAdditionalAttributes(sp)
 		i.Transmission.EnqueueSpan(sp)

collect/collect_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/facebookgo/inject"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel/trace/noop"
 
 	"github.com/honeycombio/refinery/collect/cache"
 	"github.com/honeycombio/refinery/config"
@@ -41,6 +42,7 @@ func newTestCollector(conf config.Config, transmission transmit.Transmission) *I
 	return &InMemCollector{
 		Config:       conf,
 		Logger:       &logger.NullLogger{},
+		Tracer:       noop.NewTracerProvider().Tracer("test"),
 		Transmission: transmission,
 		Metrics:      &metrics.NullMetrics{},
 		StressRelief: &MockStressReliever{},
@@ -743,6 +745,7 @@ func TestDependencyInjection(t *testing.T) {
 		&inject.Object{Value: &InMemCollector{}},
 		&inject.Object{Value: &config.MockConfig{}},
 		&inject.Object{Value: &logger.NullLogger{}},
+		&inject.Object{Value: noop.NewTracerProvider().Tracer("test"), Name: "tracer"},
 		&inject.Object{Value: &transmit.MockTransmission{}, Name: "upstreamTransmission"},
 		&inject.Object{Value: &metrics.NullMetrics{}, Name: "genericMetrics"},
 		&inject.Object{Value: &sample.SamplerFactory{}},

internal/otelutil/otel_tracing.go

@@ -77,41 +77,46 @@ func StartSpanMulti(ctx context.Context, tracer trace.Tracer, name string, field
 }
 
 func SetupTracing(cfg config.OTelTracingConfig, resourceLibrary string, resourceVersion string) (tracer trace.Tracer, shutdown func()) {
-	if cfg.APIKey != "" {
-		var protocol otelconfig.Protocol = otelconfig.ProtocolHTTPProto
+	if !cfg.Enabled {
+		pr := noop.NewTracerProvider()
+		return pr.Tracer(resourceLibrary, trace.WithInstrumentationVersion(resourceVersion)), func() {}
+	}
 
-		cfg.APIHost = strings.TrimSuffix(cfg.APIHost, "/")
-		apihost := fmt.Sprintf("%s:443", cfg.APIHost)
+	var protocol otelconfig.Protocol = otelconfig.ProtocolHTTPProto
 
-		sampleRate := cfg.SampleRate
-		if sampleRate < 1 {
-			sampleRate = 1
-		}
+	cfg.APIHost = strings.TrimSuffix(cfg.APIHost, "/")
+	apihost := fmt.Sprintf("%s:443", cfg.APIHost)
+
+	sampleRate := cfg.SampleRate
+	if sampleRate < 1 {
+		sampleRate = 1
+	}
 
-		var sampleRatio float64 = 1.0 / float64(sampleRate)
+	var sampleRatio float64 = 1.0 / float64(sampleRate)
 
-		headers := map[string]string{
+	// set up honeycomb specific headers if an API key is provided
+	headers := make(map[string]string)
+	if cfg.APIKey != "" {
+		headers = map[string]string{
 			types.APIKeyHeader: cfg.APIKey,
 		}
 
 		if types.IsLegacyAPIKey(cfg.APIKey) {
 			headers[types.DatasetHeader] = cfg.Dataset
 		}
+	}
 
-		otelshutdown, err := otelconfig.ConfigureOpenTelemetry(
-			otelconfig.WithExporterProtocol(protocol),
-			otelconfig.WithServiceName(cfg.Dataset),
-			otelconfig.WithTracesExporterEndpoint(apihost),
-			otelconfig.WithMetricsEnabled(false),
-			otelconfig.WithTracesEnabled(true),
-			otelconfig.WithSampler(samplers.TraceIDRatioBased(sampleRatio)),
-			otelconfig.WithHeaders(headers),
-		)
-		if err != nil {
-			log.Fatalf("failure configuring otel: %v", err)
-		}
-		return otel.Tracer(resourceLibrary, trace.WithInstrumentationVersion(resourceVersion)), otelshutdown
+	otelshutdown, err := otelconfig.ConfigureOpenTelemetry(
+		otelconfig.WithExporterProtocol(protocol),
+		otelconfig.WithServiceName(cfg.Dataset),
+		otelconfig.WithTracesExporterEndpoint(apihost),
+		otelconfig.WithMetricsEnabled(false),
+		otelconfig.WithTracesEnabled(true),
+		otelconfig.WithSampler(samplers.TraceIDRatioBased(sampleRatio)),
+		otelconfig.WithHeaders(headers),
+	)
+	if err != nil {
+		log.Fatalf("failure configuring otel: %v", err)
 	}
-	pr := noop.NewTracerProvider()
-	return pr.Tracer(resourceLibrary, trace.WithInstrumentationVersion(resourceVersion)), func() {}
+	return otel.Tracer(resourceLibrary, trace.WithInstrumentationVersion(resourceVersion)), otelshutdown
 }

route/route_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/honeycombio/refinery/transmit"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel/trace/noop"
 	collectortrace "go.opentelemetry.io/proto/otlp/collector/trace/v1"
 	trace "go.opentelemetry.io/proto/otlp/trace/v1"
 
@@ -474,6 +475,7 @@ func TestDependencyInjection(t *testing.T) {
 
 		&inject.Object{Value: &config.MockConfig{}},
 		&inject.Object{Value: &logger.NullLogger{}},
+		&inject.Object{Value: noop.NewTracerProvider().Tracer("test"), Name: "tracer"},
 		&inject.Object{Value: http.DefaultTransport, Name: "upstreamTransport"},
 		&inject.Object{Value: &transmit.MockTransmission{}, Name: "upstreamTransmission"},
 		&inject.Object{Value: &transmit.MockTransmission{}, Name: "peerTransmission"},