diff --git a/config/README.md b/config/README.md index a6dee49..dfb72da 100755 --- a/config/README.md +++ b/config/README.md @@ -65,7 +65,7 @@ import "github.com/go-coldbrew/core/config" -## type [Config]() +## type [Config]() Config is the configuration for the Coldbrew server It is populated from environment variables and has sensible defaults for all fields so that you can just use it as is without any configuration The following environment variables are supported and can be used to override the defaults for the fields @@ -195,14 +195,15 @@ type Config struct { // OTLPSamplingRatio is the ratio of traces to sample (0.0 to 1.0) // 1.0 means sample all traces, 0.1 means sample 10% of traces OTLPSamplingRatio float64 `envconfig:"OTLP_SAMPLING_RATIO" default:"0.2"` - // OTLPUseOpenTracingBridge determines whether to set up OpenTracing compatibility bridge - // This allows using existing OpenTracing instrumentation with OpenTelemetry - OTLPUseOpenTracingBridge bool `envconfig:"OTLP_USE_OPENTRACING_BRIDGE" default:"true"` + // Deprecated: OpenTracing bridge is provided for backwards compatibility only. + // New services should leave this false (the default). Set to true only if you + // have existing OpenTracing instrumentation that hasn't been migrated to OTEL. + OTLPUseOpenTracingBridge bool `envconfig:"OTLP_USE_OPENTRACING_BRIDGE" default:"false"` } ``` -### func \(Config\) [Validate]() +### func \(Config\) [Validate]() ```go func (c Config) Validate() []string diff --git a/core.go b/core.go index cd53e30..e80ddee 100644 --- a/core.go +++ b/core.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/pprof" + "strconv" "strings" "sync" "time" @@ -24,8 +25,10 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.12.0" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" oteltrace "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -169,37 +172,113 @@ func (c *cb) processConfig() { } } +// statusRecorder wraps http.ResponseWriter to capture the final HTTP status code. +// It records the first status >= 200, plus 101 Switching Protocols (which is +// terminal). Other 1xx statuses are informational and skipped. +// Unwrap() is provided for http.ResponseController (Go 1.20+) to access optional +// interfaces (http.Flusher, http.Hijacker, etc.) from the underlying writer. +type statusRecorder struct { + http.ResponseWriter + status int + wroteHeader bool +} + +func (sr *statusRecorder) WriteHeader(code int) { + if !sr.wroteHeader && (code >= 200 || code == http.StatusSwitchingProtocols) { + sr.status = code + sr.wroteHeader = true + } + sr.ResponseWriter.WriteHeader(code) +} + +func (sr *statusRecorder) Write(b []byte) (int, error) { + if !sr.wroteHeader { + sr.status = http.StatusOK + sr.wroteHeader = true + } + return sr.ResponseWriter.Write(b) +} + +// Unwrap returns the underlying ResponseWriter so that http.ResponseController +// and middleware can access optional interfaces (http.Flusher, http.Hijacker, etc.). +func (sr *statusRecorder) Unwrap() http.ResponseWriter { + return sr.ResponseWriter +} + +// endSpan records the HTTP status code on the span, marks it as error for 5xx, and ends it. +func endSpan(span oteltrace.Span, rec *statusRecorder) { + span.SetAttributes(semconv.HTTPResponseStatusCode(rec.status)) + if rec.status >= 500 { + span.SetStatus(codes.Error, http.StatusText(rec.status)) + } + span.End() +} + +// httpSpanAttributes returns the OTEL attributes for an incoming HTTP request, +// omitting empty-valued attributes (e.g. scheme behind a reverse proxy). +func httpSpanAttributes(r *http.Request) []attribute.KeyValue { + host, port, err := net.SplitHostPort(r.Host) + if err != nil { + host = r.Host + } + host = strings.TrimPrefix(strings.TrimSuffix(host, "]"), "[") + attrs := []attribute.KeyValue{ + semconv.HTTPRequestMethodKey.String(r.Method), + semconv.URLPath(r.URL.Path), + semconv.ServerAddress(host), + } + if port != "" { + if p, err := strconv.Atoi(port); err == nil { + attrs = append(attrs, semconv.ServerPort(p)) + } + } + if r.URL.RawQuery != "" { + attrs = append(attrs, semconv.URLQuery(r.URL.RawQuery)) + } + if r.URL.Scheme != "" { + attrs = append(attrs, semconv.URLScheme(r.URL.Scheme)) + } + return attrs +} + // tracingWrapper is a middleware that creates a new OTEL span for each incoming HTTP request. // It extracts any propagated trace context from the request headers and, for non-filtered // methods, starts a server span that is attached to the request context. func tracingWrapper(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - prop := otel.GetTextMapPropagator() - ctx := prop.Extract(r.Context(), propagation.HeaderCarrier(r.Header)) + ctx := otel.GetTextMapPropagator().Extract(r.Context(), propagation.HeaderCarrier(r.Header)) + if interceptors.FilterMethodsFunc(ctx, r.URL.Path) { var serverSpan oteltrace.Span - ctx, serverSpan = otel.Tracer("coldbrew-http").Start(ctx, "ServeHTTP", + ctx, serverSpan = otel.Tracer("coldbrew-http").Start(ctx, r.Method, oteltrace.WithSpanKind(oteltrace.SpanKindServer), - oteltrace.WithAttributes( - semconv.HTTPMethodKey.String(r.Method), - semconv.HTTPTargetKey.String(r.URL.RequestURI()), - ), + oteltrace.WithAttributes(httpSpanAttributes(r)...), ) - r = r.WithContext(ctx) - defer serverSpan.End() - } else { - r = r.WithContext(ctx) + rec := &statusRecorder{ResponseWriter: w, status: http.StatusOK} + w = rec + defer endSpan(serverSpan, rec) } + _, han := interceptors.NRHttpTracer("", h.ServeHTTP) - // add this info to log - ctx = r.Context() ctx = options.AddToOptions(ctx, "", "") ctx = loggers.AddToLogContext(ctx, "httpPath", r.URL.Path) - r = r.WithContext(ctx) - han(w, r) + han(w, r.WithContext(ctx)) }) } +// spanRouteMiddleware is a grpc-gateway middleware that updates the OTEL span +// name and http.route attribute with the matched route pattern after routing. +func spanRouteMiddleware(next runtime.HandlerFunc) runtime.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) { + if pattern, ok := runtime.HTTPPathPattern(r.Context()); ok { + span := oteltrace.SpanFromContext(r.Context()) + span.SetName(r.Method + " " + pattern) + span.SetAttributes(semconv.HTTPRoute(pattern)) + } + next(w, r, pathParams) + } +} + // getCustomHeaderMatcher returns a matcher that matches the given header and prefix func getCustomHeaderMatcher(prefixes []string, header string) func(string) (string, bool) { header = strings.ToLower(header) @@ -239,6 +318,7 @@ func (c *cb) initHTTP(ctx context.Context) (*http.Server, error) { ), runtime.WithMarshalerOption("application/proto", pMar), runtime.WithMarshalerOption("application/protobuf", pMar), + runtime.WithMiddlewares(spanRouteMiddleware), } if c.config.UseJSONBuiltinMarshaller { diff --git a/core_coverage_test.go b/core_coverage_test.go index 5e30b85..9103d67 100644 --- a/core_coverage_test.go +++ b/core_coverage_test.go @@ -170,6 +170,122 @@ func TestTracingWrapper(t *testing.T) { } } +func TestTracingWrapper_StatusCodes(t *testing.T) { + t.Parallel() + tests := []struct { + name string + handler http.HandlerFunc + wantStatus int + }{ + { + name: "records 200", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }, + wantStatus: http.StatusOK, + }, + { + name: "records 404", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }, + wantStatus: http.StatusNotFound, + }, + { + name: "records 500", + handler: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }, + wantStatus: http.StatusInternalServerError, + }, + { + name: "defaults to 200 when WriteHeader not called", + handler: func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, "ok") + }, + wantStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + wrapped := tracingWrapper(tt.handler) + req := httptest.NewRequest("GET", "/api/test", nil) + w := httptest.NewRecorder() + wrapped.ServeHTTP(w, req) + if w.Code != tt.wantStatus { + t.Fatalf("expected %d, got %d", tt.wantStatus, w.Code) + } + }) + } +} + +func TestStatusRecorder(t *testing.T) { + t.Parallel() + + t.Run("captures explicit status", func(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + rec := &statusRecorder{ResponseWriter: w, status: http.StatusOK} + rec.WriteHeader(http.StatusBadGateway) + if rec.status != http.StatusBadGateway { + t.Fatalf("expected %d, got %d", http.StatusBadGateway, rec.status) + } + if w.Code != http.StatusBadGateway { + t.Fatalf("underlying writer expected %d, got %d", http.StatusBadGateway, w.Code) + } + }) + + t.Run("unwrap returns underlying writer", func(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + rec := &statusRecorder{ResponseWriter: w} + if rec.Unwrap() != w { + t.Fatal("Unwrap did not return underlying ResponseWriter") + } + }) +} + +func TestSpanRouteMiddleware(t *testing.T) { + t.Parallel() + + t.Run("calls next handler", func(t *testing.T) { + t.Parallel() + called := false + next := func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) { + called = true + w.WriteHeader(http.StatusOK) + } + wrapped := spanRouteMiddleware(next) + req := httptest.NewRequest("GET", "/api/v1/rules", nil) + w := httptest.NewRecorder() + wrapped(w, req, nil) + if !called { + t.Fatal("expected next handler to be called") + } + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", w.Code) + } + }) + + t.Run("passes path params through", func(t *testing.T) { + t.Parallel() + var gotParams map[string]string + next := func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) { + gotParams = pathParams + } + wrapped := spanRouteMiddleware(next) + params := map[string]string{"id": "123"} + req := httptest.NewRequest("GET", "/api/v1/rules/123", nil) + w := httptest.NewRecorder() + wrapped(w, req, params) + if gotParams["id"] != "123" { + t.Fatalf("expected path param id=123, got %v", gotParams) + } + }) +} + func TestGetCustomHeaderMatcher_EmptyPrefixes(t *testing.T) { t.Parallel() matcher := getCustomHeaderMatcher(nil, "X-Trace-Id") diff --git a/initializers.go b/initializers.go index 9f365cc..f611dee 100644 --- a/initializers.go +++ b/initializers.go @@ -5,6 +5,8 @@ import ( "fmt" "os" "os/signal" + "path/filepath" + "runtime/debug" "strings" "sync" "syscall" @@ -23,13 +25,14 @@ import ( "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" otelBridge "go.opentelemetry.io/otel/bridge/opentracing" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" "go.opentelemetry.io/otel/sdk/resource" sdktrace "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.12.0" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" "go.uber.org/automaxprocs/maxprocs" "google.golang.org/grpc/encoding" _ "google.golang.org/grpc/encoding/gzip" @@ -210,12 +213,28 @@ func SetupOpenTelemetry(config OTLPConfig) error { } d := resource.Default() + attrs := []attribute.KeyValue{ + semconv.ServiceName(config.ServiceName), + semconv.ServiceVersion(config.ServiceVersion), + } + if bi, ok := debug.ReadBuildInfo(); ok { + attrs = append(attrs, + semconv.ProcessExecutableName(filepath.Base(os.Args[0])), + semconv.ProcessRuntimeVersion(bi.GoVersion), + ) + for _, s := range bi.Settings { + switch s.Key { + case "vcs.revision": + attrs = append(attrs, semconv.VCSRefHeadRevision(s.Value)) + case "vcs.time": + attrs = append(attrs, attribute.String("vcs.time", s.Value)) + case "vcs.modified": + attrs = append(attrs, attribute.Bool("vcs.modified", s.Value == "true")) + } + } + } res, err := resource.New(context.Background(), - resource.WithAttributes( - // the service name used to display traces in backends - semconv.ServiceNameKey.String(config.ServiceName), - semconv.ServiceVersionKey.String(config.ServiceVersion), - ), + resource.WithAttributes(attrs...), ) if err != nil { log.Error(context.Background(), "msg", "creating OTLP resource", "err", err)