From ba17ed8f1957adc3a3bac1dda9a5f9041983aa11 Mon Sep 17 00:00:00 2001 From: DarthSim Date: Fri, 23 Aug 2024 18:37:24 +0300 Subject: [PATCH 1/2] Add `imgproxy.source_image_url` and `imgproxy.processing_options` attributes to New Relic, DataDog, and OpenTelemetry traces --- CHANGELOG.md | 1 + metrics/datadog/datadog.go | 10 ++++++++++ metrics/metrics.go | 24 ++++++++++++++++++++++++ metrics/newrelic/newrelic.go | 23 +++++++++++++++++++++++ metrics/otel/otel.go | 28 ++++++++++++++++++++++++++++ processing_handler.go | 3 +++ structdiff/diff.go | 21 +++++++++++++++++++++ 7 files changed, 110 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efda217c52..ddc893fd60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] # Add +- Add `imgproxy.source_image_url` and `imgproxy.processing_options` attributes to New Relic, DataDog, and OpenTelemetry traces. - (pro) Add [monochrome](https://docs.imgproxy.net/latest/usage/processing#monochrome) processing option. - (pro) Add [duotone](https://docs.imgproxy.net/latest/usage/processing#duotone) processing option. diff --git a/metrics/datadog/datadog.go b/metrics/datadog/datadog.go index b3dcc1785e..ff2f9419ef 100644 --- a/metrics/datadog/datadog.go +++ b/metrics/datadog/datadog.go @@ -123,6 +123,16 @@ func StartRootSpan(ctx context.Context, rw http.ResponseWriter, r *http.Request) return context.WithValue(ctx, spanCtxKey{}, span), cancel, newRw } +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) + } +} + func StartSpan(ctx context.Context, name string) context.CancelFunc { if !enabled { return func() {} diff --git a/metrics/metrics.go b/metrics/metrics.go index 18b341ebf7..50db8e2217 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -2,6 +2,7 @@ package metrics import ( "context" + "fmt" "net/http" "github.com/imgproxy/imgproxy/v3/metrics/cloudwatch" @@ -9,6 +10,7 @@ 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" ) func Init() error { @@ -62,6 +64,28 @@ 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) { + type diffable interface { + Diff() structdiff.Entries + } + + if diff, ok := value.(diffable); ok { + m := diff.Diff().Flatten() + for k, v := range m { + setMetadata(ctx, fmt.Sprintf("%s.%s", key, k), v) + } + return + } + + setMetadata(ctx, key, value) +} + func StartQueueSegment(ctx context.Context) context.CancelFunc { promCancel := prometheus.StartQueueSegment() nrCancel := newrelic.StartSegment(ctx, "Queue") diff --git a/metrics/newrelic/newrelic.go b/metrics/newrelic/newrelic.go index f17cc72c5d..cdf0768e68 100644 --- a/metrics/newrelic/newrelic.go +++ b/metrics/newrelic/newrelic.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "net/http" + "reflect" "regexp" "sync" "time" @@ -131,6 +132,28 @@ func StartTransaction(ctx context.Context, rw http.ResponseWriter, r *http.Reque return context.WithValue(ctx, transactionCtxKey{}, txn), cancel, newRw } +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)) + } + } +} + func StartSegment(ctx context.Context, name string) context.CancelFunc { if !enabled { return func() {} diff --git a/metrics/otel/otel.go b/metrics/otel/otel.go index af6a791159..d62fbdc662 100644 --- a/metrics/otel/otel.go +++ b/metrics/otel/otel.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "os" + "reflect" "runtime" "strconv" "strings" @@ -418,6 +419,33 @@ 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 { + return + } + + span := trace.SpanFromContext(ctx) + + rv := reflect.ValueOf(value) + + switch { + case rv.Kind() == reflect.String: + span.SetAttributes(attribute.String(key, value.(string))) + case rv.Kind() == reflect.Bool: + span.SetAttributes(attribute.Bool(key, value.(bool))) + case rv.CanInt(): + span.SetAttributes(attribute.Int64(key, rv.Int())) + case rv.CanUint(): + span.SetAttributes(attribute.Int64(key, int64(rv.Uint()))) + case rv.CanFloat(): + span.SetAttributes(attribute.Float64(key, rv.Float())) + default: + // Theoretically, we can also cover slices and arrays here, + // but it's pretty complex and not really needed for now + span.SetAttributes(attribute.String(key, fmt.Sprintf("%v", value))) + } +} + func StartSpan(ctx context.Context, name string) context.CancelFunc { if !enabled { return func() {} diff --git a/processing_handler.go b/processing_handler.go index ef46a2373f..7522a1ebd2 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -248,6 +248,9 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { 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) + err = security.VerifySourceURL(imageURL) checkErr(ctx, "security", err) diff --git a/structdiff/diff.go b/structdiff/diff.go index dcf980c651..6751da54aa 100644 --- a/structdiff/diff.go +++ b/structdiff/diff.go @@ -80,6 +80,27 @@ func (d Entries) MarshalJSON() ([]byte, error) { return buf.Bytes(), nil } +func (d Entries) flatten(m map[string]interface{}, prefix string) { + for _, e := range d { + key := e.Name + if len(prefix) > 0 { + key = prefix + "." + key + } + + if dd, ok := e.Value.(Entries); ok { + dd.flatten(m, key) + } else { + m[key] = e.Value + } + } +} + +func (d Entries) Flatten() map[string]interface{} { + m := make(map[string]interface{}) + d.flatten(m, "") + return m +} + func Diff(a, b interface{}) Entries { valA := reflect.Indirect(reflect.ValueOf(a)) valB := reflect.Indirect(reflect.ValueOf(b)) From dc6acc78c32f8ef3ac38b364ea8f283418b5c01e Mon Sep 17 00:00:00 2001 From: DarthSim Date: Fri, 23 Aug 2024 18:47:31 +0300 Subject: [PATCH 2/2] Properly set the `net.host.name` and `http.url` tags in OpenTelemetry traces --- CHANGELOG.md | 3 +++ metrics/otel/otel.go | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddc893fd60..ac6db64ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ - (pro) Add [monochrome](https://docs.imgproxy.net/latest/usage/processing#monochrome) processing option. - (pro) Add [duotone](https://docs.imgproxy.net/latest/usage/processing#duotone) processing option. +# Change +- Properly set the `net.host.name` and `http.url` tags in OpenTelemetry traces. + # Fix - Fix handling `#` symbols in `local://`, `s3://`, `gcs://`, `abs://`, and `swift://` URLs. - Fix `IMGPROXY_FALLBACK_IMAGE_HTTP_CODE` value check. Allow `0` value. diff --git a/metrics/otel/otel.go b/metrics/otel/otel.go index d62fbdc662..854ca6e747 100644 --- a/metrics/otel/otel.go +++ b/metrics/otel/otel.go @@ -35,8 +35,8 @@ import ( sdkmetric "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.17.0" - "go.opentelemetry.io/otel/semconv/v1.17.0/httpconv" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" + "go.opentelemetry.io/otel/semconv/v1.20.0/httpconv" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc/credentials" @@ -397,10 +397,16 @@ func StartRootSpan(ctx context.Context, rw http.ResponseWriter, r *http.Request) ctx = propagator.Extract(ctx, propagation.HeaderCarrier(r.Header)) } + server := r.Host + if len(server) == 0 { + server = "imgproxy" + } + ctx, span := tracer.Start( ctx, "/request", trace.WithSpanKind(trace.SpanKindServer), - trace.WithAttributes(httpconv.ServerRequest("imgproxy", r)...), + trace.WithAttributes(httpconv.ServerRequest(server, r)...), + trace.WithAttributes(semconv.HTTPURL(r.RequestURI)), ) ctx = context.WithValue(ctx, hasSpanCtxKey{}, struct{}{})