Browse Source

Add metadata to nested tracing spans

DarthSim 1 month ago
parent
commit
8070da962d
5 changed files with 126 additions and 59 deletions
  1. 23 2
      metrics/datadog/datadog.go
  2. 27 31
      metrics/metrics.go
  3. 35 14
      metrics/newrelic/newrelic.go
  4. 23 5
      metrics/otel/otel.go
  5. 18 7
      processing_handler.go

+ 23 - 2
metrics/datadog/datadog.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"reflect"
 	"sync"
 	"time"
 
@@ -123,23 +124,43 @@ func StartRootSpan(ctx context.Context, rw http.ResponseWriter, r *http.Request)
 	return context.WithValue(ctx, spanCtxKey{}, span), cancel, newRw
 }
 
+func setMetadata(span tracer.Span, key string, value any) {
+	if len(key) == 0 || value == nil {
+		return
+	}
+
+	if rv := reflect.ValueOf(value); rv.Kind() == reflect.Map && rv.Type().Key().Kind() == reflect.String {
+		for _, k := range rv.MapKeys() {
+			setMetadata(span, key+"."+k.String(), rv.MapIndex(k).Interface())
+		}
+		return
+	}
+
+	span.SetTag(key, value)
+}
+
 func SetMetadata(ctx context.Context, key string, value any) {
 	if !enabled {
 		return
 	}
 
 	if rootSpan, ok := ctx.Value(spanCtxKey{}).(tracer.Span); ok {
-		rootSpan.SetTag(key, value)
+		setMetadata(rootSpan, key, value)
 	}
 }
 
