From b0b1e8d568418b15fe734df13c5265644b95bd63 Mon Sep 17 00:00:00 2001
From: Emil Tullstedt <emil.tullstedt@grafana.com>
Date: Wed, 28 Sep 2022 15:53:48 +0200
Subject: [PATCH] Tracing: Document Tracer and Span interfaces (#55926)

---
 pkg/infra/tracing/opentelemetry_tracing.go | 20 ++-----
 pkg/infra/tracing/tracing.go               | 61 ++++++++++++++++++++--
 2 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/pkg/infra/tracing/opentelemetry_tracing.go b/pkg/infra/tracing/opentelemetry_tracing.go
index b51dddac107..2ce2e6ba5f0 100644
--- a/pkg/infra/tracing/opentelemetry_tracing.go
+++ b/pkg/infra/tracing/opentelemetry_tracing.go
@@ -8,8 +8,6 @@ import (
 	"time"
 
 	"github.com/go-kit/log/level"
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/setting"
 	"go.etcd.io/etcd/api/v3/version"
 	jaegerpropagator "go.opentelemetry.io/contrib/propagators/jaeger"
 	"go.opentelemetry.io/otel"
@@ -23,6 +21,9 @@ import (
 	tracesdk "go.opentelemetry.io/otel/sdk/trace"
 	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
 	trace "go.opentelemetry.io/otel/trace"
+
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 const (
@@ -34,21 +35,6 @@ const (
 	w3cPropagator    string = "w3c"
 )
 
-type Tracer interface {
-	Run(context.Context) error
-	Start(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, Span)
-	Inject(context.Context, http.Header, Span)
-}
-
-type Span interface {
-	End()
-	SetAttributes(key string, value interface{}, kv attribute.KeyValue)
-	SetName(name string)
-	SetStatus(code codes.Code, description string)
-	RecordError(err error, options ...trace.EventOption)
-	AddEvents(keys []string, values []EventValue)
-}
-
 type Opentelemetry struct {
 	enabled       string
 	address       string
diff --git a/pkg/infra/tracing/tracing.go b/pkg/infra/tracing/tracing.go
index e125f0e501b..82c53eff6a1 100644
--- a/pkg/infra/tracing/tracing.go
+++ b/pkg/infra/tracing/tracing.go
@@ -8,9 +8,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/setting"
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/opentracing/opentracing-go/ext"
 	ol "github.com/opentracing/opentracing-go/log"
@@ -20,6 +17,10 @@ import (
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	trace "go.opentelemetry.io/otel/trace"
+
+	"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 const (
@@ -27,6 +28,56 @@ const (
 	envJaegerAgentPort = "JAEGER_AGENT_PORT"
 )
 
+// Tracer defines the service used to create new spans.
+type Tracer interface {
+	// Run implements registry.BackgroundService.
+	Run(context.Context) error
+	// Start creates a new [Span] and places trace metadata on the
+	// [context.Context] passed to the method.
+	// Chose a low cardinality spanName and use [Span.SetAttributes]
+	// or [Span.AddEvents] for high cardinality data.
+	Start(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, Span)
+	// Inject adds identifying information for the span to the
+	// headers defined in [http.Header] map (this mutates http.Header).
+	//
+	// Implementation quirk: Where OpenTelemetry is used, the [Span] is
+	// picked up from [context.Context] and for OpenTracing the
+	// information passed as [Span] is preferred.
+	// Both the context and span must be derived from the same call to
+	// [Tracer.Start].
+	Inject(context.Context, http.Header, Span)
+}
+
+// Span defines a time range for an operation. This is equivalent to a
+// single line in a flame graph.
+type Span interface {
+	// End finalizes the Span and adds its end timestamp.
+	// Any further operations on the Span are not permitted after
+	// End has been called.
+	End()
+	// SetAttributes adds additional data to a span.
+	// SetAttributes repeats the key value pair with [string] and [any]
+	// used for OpenTracing and [attribute.KeyValue] used for
+	// OpenTelemetry.
+	SetAttributes(key string, value interface{}, kv attribute.KeyValue)
+	// SetName renames the span.
+	SetName(name string)
+	// SetStatus can be used to indicate whether the span was
+	// successfully or unsuccessfully executed.
+	//
+	// Only useful for OpenTelemetry.
+	SetStatus(code codes.Code, description string)
+	// RecordError adds an error to the span.
+	//
+	// Only useful for OpenTelemetry.
+	RecordError(err error, options ...trace.EventOption)
+	// AddEvents adds additional data with a temporal dimension to the
+	// span.
+	//
+	// Panics if the length of keys is shorter than the length of values.
+	AddEvents(keys []string, values []EventValue)
+}
+
 func ProvideService(cfg *setting.Cfg) (Tracer, error) {
 	ts, ots, err := parseSettings(cfg)
 	if err != nil {
@@ -239,7 +290,9 @@ func (s OpentracingSpan) SetName(name string) {
 }
 
 func (s OpentracingSpan) SetStatus(code codes.Code, description string) {
-	ext.Error.Set(s.span, true)
+	if code == codes.Error {
+		ext.Error.Set(s.span, true)
+	}
 }
 
 func (s OpentracingSpan) RecordError(err error, options ...trace.EventOption) {