-func StartSpan(ctx context.Context, name string) context.CancelFunc {
+func StartSpan(ctx context.Context, name string, meta map[string]any) context.CancelFunc {
 	if !enabled {
 		return func() {}
 	}
 
 	if rootSpan, ok := ctx.Value(spanCtxKey{}).(tracer.Span); ok {
 		span := tracer.StartSpan(name, tracer.Measured(), tracer.ChildOf(rootSpan.Context()))
+
+		for k, v := range meta {
+			setMetadata(span, k, v)
+		}
+
 		return func() { span.Finish() }
 	}
 

+ 27 - 31
metrics/metrics.go

@@ -2,7 +2,6 @@ package metrics
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
 	"github.com/imgproxy/imgproxy/v3/metrics/cloudwatch"
@@ -10,9 +9,16 @@ import (
 	"github.com/imgproxy/imgproxy/v3/metrics/newrelic"
 	"github.com/imgproxy/imgproxy/v3/metrics/otel"
 	"github.com/imgproxy/imgproxy/v3/metrics/prometheus"
-	"github.com/imgproxy/imgproxy/v3/structdiff"
 )
 
+const (
+	MetaSourceImageURL    = "imgproxy.source_image_url"
+	MetaSourceImageOrigin = "imgproxy.source_image_origin"
+	MetaProcessingOptions = "imgproxy.processing_options"
+)
+
+type Meta map[string]any
+
 func Init() error {
 	prometheus.Init()
 
@@ -64,29 +70,19 @@ func StartRequest(ctx context.Context, rw http.ResponseWriter, r *http.Request)
 	return ctx, cancel, rw
 }
 
-func setMetadata(ctx context.Context, key string, value any) {
-	newrelic.SetMetadata(ctx, key, value)
-	datadog.SetMetadata(ctx, key, value)
-	otel.SetMetadata(ctx, key, value)
-}
-
-func SetMetadata(ctx context.Context, key string, value any) {
-	if diff, ok := value.(structdiff.Diffable); ok {
-		m := diff.Diff().Flatten()
-		for k, v := range m {
-			setMetadata(ctx, fmt.Sprintf("%s.%s", key, k), v)
-		}
-		return
+func SetMetadata(ctx context.Context, meta Meta) {
+	for key, value := range meta {
+		newrelic.SetMetadata(ctx, key, value)
+		datadog.SetMetadata(ctx, key, value)
+		otel.SetMetadata(ctx, key, value)
 	}
-
-	setMetadata(ctx, key, value)
 }
 
 func StartQueueSegment(ctx context.Context) context.CancelFunc {
 	promCancel := prometheus.StartQueueSegment()
-	nrCancel := newrelic.StartSegment(ctx, "Queue")
-	ddCancel := datadog.StartSpan(ctx, "queue")
-	otelCancel := otel.StartSpan(ctx, "queue")
+	nrCancel := newrelic.StartSegment(ctx, "Queue", nil)
+	ddCancel := datadog.StartSpan(ctx, "queue", nil)
+	otelCancel := otel.StartSpan(ctx, "queue", nil)
 
 	cancel := func() {
 		promCancel()
@@ -98,11 +94,11 @@ func StartQueueSegment(ctx context.Context) context.CancelFunc {
 	return cancel
 }
 
-func StartDownloadingSegment(ctx context.Context) context.CancelFunc {
+func StartDownloadingSegment(ctx context.Context, meta Meta) context.CancelFunc {
 	promCancel := prometheus.StartDownloadingSegment()
-	nrCancel := newrelic.StartSegment(ctx, "Downloading image")
-	ddCancel := datadog.StartSpan(ctx, "downloading_image")
-	otelCancel := otel.StartSpan(ctx, "downloading_image")
+	nrCancel := newrelic.StartSegment(ctx, "Downloading image", meta)
+	ddCancel := datadog.StartSpan(ctx, "downloading_image", meta)
+	otelCancel := otel.StartSpan(ctx, "downloading_image", meta)
 
 	cancel := func() {
 		promCancel()
@@ -114,11 +110,11 @@ func StartDownloadingSegment(ctx context.Context) context.CancelFunc {
 	return cancel
 }
 
-func StartProcessingSegment(ctx context.Context) context.CancelFunc {
+func StartProcessingSegment(ctx context.Context, meta Meta) context.CancelFunc {
 	promCancel := prometheus.StartProcessingSegment()
-	nrCancel := newrelic.StartSegment(ctx, "Processing image")
-	ddCancel := datadog.StartSpan(ctx, "processing_image")
-	otelCancel := otel.StartSpan(ctx, "processing_image")
+	nrCancel := newrelic.StartSegment(ctx, "Processing image", meta)
+	ddCancel := datadog.StartSpan(ctx, "processing_image", meta)
+	otelCancel := otel.StartSpan(ctx, "processing_image", meta)
 
 	cancel := func() {
 		promCancel()
@@ -132,9 +128,9 @@ func StartProcessingSegment(ctx context.Context) context.CancelFunc {
 
 func StartStreamingSegment(ctx context.Context) context.CancelFunc {
 	promCancel := prometheus.StartStreamingSegment()
-	nrCancel := newrelic.StartSegment(ctx, "Streaming image")
-	ddCancel := datadog.StartSpan(ctx, "streaming_image")
-	otelCancel := otel.StartSpan(ctx, "streaming_image")
+	nrCancel := newrelic.StartSegment(ctx, "Streaming image", nil)
+	ddCancel := datadog.StartSpan(ctx, "streaming_image", nil)
+	otelCancel := otel.StartSpan(ctx, "streaming_image", nil)
 
 	cancel := func() {
 		promCancel()

+ 35 - 14
metrics/newrelic/newrelic.go

@@ -23,6 +23,10 @@ type transactionCtxKey struct{}
 
 type GaugeFunc func() float64
 
+type attributable interface {
+	AddAttribute(key string, value interface{})
+}
+
 const (
 	defaultMetricURL = "https://metric-api.newrelic.com/metric/v1"
 	euMetricURL      = "https://metric-api.eu.newrelic.com/metric/v1"
@@ -132,35 +136,52 @@ func StartTransaction(ctx context.Context, rw http.ResponseWriter, r *http.Reque
 	return context.WithValue(ctx, transactionCtxKey{}, txn), cancel, newRw
 }
 
+func setMetadata(span attributable, key string, value interface{}) {
+	if len(key) == 0 || value == nil {
+		return
+	}
+
+	rv := reflect.ValueOf(value)
+	switch {
+	case rv.Kind() == reflect.String || rv.Kind() == reflect.Bool:
+		span.AddAttribute(key, value)
+	case rv.CanInt():
+		span.AddAttribute(key, rv.Int())
+	case rv.CanUint():
+		span.AddAttribute(key, rv.Uint())
+	case rv.CanFloat():
+		span.AddAttribute(key, rv.Float())
+	case rv.Kind() == reflect.Map && rv.Type().Key().Kind() == reflect.String:
+		for _, k := range rv.MapKeys() {
+			setMetadata(span, key+"."+k.String(), rv.MapIndex(k).Interface())
+		}
+	default:
+		span.AddAttribute(key, fmt.Sprintf("%v", value))
+	}
+}
+
 func SetMetadata(ctx context.Context, key string, value interface{}) {
 	if !enabled {
 		return
 	}
 
 	if txn, ok := ctx.Value(transactionCtxKey{}).(*newrelic.Transaction); ok {
-		rv := reflect.ValueOf(value)
-		switch {
-		case rv.Kind() == reflect.String || rv.Kind() == reflect.Bool:
-			txn.AddAttribute(key, value)
-		case rv.CanInt():
-			txn.AddAttribute(key, rv.Int())
-		case rv.CanUint():
-			txn.AddAttribute(key, rv.Uint())
-		case rv.CanFloat():
-			txn.AddAttribute(key, rv.Float())
-		default:
-			txn.AddAttribute(key, fmt.Sprintf("%v", value))
-		}
+		setMetadata(txn, key, value)
 	}
 }
 
-func StartSegment(ctx context.Context, name string) context.CancelFunc {
+func StartSegment(ctx context.Context, name string, meta map[string]any) context.CancelFunc {
 	if !enabled {
 		return func() {}
 	}
 
 	if txn, ok := ctx.Value(transactionCtxKey{}).(*newrelic.Transaction); ok {
 		segment := txn.StartSegment(name)
+
+		for k, v := range meta {
+			setMetadata(segment, k, v)
+		}
+
 		return func() { segment.End() }
 	}
 

+ 23 - 5
metrics/otel/otel.go

@@ -426,13 +426,11 @@ func StartRootSpan(ctx context.Context, rw http.ResponseWriter, r *http.Request)
 	return ctx, cancel, newRw
 }
 
-func SetMetadata(ctx context.Context, key string, value interface{}) {
-	if !enabled {
+func setMetadata(span trace.Span, key string, value interface{}) {
+	if len(key) == 0 || value == nil {
 		return
 	}
 
-	span := trace.SpanFromContext(ctx)
-
 	rv := reflect.ValueOf(value)
 
 	switch {
@@ -446,6 +444,10 @@ func SetMetadata(ctx context.Context, key string, value interface{}) {
 		span.SetAttributes(attribute.Int64(key, int64(rv.Uint())))
 	case rv.CanFloat():
 		span.SetAttributes(attribute.Float64(key, rv.Float()))
+	case rv.Kind() == reflect.Map && rv.Type().Key().Kind() == reflect.String:
+		for _, k := range rv.MapKeys() {
+			setMetadata(span, key+"."+k.String(), rv.MapIndex(k).Interface())
+		}
 	default:
 		// Theoretically, we can also cover slices and arrays here,
 		// but it's pretty complex and not really needed for now
@@ -453,7 +455,19 @@ func SetMetadata(ctx context.Context, key string, value interface{}) {
 	}
 }
 
-func StartSpan(ctx context.Context, name string) context.CancelFunc {
+func SetMetadata(ctx context.Context, key string, value interface{}) {
+	if !enabled {
+		return
+	}
+
+	if ctx.Value(hasSpanCtxKey{}) != nil {
+		if span := trace.SpanFromContext(ctx); span != nil {
+			setMetadata(span, key, value)
+		}
+	}
+}
+
+func StartSpan(ctx context.Context, name string, meta map[string]any) context.CancelFunc {
 	if !enabled {
 		return func() {}
 	}
@@ -461,6 +475,10 @@ func StartSpan(ctx context.Context, name string) context.CancelFunc {
 	if ctx.Value(hasSpanCtxKey{}) != nil {
 		_, span := tracer.Start(ctx, name, trace.WithSpanKind(trace.SpanKindInternal))
 
+		for k, v := range meta {
+			setMetadata(span, k, v)
+		}
+
 		return func() { span.End() }
 	}
 

+ 18 - 7
processing_handler.go

@@ -246,16 +246,22 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	po, imageURL, err := options.ParsePath(path, r.Header)
 	checkErr(ctx, "path_parsing", err)
 
+	var imageOrigin any
+	if u, uerr := url.Parse(imageURL); uerr == nil {
+		imageOrigin = u.Scheme + "://" + u.Host
+	}
+
 	errorreport.SetMetadata(r, "Source Image URL", imageURL)
 	errorreport.SetMetadata(r, "Processing Options", po)
 
-	metrics.SetMetadata(ctx, "imgproxy.source_image_url", imageURL)
-	metrics.SetMetadata(ctx, "imgproxy.processing_options", po)
-
-	if u, ue := url.Parse(imageURL); ue == nil {
-		metrics.SetMetadata(ctx, "imgproxy.source_image_origin", u.Scheme+"://"+u.Host)
+	metricsMeta := metrics.Meta{
+		metrics.MetaSourceImageURL:    imageURL,
+		metrics.MetaSourceImageOrigin: imageOrigin,
+		metrics.MetaProcessingOptions: po.Diff().Flatten(),
 	}
 
+	metrics.SetMetadata(ctx, metricsMeta)
+
 	err = security.VerifySourceURL(imageURL)
 	checkErr(ctx, "security", err)
 
@@ -323,7 +329,10 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	statusCode := http.StatusOK
 
 	originData, err := func() (*imagedata.ImageData, error) {
-		defer metrics.StartDownloadingSegment(ctx)()
+		defer metrics.StartDownloadingSegment(ctx, metrics.Meta{
+			metrics.MetaSourceImageURL:    metricsMeta[metrics.MetaSourceImageURL],
+			metrics.MetaSourceImageOrigin: metricsMeta[metrics.MetaSourceImageOrigin],
+		})()
 
 		downloadOpts := imagedata.DownloadOptions{
 			Header:    imgRequestHeader,
@@ -452,7 +461,9 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 	}
 
 	resultData, err := func() (*imagedata.ImageData, error) {
-		defer metrics.StartProcessingSegment(ctx)()
+		defer metrics.StartProcessingSegment(ctx, metrics.Meta{
+			metrics.MetaProcessingOptions: metricsMeta[metrics.MetaProcessingOptions],
+		})()
 		return processing.ProcessImage(ctx, originData, po)
 	}()
 	checkErr(ctx, "processing", err)