From 2c9466a1f27ce1f4be17bc99afd08791f4b77c9e Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 23 Dec 2025 22:53:48 +0530 Subject: [PATCH 01/29] add http middleware --- telemetry/instrumentation/http/config.go | 39 +++++ telemetry/instrumentation/http/middleware.go | 148 +++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 telemetry/instrumentation/http/config.go create mode 100644 telemetry/instrumentation/http/middleware.go diff --git a/telemetry/instrumentation/http/config.go b/telemetry/instrumentation/http/config.go new file mode 100644 index 000000000..9bed20e84 --- /dev/null +++ b/telemetry/instrumentation/http/config.go @@ -0,0 +1,39 @@ +package http + +import ( + "fmt" + + "github.com/goravel/framework/contracts/http" +) + +type Filter func(ctx http.Context) bool + +type SpanNameFormatter func(route string, ctx http.Context) string + +type Option func(*ServerConfig) + +func WithSpanNameFormatter(f SpanNameFormatter) Option { + return func(c *ServerConfig) { + c.SpanNameFormatter = f + } +} + +func WithFilter(f Filter) Option { + return func(c *ServerConfig) { + c.Filters = append(c.Filters, f) + } +} + +// ServerConfig maps to the "telemetry.instrumentation.http_server" key in the config file. +type ServerConfig struct { + Enabled bool `mapstructure:"enabled"` + Name string `mapstructure:"name"` + ExcludedPaths []string `mapstructure:"excluded_paths"` + ExcludedMethods []string `mapstructure:"excluded_methods"` + SpanNameFormatter SpanNameFormatter `mapstructure:"span_name_formatter"` + Filters []Filter `mapstructure:"filters"` +} + +func defaultSpanNameFormatter(route string, ctx http.Context) string { + return fmt.Sprintf("%s %s", ctx.Request().Method(), route) +} diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go new file mode 100644 index 000000000..e89e9bf9a --- /dev/null +++ b/telemetry/instrumentation/http/middleware.go @@ -0,0 +1,148 @@ +package http + +import ( + "fmt" + "time" + + "go.opentelemetry.io/otel/metric" + semconv "go.opentelemetry.io/otel/semconv/v1.37.0" + + "github.com/goravel/framework/contracts/http" + "github.com/goravel/framework/telemetry" +) + +const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/http" + +func Telemetry(opts ...Option) http.Middleware { + var cfg ServerConfig + _ = telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &cfg) + for _, opt := range opts { + opt(&cfg) + } + + if !cfg.Enabled { + return noopMiddleware() + } + + if cfg.SpanNameFormatter == nil { + cfg.SpanNameFormatter = defaultSpanNameFormatter + } + + instrumentationName := cfg.Name + if instrumentationName == "" { + instrumentationName = defaultInstrumentationName + } + + tracer := telemetry.TelemetryFacade.Tracer(instrumentationName) + meter := telemetry.TelemetryFacade.Meter(instrumentationName) + propagator := telemetry.TelemetryFacade.Propagator() + + durationHist, _ := meter.Float64Histogram("http.server.request.duration", metric.WithUnit("s"), metric.WithDescription("Duration of HTTP server requests")) + requestSizeHist, _ := meter.Int64Histogram("http.server.request.body.size", metric.WithUnit("By"), metric.WithDescription("Size of HTTP server request body")) + responseSizeHist, _ := meter.Int64Histogram("http.server.response.body.size", metric.WithUnit("By"), metric.WithDescription("Size of HTTP server response body")) + + excludedPathsMap := sliceToMap(cfg.ExcludedPaths) + excludedMethodsMap := sliceToMap(cfg.ExcludedMethods) + return func(ctx http.Context) { + start := time.Now() + req := ctx.Request() + + routePattern := req.OriginPath() + if routePattern == "" { + routePattern = req.Path() + } + if excludedPathsMap[routePattern] || excludedMethodsMap[req.Method()] { + req.Next() + return + } + + for _, f := range cfg.Filters { + if !f(ctx) { + req.Next() + return + } + } + + parentCtx := propagator.Extract(ctx.Context(), telemetry.PropagationHeaderCarrier(req.Headers())) + + spanName := cfg.SpanNameFormatter(routePattern, ctx) + + scheme := "http" + if proto := req.Header("X-Forwarded-Proto"); proto != "" { + scheme = proto + } + + baseAttrs := []telemetry.KeyValue{ + semconv.HTTPRequestMethodOriginal(req.Method()), + semconv.HTTPRoute(routePattern), + semconv.URLScheme(scheme), + semconv.ServerAddress(req.Host()), + semconv.ClientAddress(req.Ip()), + semconv.UserAgentOriginal(req.Header("User-Agent")), + } + + spanCtx, span := tracer.Start(parentCtx, spanName, + telemetry.WithAttributes(baseAttrs...), + telemetry.WithSpanKind(telemetry.SpanKindServer), + ) + + ctx.WithContext(spanCtx) + + defer func() { + if r := recover(); r != nil { + err := fmt.Errorf("panic: %v", r) + span.RecordError(err) + span.SetStatus(telemetry.CodeError, "Internal Server Error") + span.End() + + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(500))...) + durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + + panic(r) + } else { + span.End() + } + }() + + req.Next() + + status := ctx.Response().Origin().Status() + span.SetAttributes(semconv.HTTPResponseStatusCode(status)) + + if status >= 500 { + span.SetStatus(telemetry.CodeError, "") + } else { + span.SetStatus(telemetry.CodeOk, "") + } + + if err := ctx.Err(); err != nil { + span.RecordError(err) + } + + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) + + reqSize := req.Origin().ContentLength + if reqSize < 0 { + reqSize = 0 + } + requestSizeHist.Record(spanCtx, reqSize, metricAttrs) + + responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) + + durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + } +} + +func noopMiddleware() http.Middleware { + return func(ctx http.Context) { + ctx.Request().Next() + } +} + +func sliceToMap(slice []string) map[string]bool { + m := make(map[string]bool, len(slice)) + for _, s := range slice { + m[s] = true + } + return m +} From 7ccca56b0cf036553b162d583d04ce2786924998 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 30 Dec 2025 01:10:08 +0530 Subject: [PATCH 02/29] add http transport --- go.mod | 2 + go.sum | 4 + telemetry/instrumentation/http/transport.go | 27 +++++ .../instrumentation/http/transport_test.go | 112 ++++++++++++++++++ 4 files changed, 145 insertions(+) create mode 100644 telemetry/instrumentation/http/transport.go create mode 100644 telemetry/instrumentation/http/transport_test.go diff --git a/go.mod b/go.mod index f60833562..5c428ef15 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( github.com/stretchr/testify v1.11.1 github.com/urfave/cli/v3 v3.6.1 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.64.0 + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.64.0 go.opentelemetry.io/contrib/propagators/b3 v1.39.0 go.opentelemetry.io/otel v1.39.0 go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.15.0 @@ -63,6 +64,7 @@ require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/chigopher/pathlib v0.19.1 // indirect github.com/clipperhouse/uax29/v2 v2.2.0 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/google/go-cmp v0.7.0 // indirect diff --git a/go.sum b/go.sum index 9861ba5b0..6e1e0b321 100644 --- a/go.sum +++ b/go.sum @@ -91,6 +91,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= +github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= +github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= @@ -274,6 +276,8 @@ go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.64.0 h1:RN3ifU8y4prNWeEnQp2kRRHz8UwonAEYZl8tUzHEXAk= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.64.0/go.mod h1:habDz3tEWiFANTo6oUE99EmaFUrCNYAAg3wiVmusm70= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.64.0 h1:ssfIgGNANqpVFCndZvcuyKbl0g+UAVcbBcqGkG28H0Y= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.64.0/go.mod h1:GQ/474YrbE4Jx8gZ4q5I4hrhUzM6UPzyrqJYV2AqPoQ= go.opentelemetry.io/contrib/propagators/b3 v1.39.0 h1:PI7pt9pkSnimWcp5sQhUA9OzLbc3Ba4sL+VEUTNsxrk= go.opentelemetry.io/contrib/propagators/b3 v1.39.0/go.mod h1:5gV/EzPnfYIwjzj+6y8tbGW2PKWhcsz5e/7twptRVQY= go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= diff --git a/telemetry/instrumentation/http/transport.go b/telemetry/instrumentation/http/transport.go new file mode 100644 index 000000000..a4d63a5ab --- /dev/null +++ b/telemetry/instrumentation/http/transport.go @@ -0,0 +1,27 @@ +package http + +import ( + "net/http" + + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + + "github.com/goravel/framework/support/color" + "github.com/goravel/framework/telemetry" +) + +func NewTransport(base http.RoundTripper) http.RoundTripper { + if telemetry.TelemetryFacade == nil { + color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") + if base == nil { + return http.DefaultTransport + } + return base + } + + return otelhttp.NewTransport( + base, + otelhttp.WithTracerProvider(telemetry.TelemetryFacade.TracerProvider()), + otelhttp.WithMeterProvider(telemetry.TelemetryFacade.MeterProvider()), + otelhttp.WithPropagators(telemetry.TelemetryFacade.Propagator()), + ) +} diff --git a/telemetry/instrumentation/http/transport_test.go b/telemetry/instrumentation/http/transport_test.go new file mode 100644 index 000000000..ade962496 --- /dev/null +++ b/telemetry/instrumentation/http/transport_test.go @@ -0,0 +1,112 @@ +package http + +import ( + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/suite" + metricnoop "go.opentelemetry.io/otel/metric/noop" + "go.opentelemetry.io/otel/propagation" + tracenoop "go.opentelemetry.io/otel/trace/noop" + + contractstelemetry "github.com/goravel/framework/contracts/telemetry" + mockstelemetry "github.com/goravel/framework/mocks/telemetry" + "github.com/goravel/framework/support/color" + "github.com/goravel/framework/telemetry" +) + +type TransportTestSuite struct { + suite.Suite + originalFacade contractstelemetry.Telemetry +} + +func (s *TransportTestSuite) SetupTest() { + s.originalFacade = telemetry.TelemetryFacade +} + +func (s *TransportTestSuite) TearDownTest() { + telemetry.TelemetryFacade = s.originalFacade +} + +func TestTransportTestSuite(t *testing.T) { + suite.Run(t, new(TransportTestSuite)) +} + +func (s *TransportTestSuite) TestNewTransport() { + tests := []struct { + name string + setup func(*mockstelemetry.Telemetry) + base http.RoundTripper + assert func(res http.RoundTripper) + }{ + { + name: "fallback: returns base when facade is nil", + setup: func(_ *mockstelemetry.Telemetry) { + telemetry.TelemetryFacade = nil + }, + base: http.DefaultTransport, + assert: func(res http.RoundTripper) { + // We expect it to return exactly what we passed + s.Equal(http.DefaultTransport, res) + }, + }, + { + name: "fallback: returns DefaultTransport when facade is nil AND base is nil", + setup: func(_ *mockstelemetry.Telemetry) { + telemetry.TelemetryFacade = nil + }, + base: nil, + assert: func(res http.RoundTripper) { + s.NotNil(res) + s.Equal(http.DefaultTransport, res) + }, + }, + { + name: "success: returns wrapped transport when facade is set", + setup: func(mockTelemetry *mockstelemetry.Telemetry) { + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() + mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() + mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() + + telemetry.TelemetryFacade = mockTelemetry + }, + base: http.DefaultTransport, + assert: func(res http.RoundTripper) { + s.NotNil(res) + s.NotEqual(http.DefaultTransport, res) + }, + }, + { + name: "success: handles nil base automatically (otelhttp behavior)", + setup: func(mockTelemetry *mockstelemetry.Telemetry) { + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() + mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() + mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() + + telemetry.TelemetryFacade = mockTelemetry + }, + base: nil, + assert: func(res http.RoundTripper) { + // otelhttp.NewTransport(nil) will wrap DefaultTransport. + // So result should be NotNil. + s.NotNil(res) + }, + }, + } + + for _, test := range tests { + s.Run(test.name, func() { + mockTelemetry := mockstelemetry.NewTelemetry(s.T()) + + test.setup(mockTelemetry) + + var res http.RoundTripper + color.CaptureOutput(func(w io.Writer) { + res = NewTransport(test.base) + }) + + test.assert(res) + }) + } +} From 8df0782e86385e0edcc4df4ea058c0543709e7ca Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 30 Dec 2025 01:35:00 +0530 Subject: [PATCH 03/29] optimise http telemetry package --- telemetry/instrumentation/http/config.go | 35 +++-- telemetry/instrumentation/http/middleware.go | 134 ++++++++++--------- 2 files changed, 97 insertions(+), 72 deletions(-) diff --git a/telemetry/instrumentation/http/config.go b/telemetry/instrumentation/http/config.go index 9bed20e84..991800a7f 100644 --- a/telemetry/instrumentation/http/config.go +++ b/telemetry/instrumentation/http/config.go @@ -3,19 +3,28 @@ package http import ( "fmt" + "go.opentelemetry.io/otel/attribute" + "github.com/goravel/framework/contracts/http" ) +// Filter allows excluding specific requests from being traced. type Filter func(ctx http.Context) bool +// SpanNameFormatter allows customizing the span name. type SpanNameFormatter func(route string, ctx http.Context) string +// Option applies configuration to the server instrumentation. type Option func(*ServerConfig) -func WithSpanNameFormatter(f SpanNameFormatter) Option { - return func(c *ServerConfig) { - c.SpanNameFormatter = f - } +// ServerConfig maps to "telemetry.instrumentation.http_server". +type ServerConfig struct { + Enabled bool `mapstructure:"enabled"` + ExcludedPaths []string `mapstructure:"excluded_paths"` + ExcludedMethods []string `mapstructure:"excluded_methods"` + Filters []Filter `mapstructure:"-"` + SpanNameFormatter SpanNameFormatter `mapstructure:"-"` + MetricAttributes []attribute.KeyValue `mapstructure:"-"` } func WithFilter(f Filter) Option { @@ -24,14 +33,16 @@ func WithFilter(f Filter) Option { } } -// ServerConfig maps to the "telemetry.instrumentation.http_server" key in the config file. -type ServerConfig struct { - Enabled bool `mapstructure:"enabled"` - Name string `mapstructure:"name"` - ExcludedPaths []string `mapstructure:"excluded_paths"` - ExcludedMethods []string `mapstructure:"excluded_methods"` - SpanNameFormatter SpanNameFormatter `mapstructure:"span_name_formatter"` - Filters []Filter `mapstructure:"filters"` +func WithSpanNameFormatter(f SpanNameFormatter) Option { + return func(c *ServerConfig) { + c.SpanNameFormatter = f + } +} + +func WithMetricAttributes(attrs ...attribute.KeyValue) Option { + return func(c *ServerConfig) { + c.MetricAttributes = append(c.MetricAttributes, attrs...) + } } func defaultSpanNameFormatter(route string, ctx http.Context) string { diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index e89e9bf9a..8f11059bf 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -4,54 +4,73 @@ import ( "fmt" "time" - "go.opentelemetry.io/otel/metric" - semconv "go.opentelemetry.io/otel/semconv/v1.37.0" - "github.com/goravel/framework/contracts/http" + "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/propagation" + semconv "go.opentelemetry.io/otel/semconv/v1.37.0" ) -const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/http" +const ( + instrumentationName = "github.com/goravel/framework/telemetry/instrumentation/http" + + metricRequestDuration = "http.server.request.duration" + metricRequestBodySize = "http.server.request.body.size" + metricResponseBodySize = "http.server.response.body.size" + + unitSeconds = "s" + unitBytes = "By" +) func Telemetry(opts ...Option) http.Middleware { + if telemetry.TelemetryFacade == nil { + color.Warningln("[Telemetry] Facade not initialized. HTTP middleware disabled.") + return func(ctx http.Context) { ctx.Request().Next() } + } + var cfg ServerConfig _ = telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &cfg) + for _, opt := range opts { opt(&cfg) } if !cfg.Enabled { - return noopMiddleware() + return func(ctx http.Context) { ctx.Request().Next() } } if cfg.SpanNameFormatter == nil { cfg.SpanNameFormatter = defaultSpanNameFormatter } - instrumentationName := cfg.Name - if instrumentationName == "" { - instrumentationName = defaultInstrumentationName - } - tracer := telemetry.TelemetryFacade.Tracer(instrumentationName) meter := telemetry.TelemetryFacade.Meter(instrumentationName) propagator := telemetry.TelemetryFacade.Propagator() - durationHist, _ := meter.Float64Histogram("http.server.request.duration", metric.WithUnit("s"), metric.WithDescription("Duration of HTTP server requests")) - requestSizeHist, _ := meter.Int64Histogram("http.server.request.body.size", metric.WithUnit("By"), metric.WithDescription("Size of HTTP server request body")) - responseSizeHist, _ := meter.Int64Histogram("http.server.response.body.size", metric.WithUnit("By"), metric.WithDescription("Size of HTTP server response body")) + durationHist, _ := meter.Float64Histogram(metricRequestDuration, metric.WithUnit(unitSeconds), metric.WithDescription("Duration of HTTP server requests")) + requestSizeHist, _ := meter.Int64Histogram(metricRequestBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server request body")) + responseSizeHist, _ := meter.Int64Histogram(metricResponseBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server response body")) + + excludedPaths := make(map[string]bool, len(cfg.ExcludedPaths)) + for _, p := range cfg.ExcludedPaths { + excludedPaths[p] = true + } + excludedMethods := make(map[string]bool, len(cfg.ExcludedMethods)) + for _, m := range cfg.ExcludedMethods { + excludedMethods[m] = true + } - excludedPathsMap := sliceToMap(cfg.ExcludedPaths) - excludedMethodsMap := sliceToMap(cfg.ExcludedMethods) return func(ctx http.Context) { - start := time.Now() req := ctx.Request() - routePattern := req.OriginPath() - if routePattern == "" { - routePattern = req.Path() + routePath := req.OriginPath() + if routePath == "" { + routePath = req.Path() } - if excludedPathsMap[routePattern] || excludedMethodsMap[req.Method()] { + + if excludedPaths[routePath] || excludedMethods[req.Method()] { req.Next() return } @@ -63,9 +82,9 @@ func Telemetry(opts ...Option) http.Middleware { } } - parentCtx := propagator.Extract(ctx.Context(), telemetry.PropagationHeaderCarrier(req.Headers())) - - spanName := cfg.SpanNameFormatter(routePattern, ctx) + start := time.Now() + parentCtx := propagator.Extract(ctx.Context(), propagation.HeaderCarrier(req.Headers())) + spanName := cfg.SpanNameFormatter(routePath, ctx) scheme := "http" if proto := req.Header("X-Forwarded-Proto"); proto != "" { @@ -73,14 +92,16 @@ func Telemetry(opts ...Option) http.Middleware { } baseAttrs := []telemetry.KeyValue{ - semconv.HTTPRequestMethodOriginal(req.Method()), - semconv.HTTPRoute(routePattern), + semconv.HTTPRequestMethodKey.String(req.Method()), + semconv.HTTPRoute(routePath), semconv.URLScheme(scheme), semconv.ServerAddress(req.Host()), semconv.ClientAddress(req.Ip()), semconv.UserAgentOriginal(req.Header("User-Agent")), } + baseAttrs = append(baseAttrs, cfg.MetricAttributes...) + spanCtx, span := tracer.Start(parentCtx, spanName, telemetry.WithAttributes(baseAttrs...), telemetry.WithSpanKind(telemetry.SpanKindServer), @@ -88,61 +109,54 @@ func Telemetry(opts ...Option) http.Middleware { ctx.WithContext(spanCtx) - defer func() { - if r := recover(); r != nil { - err := fmt.Errorf("panic: %v", r) - span.RecordError(err) - span.SetStatus(telemetry.CodeError, "Internal Server Error") - span.End() + func() { + defer func() { + if r := recover(); r != nil { + err := fmt.Errorf("panic: %v", r) + span.RecordError(err) + span.SetStatus(codes.Error, "Internal Server Error") - metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(500))...) - durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(500))...) - panic(r) - } else { - span.End() - } - }() + durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) + responseSizeHist.Record(spanCtx, 0, metricAttrs) - req.Next() + span.End() + panic(r) + } + }() + req.Next() + }() status := ctx.Response().Origin().Status() + span.SetAttributes(semconv.HTTPResponseStatusCode(status)) if status >= 500 { - span.SetStatus(telemetry.CodeError, "") + span.SetStatus(codes.Error, "") } else { - span.SetStatus(telemetry.CodeOk, "") + span.SetStatus(codes.Ok, "") } if err := ctx.Err(); err != nil { span.RecordError(err) } - metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) - - reqSize := req.Origin().ContentLength - if reqSize < 0 { - reqSize = 0 - } - requestSizeHist.Record(spanCtx, reqSize, metricAttrs) + span.End() - responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) + responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) } } -func noopMiddleware() http.Middleware { - return func(ctx http.Context) { - ctx.Request().Next() - } -} - -func sliceToMap(slice []string) map[string]bool { - m := make(map[string]bool, len(slice)) - for _, s := range slice { - m[s] = true +func getRequestSize(req http.ContextRequest) int64 { + size := req.Origin().ContentLength + if size < 0 { + return 0 } - return m + return size } From 01ecf4b5a3daeefb6fe03609f83aea293e94797c Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 30 Dec 2025 01:50:44 +0530 Subject: [PATCH 04/29] add test cases for Telemetry middleware --- .../instrumentation/http/middleware_test.go | 297 ++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 telemetry/instrumentation/http/middleware_test.go diff --git a/telemetry/instrumentation/http/middleware_test.go b/telemetry/instrumentation/http/middleware_test.go new file mode 100644 index 000000000..3d4e6b34d --- /dev/null +++ b/telemetry/instrumentation/http/middleware_test.go @@ -0,0 +1,297 @@ +package http + +import ( + "bytes" + "context" + nethttp "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + metricnoop "go.opentelemetry.io/otel/metric/noop" + "go.opentelemetry.io/otel/propagation" + tracenoop "go.opentelemetry.io/otel/trace/noop" + + contractshttp "github.com/goravel/framework/contracts/http" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" + configmocks "github.com/goravel/framework/mocks/config" + telemetrymocks "github.com/goravel/framework/mocks/telemetry" + "github.com/goravel/framework/telemetry" +) + +type MiddlewareTestSuite struct { + suite.Suite + originalTelemetry contractstelemetry.Telemetry +} + +func (s *MiddlewareTestSuite) SetupTest() { + s.originalTelemetry = telemetry.TelemetryFacade +} + +func (s *MiddlewareTestSuite) TearDownTest() { + telemetry.TelemetryFacade = s.originalTelemetry +} + +func TestMiddlewareTestSuite(t *testing.T) { + suite.Run(t, new(MiddlewareTestSuite)) +} + +func (s *MiddlewareTestSuite) TestTelemetry() { + defaultTelemetrySetup := func(mockTelemetry *telemetrymocks.Telemetry) { + mockTelemetry.EXPECT().Tracer(instrumentationName).Return(tracenoop.NewTracerProvider().Tracer("test")).Once() + mockTelemetry.EXPECT().Meter(instrumentationName).Return(metricnoop.NewMeterProvider().Meter("test")).Once() + mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() + } + + tests := []struct { + name string + configSetup func(*configmocks.Config) + telemetrySetup func(*telemetrymocks.Telemetry) + handler nethttp.HandlerFunc + requestPath string + expectPanic bool + }{ + { + name: "Success: Request is traced and metrics recorded", + requestPath: "/users", + configSetup: func(mockConfig *configmocks.Config) { + mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). + Run(func(_ string, cfg any) { + c := cfg.(*ServerConfig) + c.Enabled = true + }).Return(nil).Once() + }, + telemetrySetup: defaultTelemetrySetup, + handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { + w.WriteHeader(nethttp.StatusOK) + _, _ = w.Write([]byte("OK")) + }, + }, + { + name: "Ignored: Excluded path is skipped", + requestPath: "/health", + configSetup: func(mockConfig *configmocks.Config) { + mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). + Run(func(_ string, cfg interface{}) { + c := cfg.(*ServerConfig) + c.Enabled = true + c.ExcludedPaths = []string{"/health"} + }).Return(nil).Once() + }, + telemetrySetup: defaultTelemetrySetup, + handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { + w.WriteHeader(nethttp.StatusOK) + }, + }, + { + name: "Ignored: Disabled via config", + requestPath: "/users", + configSetup: func(mockConfig *configmocks.Config) { + mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). + Run(func(_ string, cfg interface{}) { + c := cfg.(*ServerConfig) + c.Enabled = false + }).Return(nil).Once() + }, + telemetrySetup: func(mockTelemetry *telemetrymocks.Telemetry) { + // If disabled, Tracer/Meter should NOT be initialized + }, + handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { + w.WriteHeader(nethttp.StatusOK) + }, + }, + { + name: "Panic: Metrics recorded as 500 and panic re-thrown", + requestPath: "/crash", + expectPanic: true, + configSetup: func(mockConfig *configmocks.Config) { + mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). + Run(func(_ string, cfg any) { + c := cfg.(*ServerConfig) + c.Enabled = true + }).Return(nil).Once() + }, + telemetrySetup: defaultTelemetrySetup, + handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { + panic("server crash") + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + mockConfig := configmocks.NewConfig(s.T()) + mockTelemetry := telemetrymocks.NewTelemetry(s.T()) + + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + tt.configSetup(mockConfig) + tt.telemetrySetup(mockTelemetry) + + handler := testMiddleware(tt.handler) + server := httptest.NewServer(handler) + defer server.Close() + + client := &nethttp.Client{} + + action := func() { + _, err := client.Get(server.URL + tt.requestPath) + if !tt.expectPanic { + s.NoError(err) + } + } + + action() + }) + } +} + +func testMiddleware(next nethttp.Handler) nethttp.Handler { + return nethttp.HandlerFunc(func(w nethttp.ResponseWriter, r *nethttp.Request) { + mw := Telemetry() + ctx := NewTestContext(r.Context(), next, w, r) + mw(ctx) + }) +} + +type TestContext struct { + ctx context.Context + next nethttp.Handler + request *nethttp.Request + writer *TestResponseWriter +} + +func NewTestContext(ctx context.Context, next nethttp.Handler, w nethttp.ResponseWriter, r *nethttp.Request) *TestContext { + return &TestContext{ + ctx: ctx, + next: next, + request: r, + writer: &TestResponseWriter{ResponseWriter: w, status: 200}, + } +} + +func (c *TestContext) Request() contractshttp.ContextRequest { + return NewTestRequest(c) +} + +func (c *TestContext) Response() contractshttp.ContextResponse { + return NewTestResponse(c) +} + +func (c *TestContext) WithContext(ctx context.Context) { + c.ctx = ctx + c.request = c.request.WithContext(ctx) +} + +func (c *TestContext) Context() context.Context { + return c.ctx +} + +func (c *TestContext) Err() error { + return c.ctx.Err() +} + +func (c *TestContext) Deadline() (deadline time.Time, ok bool) { return c.ctx.Deadline() } +func (c *TestContext) Done() <-chan struct{} { return c.ctx.Done() } +func (c *TestContext) Value(key any) any { return c.ctx.Value(key) } +func (c *TestContext) WithValue(key any, value any) { c.ctx = context.WithValue(c.ctx, key, value) } + +type TestRequest struct { + contractshttp.ContextRequest + ctx *TestContext +} + +func NewTestRequest(ctx *TestContext) *TestRequest { + return &TestRequest{ctx: ctx} +} + +func (r *TestRequest) Next() { + r.ctx.next.ServeHTTP(r.ctx.writer, r.ctx.request) +} + +func (r *TestRequest) Method() string { + return r.ctx.request.Method +} + +func (r *TestRequest) Path() string { + return r.ctx.request.URL.Path +} + +func (r *TestRequest) OriginPath() string { + return r.ctx.request.URL.Path +} + +func (r *TestRequest) Headers() nethttp.Header { + return r.ctx.request.Header +} + +func (r *TestRequest) Header(key string, defaultValue ...string) string { + val := r.ctx.request.Header.Get(key) + if val == "" && len(defaultValue) > 0 { + return defaultValue[0] + } + return val +} + +func (r *TestRequest) Host() string { + return r.ctx.request.Host +} + +func (r *TestRequest) Ip() string { + return "127.0.0.1" +} + +func (r *TestRequest) Origin() *nethttp.Request { + return r.ctx.request +} + +type TestResponse struct { + contractshttp.ContextResponse + ctx *TestContext +} + +func NewTestResponse(ctx *TestContext) *TestResponse { + return &TestResponse{ctx: ctx} +} + +func (r *TestResponse) Origin() contractshttp.ResponseOrigin { + return r.ctx.writer +} + +// --- TestResponseWriter --- + +type TestResponseWriter struct { + nethttp.ResponseWriter + status int + size int +} + +func (w *TestResponseWriter) WriteHeader(code int) { + w.status = code + w.ResponseWriter.WriteHeader(code) +} + +func (w *TestResponseWriter) Write(b []byte) (int, error) { + n, err := w.ResponseWriter.Write(b) + w.size += n + return n, err +} + +func (w *TestResponseWriter) Status() int { + return w.status +} + +func (w *TestResponseWriter) Size() int { + return w.size +} + +func (w *TestResponseWriter) Header() nethttp.Header { + return w.ResponseWriter.Header() +} + +func (w *TestResponseWriter) Body() *bytes.Buffer { + return nil +} From 0d8fda5af8d4cef354deae295bdf64d2133b0e13 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 30 Dec 2025 01:59:40 +0530 Subject: [PATCH 05/29] add auto instrumentation configs --- telemetry/setup/stubs.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/telemetry/setup/stubs.go b/telemetry/setup/stubs.go index f8a27ca67..3fd17165c 100644 --- a/telemetry/setup/stubs.go +++ b/telemetry/setup/stubs.go @@ -131,6 +131,20 @@ func init() { }, }, + // Instrumentation Configuration + // + // Configures the automatic instrumentation for specific components. + "instrumentation": map[string]any{ + // HTTP Server Instrumentation + // + // Configures the telemetry middleware for incoming HTTP requests. + "http_server": map[string]any{ + "enabled": config.Env("OTEL_HTTP_SERVER_ENABLED", true), + "excluded_paths": []string{}, // e.g., ["/health", "/metrics"] + "excluded_methods": []string{}, // e.g., ["OPTIONS", "HEAD"] + }, + }, + // Exporters Configuration // // Defines the details for connecting to external telemetry backends. From 395f1096c97c410dd76e37b4ea5d9cf361c15037 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sat, 3 Jan 2026 02:28:55 +0530 Subject: [PATCH 06/29] add config to enable Telemetry for http clients --- http/client/config.go | 3 +++ http/client/factory.go | 15 ++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/http/client/config.go b/http/client/config.go index d35727aa0..6a2d05ad0 100644 --- a/http/client/config.go +++ b/http/client/config.go @@ -33,4 +33,7 @@ type Config struct { // IdleConnTimeout is the maximum amount of time an idle (keep-alive) connection // will remain idle before closing itself. IdleConnTimeout time.Duration `json:"idle_conn_timeout"` + + // EnableTelemetry determines if OpenTelemetry tracing/metrics are enabled for this client. + EnableTelemetry bool `json:"enable_telemetry"` } diff --git a/http/client/factory.go b/http/client/factory.go index ef10b90e1..e45c42e02 100644 --- a/http/client/factory.go +++ b/http/client/factory.go @@ -7,6 +7,7 @@ import ( "github.com/goravel/framework/contracts/foundation" "github.com/goravel/framework/contracts/http/client" httperrors "github.com/goravel/framework/errors" + telemetryhttp "github.com/goravel/framework/telemetry/instrumentation/http" ) var _ client.Factory = (*Factory)(nil) @@ -105,13 +106,17 @@ func (f *Factory) resolveClient(name string) (*http.Client, error) { func (f *Factory) createHTTPClient(cfg *Config) *http.Client { // Clone the default transport to ensure strict isolation between clients. // This prevents shared state (like global timeouts) from leaking between instances. - transport := http.DefaultTransport.(*http.Transport).Clone() + baseTransport := http.DefaultTransport.(*http.Transport).Clone() - transport.MaxIdleConns = cfg.MaxIdleConns - transport.MaxIdleConnsPerHost = cfg.MaxIdleConnsPerHost - transport.MaxConnsPerHost = cfg.MaxConnsPerHost - transport.IdleConnTimeout = cfg.IdleConnTimeout + baseTransport.MaxIdleConns = cfg.MaxIdleConns + baseTransport.MaxIdleConnsPerHost = cfg.MaxIdleConnsPerHost + baseTransport.MaxConnsPerHost = cfg.MaxConnsPerHost + baseTransport.IdleConnTimeout = cfg.IdleConnTimeout + var transport http.RoundTripper = baseTransport + if cfg.EnableTelemetry { + transport = telemetryhttp.NewTransport(transport) + } return &http.Client{ Timeout: cfg.Timeout, Transport: transport, From bb1f529eed4b66b97f0e290f311e90a18dc8c515 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sat, 3 Jan 2026 02:32:38 +0530 Subject: [PATCH 07/29] add docs for config facade --- telemetry/instrumentation/http/middleware.go | 7 +++++++ telemetry/instrumentation/http/transport.go | 14 +++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index 8f11059bf..431c164ff 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -24,6 +24,13 @@ const ( unitBytes = "By" ) +// Telemetry creates HTTP server telemetry middleware that instruments incoming +// requests with tracing and metrics. The optional opts parameters allow +// customizing the server configuration (such as span naming and enabling or +// disabling instrumentation). It returns an http.Middleware that propagates +// context, records spans and metrics when telemetry is enabled, and otherwise +// transparently passes requests through when telemetry is disabled or not +// initialized. func Telemetry(opts ...Option) http.Middleware { if telemetry.TelemetryFacade == nil { color.Warningln("[Telemetry] Facade not initialized. HTTP middleware disabled.") diff --git a/telemetry/instrumentation/http/transport.go b/telemetry/instrumentation/http/transport.go index a4d63a5ab..56515dfa3 100644 --- a/telemetry/instrumentation/http/transport.go +++ b/telemetry/instrumentation/http/transport.go @@ -9,12 +9,20 @@ import ( "github.com/goravel/framework/telemetry" ) +// NewTransport returns an http.RoundTripper instrumented with OpenTelemetry. +// It wraps the provided base RoundTripper with otelhttp using the configured +// telemetry facade's tracer provider, meter provider, and propagator. +// +// If telemetry.TelemetryFacade is nil, a warning is logged and no +// instrumentation is applied. In that case, http.DefaultTransport is returned +// when base is nil; otherwise the provided base RoundTripper is returned. func NewTransport(base http.RoundTripper) http.RoundTripper { + if base == nil { + base = http.DefaultTransport + } + if telemetry.TelemetryFacade == nil { color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") - if base == nil { - return http.DefaultTransport - } return base } From 441ba965ed51a6a9c5db59a492ae848b0f98757b Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sat, 3 Jan 2026 02:34:53 +0530 Subject: [PATCH 08/29] add ConfigFacade nil warning --- telemetry/instrumentation/http/middleware.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index 431c164ff..fd524fede 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -37,6 +37,11 @@ func Telemetry(opts ...Option) http.Middleware { return func(ctx http.Context) { ctx.Request().Next() } } + if telemetry.ConfigFacade == nil { + color.Warningln("[Telemetry] Config facade not initialized. HTTP middleware disabled.") + return func(ctx http.Context) { ctx.Request().Next() } + } + var cfg ServerConfig _ = telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &cfg) From 7c59c97b63e80093ea836f13d55bc26ad1409384 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 15:32:34 +0530 Subject: [PATCH 09/29] disable default telemetry --- telemetry/setup/stubs.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/telemetry/setup/stubs.go b/telemetry/setup/stubs.go index 643de3170..c8e98a1d9 100644 --- a/telemetry/setup/stubs.go +++ b/telemetry/setup/stubs.go @@ -58,7 +58,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable tracing. - "exporter": config.Env("OTEL_TRACES_EXPORTER", "otlptrace"), + "exporter": config.Env("OTEL_TRACES_EXPORTER", ""), // Sampler Configuration // @@ -89,7 +89,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable metrics. - "exporter": config.Env("OTEL_METRICS_EXPORTER", "otlpmetric"), + "exporter": config.Env("OTEL_METRICS_EXPORTER", ""), // Reader Configuration // @@ -115,7 +115,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable OTel logging. - "exporter": config.Env("OTEL_LOGS_EXPORTER", "otlplog"), + "exporter": config.Env("OTEL_LOGS_EXPORTER", ""), // Processor Configuration // From ab72e2a98821bffb5115601573717d5e2663e96f Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 15:51:30 +0530 Subject: [PATCH 10/29] optimise transport --- telemetry/instrumentation/http/transport.go | 6 ++ .../instrumentation/http/transport_test.go | 66 +++++++++++++------ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/telemetry/instrumentation/http/transport.go b/telemetry/instrumentation/http/transport.go index 56515dfa3..bd60720a3 100644 --- a/telemetry/instrumentation/http/transport.go +++ b/telemetry/instrumentation/http/transport.go @@ -21,6 +21,12 @@ func NewTransport(base http.RoundTripper) http.RoundTripper { base = http.DefaultTransport } + // If telemetry.ConfigFacade is missing or set to false, return the original transport immediately. + // We use "telemetry.instrumentation.http_client" as the standard key for outgoing HTTP. + if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.http_client", true) { + return base + } + if telemetry.TelemetryFacade == nil { color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") return base diff --git a/telemetry/instrumentation/http/transport_test.go b/telemetry/instrumentation/http/transport_test.go index ade962496..599d76e87 100644 --- a/telemetry/instrumentation/http/transport_test.go +++ b/telemetry/instrumentation/http/transport_test.go @@ -10,7 +10,9 @@ import ( "go.opentelemetry.io/otel/propagation" tracenoop "go.opentelemetry.io/otel/trace/noop" + contractsconfig "github.com/goravel/framework/contracts/config" contractstelemetry "github.com/goravel/framework/contracts/telemetry" + mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" @@ -18,15 +20,18 @@ import ( type TransportTestSuite struct { suite.Suite - originalFacade contractstelemetry.Telemetry + originalTelemetryFacade contractstelemetry.Telemetry + originalConfigFacade contractsconfig.Config } func (s *TransportTestSuite) SetupTest() { - s.originalFacade = telemetry.TelemetryFacade + s.originalTelemetryFacade = telemetry.TelemetryFacade + s.originalConfigFacade = telemetry.ConfigFacade } func (s *TransportTestSuite) TearDownTest() { - telemetry.TelemetryFacade = s.originalFacade + telemetry.TelemetryFacade = s.originalTelemetryFacade + telemetry.ConfigFacade = s.originalConfigFacade } func TestTransportTestSuite(t *testing.T) { @@ -36,40 +41,55 @@ func TestTransportTestSuite(t *testing.T) { func (s *TransportTestSuite) TestNewTransport() { tests := []struct { name string - setup func(*mockstelemetry.Telemetry) + setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) base http.RoundTripper assert func(res http.RoundTripper) }{ { - name: "fallback: returns base when facade is nil", - setup: func(_ *mockstelemetry.Telemetry) { - telemetry.TelemetryFacade = nil + name: "fallback: returns base when ConfigFacade is nil", + setup: func(_ *mockstelemetry.Telemetry, _ *mocksconfig.Config) { + telemetry.ConfigFacade = nil }, base: http.DefaultTransport, assert: func(res http.RoundTripper) { - // We expect it to return exactly what we passed s.Equal(http.DefaultTransport, res) }, }, { - name: "fallback: returns DefaultTransport when facade is nil AND base is nil", - setup: func(_ *mockstelemetry.Telemetry) { + name: "kill switch: returns base when http_client is disabled in config", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(false).Once() + }, + base: http.DefaultTransport, + assert: func(res http.RoundTripper) { + s.Equal(http.DefaultTransport, res) + }, + }, + { + name: "fallback: returns base when TelemetryFacade is nil (even if config enabled)", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig telemetry.TelemetryFacade = nil + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() }, - base: nil, + base: http.DefaultTransport, assert: func(res http.RoundTripper) { - s.NotNil(res) s.Equal(http.DefaultTransport, res) }, }, { - name: "success: returns wrapped transport when facade is set", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + name: "success: returns wrapped transport when enabled and facades exist", + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, base: http.DefaultTransport, assert: func(res http.RoundTripper) { @@ -78,19 +98,22 @@ func (s *TransportTestSuite) TestNewTransport() { }, }, { - name: "success: handles nil base automatically (otelhttp behavior)", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + name: "success: handles nil base automatically (wraps DefaultTransport)", + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, base: nil, assert: func(res http.RoundTripper) { // otelhttp.NewTransport(nil) will wrap DefaultTransport. // So result should be NotNil. s.NotNil(res) + s.NotEqual(http.DefaultTransport, res) }, }, } @@ -98,8 +121,9 @@ func (s *TransportTestSuite) TestNewTransport() { for _, test := range tests { s.Run(test.name, func() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) + mockConfig := mocksconfig.NewConfig(s.T()) - test.setup(mockTelemetry) + test.setup(mockTelemetry, mockConfig) var res http.RoundTripper color.CaptureOutput(func(w io.Writer) { From 413bdd6fa59b80645ca16f1c3c5bcbee9553bbcf Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 16:18:34 +0530 Subject: [PATCH 11/29] add kill switch for instrumentation --- telemetry/instrumentation/grpc/handler.go | 8 ++ .../instrumentation/grpc/handler_test.go | 91 ++++++++++++++----- telemetry/instrumentation/log/channel.go | 15 ++- telemetry/instrumentation/log/channel_test.go | 20 +++- telemetry/instrumentation/log/handler.go | 18 +++- telemetry/instrumentation/log/handler_test.go | 6 +- 6 files changed, 123 insertions(+), 35 deletions(-) diff --git a/telemetry/instrumentation/grpc/handler.go b/telemetry/instrumentation/grpc/handler.go index bbd12f676..746e51493 100644 --- a/telemetry/instrumentation/grpc/handler.go +++ b/telemetry/instrumentation/grpc/handler.go @@ -10,6 +10,10 @@ import ( // NewServerStatsHandler creates an OTel stats handler for the server. func NewServerStatsHandler(opts ...Option) stats.Handler { + if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.grpc_server", true) { + return nil + } + if telemetry.TelemetryFacade == nil { color.Warningln("[Telemetry] Facade not initialized. gRPC server stats instrumentation is disabled.") return nil @@ -22,6 +26,10 @@ func NewServerStatsHandler(opts ...Option) stats.Handler { // NewClientStatsHandler creates an OTel stats handler for the client. func NewClientStatsHandler(opts ...Option) stats.Handler { + if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.grpc_client", true) { + return nil + } + if telemetry.TelemetryFacade == nil { color.Warningln("[Telemetry] Facade not initialized. gRPC client stats instrumentation is disabled.") return nil diff --git a/telemetry/instrumentation/grpc/handler_test.go b/telemetry/instrumentation/grpc/handler_test.go index c75754c4d..2e39643a0 100644 --- a/telemetry/instrumentation/grpc/handler_test.go +++ b/telemetry/instrumentation/grpc/handler_test.go @@ -10,7 +10,9 @@ import ( tracenoop "go.opentelemetry.io/otel/trace/noop" "google.golang.org/grpc/stats" + contractsconfig "github.com/goravel/framework/contracts/config" contractstelemetry "github.com/goravel/framework/contracts/telemetry" + mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" @@ -18,15 +20,18 @@ import ( type HandlerTestSuite struct { suite.Suite - originalFacade contractstelemetry.Telemetry + originalTelemetryFacade contractstelemetry.Telemetry + originalConfigFacade contractsconfig.Config } func (s *HandlerTestSuite) SetupTest() { - s.originalFacade = telemetry.TelemetryFacade + s.originalTelemetryFacade = telemetry.TelemetryFacade + s.originalConfigFacade = telemetry.ConfigFacade } func (s *HandlerTestSuite) TearDownTest() { - telemetry.TelemetryFacade = s.originalFacade + telemetry.TelemetryFacade = s.originalTelemetryFacade + telemetry.ConfigFacade = s.originalConfigFacade } func TestHandlerTestSuite(t *testing.T) { @@ -36,13 +41,26 @@ func TestHandlerTestSuite(t *testing.T) { func (s *HandlerTestSuite) TestServerStatsHandler() { tests := []struct { name string - setup func(*mockstelemetry.Telemetry) + setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) assert func() }{ { - name: "returns nil and logs warning when telemetry facade is nil", - setup: func(_ *mockstelemetry.Telemetry) { + name: "returns nil immediately if config is disabled", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(false).Once() + }, + assert: func() { + s.Nil(NewServerStatsHandler()) + }, + }, + { + name: "returns nil and logs warning when config enabled but telemetry facade is nil", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig telemetry.TelemetryFacade = nil + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() }, assert: func() { var handler stats.Handler @@ -55,13 +73,16 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { }, }, { - name: "returns handler when telemetry facade is set", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + name: "returns handler when enabled and facade is set", + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, assert: func() { s.NotNil(NewServerStatsHandler()) @@ -69,12 +90,15 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { }, { name: "accepts options", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, assert: func() { handler := NewServerStatsHandler( @@ -90,8 +114,9 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { for _, test := range tests { s.Run(test.name, func() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) + mockConfig := mocksconfig.NewConfig(s.T()) - test.setup(mockTelemetry) + test.setup(mockTelemetry, mockConfig) test.assert() }) } @@ -100,13 +125,26 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { func (s *HandlerTestSuite) TestClientStatsHandler() { tests := []struct { name string - setup func(*mockstelemetry.Telemetry) + setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) assert func() }{ + { + name: "returns nil immediately if config is disabled", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(false).Once() + }, + assert: func() { + s.Nil(NewClientStatsHandler()) + }, + }, { name: "returns nil and logs warning when telemetry facade is nil", - setup: func(_ *mockstelemetry.Telemetry) { + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig telemetry.TelemetryFacade = nil + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() }, assert: func() { var handler stats.Handler @@ -120,12 +158,15 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { }, { name: "returns handler when telemetry facade is set", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, assert: func() { s.NotNil(NewClientStatsHandler()) @@ -133,12 +174,15 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { }, { name: "accepts options", - setup: func(mockTelemetry *mockstelemetry.Telemetry) { + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + telemetry.ConfigFacade = mockConfig + telemetry.TelemetryFacade = mockTelemetry + + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() + mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - - telemetry.TelemetryFacade = mockTelemetry }, assert: func() { handler := NewClientStatsHandler( @@ -153,8 +197,9 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { for _, test := range tests { s.Run(test.name, func() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) + mockConfig := mocksconfig.NewConfig(s.T()) - test.setup(mockTelemetry) + test.setup(mockTelemetry, mockConfig) test.assert() }) } diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index 961ae61d5..a573af190 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -15,17 +15,22 @@ func NewTelemetryChannel() *TelemetryChannel { } func (r *TelemetryChannel) Handle(channelPath string) (log.Handler, error) { - if telemetry.TelemetryFacade == nil { - return nil, errors.TelemetryFacadeNotSet - } - config := telemetry.ConfigFacade if config == nil { return nil, errors.ConfigFacadeNotSet } + if !config.GetBool("telemetry.instrumentation.log", true) { + return &handler{enabled: false}, nil + } + + if telemetry.TelemetryFacade == nil { + return nil, errors.TelemetryFacadeNotSet + } + instrumentName := config.GetString(channelPath+".instrument_name", defaultInstrumentationName) return &handler{ - logger: telemetry.TelemetryFacade.Logger(instrumentName), + enabled: true, + logger: telemetry.TelemetryFacade.Logger(instrumentName), }, nil } diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index 8482b0dd8..bedeb81a3 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/suite" "go.opentelemetry.io/otel/log/noop" + "github.com/goravel/framework/contracts/log" "github.com/goravel/framework/errors" mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" @@ -37,8 +38,8 @@ func (s *TelemetryChannelTestSuite) TearDownTest() { func (s *TelemetryChannelTestSuite) TestHandle_Success_DefaultName() { channelPath := "logging.channels.otel" + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(noop.NewLoggerProvider().Logger("test")).Once() channel := NewTelemetryChannel() @@ -46,6 +47,7 @@ func (s *TelemetryChannelTestSuite) TestHandle_Success_DefaultName() { s.NoError(err) s.NotNil(h) + s.True(h.Enabled(log.LevelInfo)) s.mockTelemetry.AssertExpectations(s.T()) } @@ -53,8 +55,8 @@ func (s *TelemetryChannelTestSuite) TestHandle_Success_CustomName() { channelPath := "logging.channels.otel" customName := "my-service-logs" + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(customName).Once() - s.mockTelemetry.On("Logger", customName).Return(noop.NewLoggerProvider().Logger("test")).Once() channel := NewTelemetryChannel() @@ -62,12 +64,26 @@ func (s *TelemetryChannelTestSuite) TestHandle_Success_CustomName() { s.NoError(err) s.NotNil(h) + s.True(h.Enabled(log.LevelInfo)) s.mockTelemetry.AssertExpectations(s.T()) } +func (s *TelemetryChannelTestSuite) TestHandle_Disabled_KillSwitch() { + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(false).Once() + + channel := NewTelemetryChannel() + h, err := channel.Handle("logging.channels.otel") + + s.NoError(err) + s.NotNil(h) + s.False(h.Enabled(log.LevelInfo)) +} + func (s *TelemetryChannelTestSuite) TestHandle_Error_TelemetryFacadeNotSet() { telemetry.TelemetryFacade = nil + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() + channel := NewTelemetryChannel() h, err := channel.Handle("logging.channels.otel") diff --git a/telemetry/instrumentation/log/handler.go b/telemetry/instrumentation/log/handler.go index 99998eb29..5fbb4a408 100644 --- a/telemetry/instrumentation/log/handler.go +++ b/telemetry/instrumentation/log/handler.go @@ -12,15 +12,16 @@ import ( var _ contractslog.Handler = (*handler)(nil) type handler struct { - logger otellog.Logger + logger otellog.Logger + enabled bool } func (r *handler) Enabled(level contractslog.Level) bool { - return true + return r.enabled } func (r *handler) Handle(entry contractslog.Entry) error { - if r.logger == nil { + if !r.enabled || r.logger == nil { return nil } @@ -89,3 +90,14 @@ func (r *handler) convertEntry(e contractslog.Entry) otellog.Record { record.AddAttributes(attrs...) return record } + +// noopHandler is a hollow logger used when telemetry is disabled via config. +type noopHandler struct{} + +func (h *noopHandler) Enabled(level contractslog.Level) bool { + return false +} + +func (h *noopHandler) Handle(entry contractslog.Entry) error { + return nil +} diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index 6be9865e2..4a038bdc5 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -25,7 +25,8 @@ func (s *HandlerTestSuite) SetupTest() { s.loggerName = "test-logger" s.recorder = logtest.NewRecorder() s.handler = &handler{ - logger: s.recorder.Logger(s.loggerName), + enabled: true, + logger: s.recorder.Logger(s.loggerName), } s.now = time.Date(2024, 1, 1, 10, 0, 0, 0, time.UTC) @@ -187,7 +188,8 @@ func (s *HandlerTestSuite) TestHandle() { // Reset the recorder for each test case s.recorder = logtest.NewRecorder() s.handler = &handler{ - logger: s.recorder.Logger(s.loggerName), + enabled: true, + logger: s.recorder.Logger(s.loggerName), } err := s.handler.Handle(tt.entry) From f999029fafff21097f4958684061147282689e98 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 16:27:16 +0530 Subject: [PATCH 12/29] update the stubs --- telemetry/setup/stubs.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/telemetry/setup/stubs.go b/telemetry/setup/stubs.go index c8e98a1d9..1ee3169d6 100644 --- a/telemetry/setup/stubs.go +++ b/telemetry/setup/stubs.go @@ -143,6 +143,39 @@ func init() { "excluded_paths": []string{}, // e.g., ["/health", "/metrics"] "excluded_methods": []string{}, // e.g., ["OPTIONS", "HEAD"] }, + + // HTTP Client Instrumentation + // + // Configures the instrumentation for outgoing HTTP requests made via + // the application's HTTP client Facade. This acts as a global kill switch + // for all clients. To disable telemetry for specific clients, configure + // "enable_telemetry" within the "http.clients.{client_name}" property. + "http_client": map[string]any{ + "enabled": config.Env("OTEL_HTTP_CLIENT_ENABLED", true), + }, + + // gRPC Server Instrumentation + // + // Configures the instrumentation for incoming gRPC requests to your server. + "grpc_server": map[string]any{ + "enabled": config.Env("OTEL_GRPC_SERVER_ENABLED", true), + }, + + // gRPC Client Instrumentation + // + // Configures the instrumentation for outgoing gRPC calls made by your application. + "grpc_client": map[string]any{ + "enabled": config.Env("OTEL_GRPC_CLIENT_ENABLED", true), + }, + + // Log Instrumentation + // + // Configures the instrumentation for the application logger. + // Disabling this acts as a global kill switch for sending logs to the OTel exporter, + // which can be useful for reducing cost/noise without changing logging config. + "log": map[string]any{ + "enabled": config.Env("OTEL_LOG_ENABLED", true), + }, }, // Exporters Configuration From 98ec1f79cbe018710888bbff1d4e47b24d67f04f Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 16:28:11 +0530 Subject: [PATCH 13/29] remove unnecessary handler --- telemetry/instrumentation/log/handler.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/telemetry/instrumentation/log/handler.go b/telemetry/instrumentation/log/handler.go index 5fbb4a408..ae1a71d78 100644 --- a/telemetry/instrumentation/log/handler.go +++ b/telemetry/instrumentation/log/handler.go @@ -90,14 +90,3 @@ func (r *handler) convertEntry(e contractslog.Entry) otellog.Record { record.AddAttributes(attrs...) return record } - -// noopHandler is a hollow logger used when telemetry is disabled via config. -type noopHandler struct{} - -func (h *noopHandler) Enabled(level contractslog.Level) bool { - return false -} - -func (h *noopHandler) Handle(entry contractslog.Entry) error { - return nil -} From ad59c56045c9e0652d1e57c8fcd127ae7e42cff2 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 18:27:12 +0530 Subject: [PATCH 14/29] optimise log instrumentation --- log/application.go | 2 +- telemetry/instrumentation/log/channel.go | 30 +++---- telemetry/instrumentation/log/channel_test.go | 80 +++++++++++-------- telemetry/instrumentation/log/handler.go | 32 +++++++- telemetry/instrumentation/log/handler_test.go | 65 +++++++++++---- 5 files changed, 140 insertions(+), 69 deletions(-) diff --git a/log/application.go b/log/application.go index 02a9d2317..a1cb51f18 100644 --- a/log/application.go +++ b/log/application.go @@ -156,7 +156,7 @@ func getHandlers(config config.Config, json foundation.Json, channel string) ([] handlers = append(handlers, HandlerToSlogHandler(logger.NewConsoleHandler(config, json, level, formatter))) } case log.DriverOtel: - logLogger := telemetrylog.NewTelemetryChannel() + logLogger := telemetrylog.NewTelemetryChannel(config) handler, err := logLogger.Handle(channelPath) if err != nil { return nil, err diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index a573af190..d71c54ab5 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -1,36 +1,30 @@ package log import ( + "github.com/goravel/framework/contracts/config" "github.com/goravel/framework/contracts/log" - "github.com/goravel/framework/errors" - "github.com/goravel/framework/telemetry" ) const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/log" -type TelemetryChannel struct{} - -func NewTelemetryChannel() *TelemetryChannel { - return &TelemetryChannel{} +type TelemetryChannel struct { + config config.Config } -func (r *TelemetryChannel) Handle(channelPath string) (log.Handler, error) { - config := telemetry.ConfigFacade - if config == nil { - return nil, errors.ConfigFacadeNotSet +func NewTelemetryChannel(config config.Config) *TelemetryChannel { + return &TelemetryChannel{ + config: config, } +} - if !config.GetBool("telemetry.instrumentation.log", true) { +func (r *TelemetryChannel) Handle(channelPath string) (log.Handler, error) { + if !r.config.GetBool("telemetry.instrumentation.log", true) { return &handler{enabled: false}, nil } - if telemetry.TelemetryFacade == nil { - return nil, errors.TelemetryFacadeNotSet - } - - instrumentName := config.GetString(channelPath+".instrument_name", defaultInstrumentationName) + instrumentName := r.config.GetString(channelPath+".instrument_name", defaultInstrumentationName) return &handler{ - enabled: true, - logger: telemetry.TelemetryFacade.Logger(instrumentName), + enabled: true, + instrumentName: instrumentName, }, nil } diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index bedeb81a3..bf90d296f 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -1,14 +1,16 @@ package log import ( + "context" "testing" + "time" "github.com/stretchr/testify/suite" "go.opentelemetry.io/otel/log/noop" - "github.com/goravel/framework/contracts/log" - "github.com/goravel/framework/errors" + contractslog "github.com/goravel/framework/contracts/log" mocksconfig "github.com/goravel/framework/mocks/config" + mockslog "github.com/goravel/framework/mocks/log" mockstelemetry "github.com/goravel/framework/mocks/telemetry" "github.com/goravel/framework/telemetry" ) @@ -17,6 +19,7 @@ type TelemetryChannelTestSuite struct { suite.Suite mockConfig *mocksconfig.Config mockTelemetry *mockstelemetry.Telemetry + mockEntry *mockslog.Entry } func TestTelemetryChannelTestSuite(t *testing.T) { @@ -26,77 +29,90 @@ func TestTelemetryChannelTestSuite(t *testing.T) { func (s *TelemetryChannelTestSuite) SetupTest() { s.mockConfig = mocksconfig.NewConfig(s.T()) s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) + s.mockEntry = mockslog.NewEntry(s.T()) - telemetry.ConfigFacade = s.mockConfig telemetry.TelemetryFacade = s.mockTelemetry } func (s *TelemetryChannelTestSuite) TearDownTest() { - telemetry.ConfigFacade = nil telemetry.TelemetryFacade = nil } -func (s *TelemetryChannelTestSuite) TestHandle_Success_DefaultName() { +func (s *TelemetryChannelTestSuite) TestHandle_Factory_Success_DefaultName() { channelPath := "logging.channels.otel" + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(noop.NewLoggerProvider().Logger("test")).Once() - channel := NewTelemetryChannel() + channel := NewTelemetryChannel(s.mockConfig) h, err := channel.Handle(channelPath) s.NoError(err) s.NotNil(h) - s.True(h.Enabled(log.LevelInfo)) - s.mockTelemetry.AssertExpectations(s.T()) + + impl, ok := h.(*handler) + s.True(ok) + s.True(impl.enabled) + s.Equal(defaultInstrumentationName, impl.instrumentName) } -func (s *TelemetryChannelTestSuite) TestHandle_Success_CustomName() { +func (s *TelemetryChannelTestSuite) TestHandle_Factory_Success_CustomName() { channelPath := "logging.channels.otel" customName := "my-service-logs" s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(customName).Once() - s.mockTelemetry.On("Logger", customName).Return(noop.NewLoggerProvider().Logger("test")).Once() - channel := NewTelemetryChannel() + channel := NewTelemetryChannel(s.mockConfig) h, err := channel.Handle(channelPath) s.NoError(err) s.NotNil(h) - s.True(h.Enabled(log.LevelInfo)) - s.mockTelemetry.AssertExpectations(s.T()) + + impl, ok := h.(*handler) + s.True(ok) + s.Equal(customName, impl.instrumentName) } -func (s *TelemetryChannelTestSuite) TestHandle_Disabled_KillSwitch() { +func (s *TelemetryChannelTestSuite) TestHandle_Factory_Disabled() { s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(false).Once() - channel := NewTelemetryChannel() + channel := NewTelemetryChannel(s.mockConfig) h, err := channel.Handle("logging.channels.otel") s.NoError(err) s.NotNil(h) - s.False(h.Enabled(log.LevelInfo)) + + impl, ok := h.(*handler) + s.True(ok) + s.False(impl.enabled) + s.False(h.Enabled(contractslog.LevelInfo)) } -func (s *TelemetryChannelTestSuite) TestHandle_Error_TelemetryFacadeNotSet() { - telemetry.TelemetryFacade = nil +func (s *TelemetryChannelTestSuite) TestHandle_Runtime_LazyLoading_TriggersTelemetry() { + channelPath := "logging.channels.otel" s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() + s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - channel := NewTelemetryChannel() - h, err := channel.Handle("logging.channels.otel") - - s.ErrorIs(err, errors.TelemetryFacadeNotSet) - s.Nil(h) -} - -func (s *TelemetryChannelTestSuite) TestHandle_Error_ConfigFacadeNotSet() { - telemetry.ConfigFacade = nil + s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(noop.NewLoggerProvider().Logger("test")).Once() - channel := NewTelemetryChannel() - h, err := channel.Handle("logging.channels.otel") + s.mockEntry.On("Context").Return(context.Background()) + s.mockEntry.On("Time").Return(time.Now()) + s.mockEntry.On("Message").Return("test message") + s.mockEntry.On("Level").Return(contractslog.InfoLevel) + s.mockEntry.On("Code").Return("") + s.mockEntry.On("Domain").Return("") + s.mockEntry.On("Hint").Return("") + s.mockEntry.On("Owner").Return(nil) + s.mockEntry.On("User").Return(nil) + s.mockEntry.On("With").Return(map[string]any{}) + s.mockEntry.On("Data").Return(map[string]any{}) + + channel := NewTelemetryChannel(s.mockConfig) + h, err := channel.Handle(channelPath) + s.NoError(err) + s.NoError(h.Handle(s.mockEntry)) - s.ErrorIs(err, errors.ConfigFacadeNotSet) - s.Nil(h) + s.mockTelemetry.AssertExpectations(s.T()) } diff --git a/telemetry/instrumentation/log/handler.go b/telemetry/instrumentation/log/handler.go index ae1a71d78..cc81217fd 100644 --- a/telemetry/instrumentation/log/handler.go +++ b/telemetry/instrumentation/log/handler.go @@ -2,8 +2,10 @@ package log import ( "context" + "sync" "time" + "github.com/goravel/framework/telemetry" otellog "go.opentelemetry.io/otel/log" contractslog "github.com/goravel/framework/contracts/log" @@ -12,8 +14,10 @@ import ( var _ contractslog.Handler = (*handler)(nil) type handler struct { - logger otellog.Logger - enabled bool + enabled bool + instrumentName string + logger otellog.Logger + mu sync.Mutex } func (r *handler) Enabled(level contractslog.Level) bool { @@ -21,7 +25,12 @@ func (r *handler) Enabled(level contractslog.Level) bool { } func (r *handler) Handle(entry contractslog.Entry) error { - if !r.enabled || r.logger == nil { + if !r.enabled { + return nil + } + + logger := r.getLogger() + if logger == nil { return nil } @@ -30,11 +39,26 @@ func (r *handler) Handle(entry contractslog.Entry) error { ctx = context.Background() } - r.logger.Emit(ctx, r.convertEntry(entry)) + logger.Emit(ctx, r.convertEntry(entry)) return nil } +func (r *handler) getLogger() otellog.Logger { + r.mu.Lock() + defer r.mu.Unlock() + + if r.logger != nil { + return r.logger + } + + if telemetry.TelemetryFacade != nil { + r.logger = telemetry.TelemetryFacade.Logger(r.instrumentName) + } + + return r.logger +} + func (r *handler) convertEntry(e contractslog.Entry) otellog.Record { var record otellog.Record record.SetTimestamp(e.Time()) diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index 4a038bdc5..821a23547 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -10,23 +10,32 @@ import ( "go.opentelemetry.io/otel/log/logtest" contractslog "github.com/goravel/framework/contracts/log" + mockstelemetry "github.com/goravel/framework/mocks/telemetry" + "github.com/goravel/framework/telemetry" ) type HandlerTestSuite struct { suite.Suite - recorder *logtest.Recorder - handler *handler - loggerName string - ctx context.Context - now time.Time + recorder *logtest.Recorder + handler *handler + loggerName string + ctx context.Context + now time.Time + mockTelemetry *mockstelemetry.Telemetry // Added for lazy test +} + +func TestHandlerTestSuite(t *testing.T) { + suite.Run(t, new(HandlerTestSuite)) } func (s *HandlerTestSuite) SetupTest() { s.loggerName = "test-logger" s.recorder = logtest.NewRecorder() + s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) s.handler = &handler{ - enabled: true, - logger: s.recorder.Logger(s.loggerName), + enabled: true, + instrumentName: s.loggerName, + logger: s.recorder.Logger(s.loggerName), } s.now = time.Date(2024, 1, 1, 10, 0, 0, 0, time.UTC) @@ -34,6 +43,38 @@ func (s *HandlerTestSuite) SetupTest() { s.ctx = context.WithValue(context.Background(), ctxKey("request_id"), "req-123") } +func (s *HandlerTestSuite) TearDownTest() { + telemetry.TelemetryFacade = nil +} + +func (s *HandlerTestSuite) TestHandle_Lazy_Success() { + s.handler.logger = nil + telemetry.TelemetryFacade = s.mockTelemetry + s.mockTelemetry.On("Logger", s.loggerName).Return(s.recorder.Logger(s.loggerName)).Once() + + entry := &TestEntry{ + ctx: context.Background(), + level: contractslog.LevelInfo, + time: s.now, + } + err := s.handler.Handle(entry) + s.NoError(err) + s.mockTelemetry.AssertExpectations(s.T()) + s.NotNil(s.handler.logger) +} + +func (s *HandlerTestSuite) TestHandle_Lazy_FacadeNotSet() { + s.handler.logger = nil + telemetry.TelemetryFacade = nil + entry := &TestEntry{ + ctx: context.Background(), + level: contractslog.LevelInfo, + } + err := s.handler.Handle(entry) + s.NoError(err) + s.Nil(s.handler.logger) +} + func (s *HandlerTestSuite) TestEnabled() { s.True(s.handler.Enabled(contractslog.LevelDebug)) s.True(s.handler.Enabled(contractslog.LevelInfo)) @@ -185,11 +226,11 @@ func (s *HandlerTestSuite) TestHandle() { for _, tt := range tests { s.Run(tt.name, func() { - // Reset the recorder for each test case s.recorder = logtest.NewRecorder() s.handler = &handler{ - enabled: true, - logger: s.recorder.Logger(s.loggerName), + enabled: true, + instrumentName: s.loggerName, + logger: s.recorder.Logger(s.loggerName), } err := s.handler.Handle(tt.entry) @@ -215,10 +256,6 @@ func (s *HandlerTestSuite) normalizeObservedTimestamp(result logtest.Recording) } } -func TestHandlerTestSuite(t *testing.T) { - suite.Run(t, new(HandlerTestSuite)) -} - type TestEntry struct { ctx context.Context level contractslog.Level From 1fea34bbcd1e67794cb6ea2baf5618d927d12b5a Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 4 Jan 2026 23:10:19 +0530 Subject: [PATCH 15/29] move route registration in the end --- foundation/application_builder.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/foundation/application_builder.go b/foundation/application_builder.go index 356bdc4bf..5d25afe36 100644 --- a/foundation/application_builder.go +++ b/foundation/application_builder.go @@ -89,11 +89,6 @@ func (r *ApplicationBuilder) Create() foundation.Application { } } - // Register routes - for _, route := range r.routes { - route() - } - // Register event listeners if len(r.eventToListeners) > 0 { eventFacade := r.app.MakeEvent() @@ -203,6 +198,11 @@ func (r *ApplicationBuilder) Create() foundation.Application { r.callback() } + // Register routes + for _, route := range r.routes { + route() + } + // Boot service providers after all settings r.app.BootServiceProviders() From 794ed71139a9d0ccd7b4269421d646bf8f63d331 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 5 Jan 2026 00:22:14 +0530 Subject: [PATCH 16/29] lazily initialize middleware and transport to work with new application_builder --- telemetry/instrumentation/http/middleware.go | 231 ++++++++++-------- .../instrumentation/http/middleware_test.go | 30 +-- telemetry/instrumentation/http/transport.go | 45 +++- .../instrumentation/http/transport_test.go | 93 +++---- 4 files changed, 221 insertions(+), 178 deletions(-) diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index fd524fede..70dcc5224 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -2,15 +2,18 @@ package http import ( "fmt" + "sync" "time" - "github.com/goravel/framework/contracts/http" - "github.com/goravel/framework/support/color" - "github.com/goravel/framework/telemetry" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" semconv "go.opentelemetry.io/otel/semconv/v1.37.0" + "go.opentelemetry.io/otel/trace" + + "github.com/goravel/framework/contracts/http" + "github.com/goravel/framework/support/color" + "github.com/goravel/framework/telemetry" ) const ( @@ -32,136 +35,168 @@ const ( // transparently passes requests through when telemetry is disabled or not // initialized. func Telemetry(opts ...Option) http.Middleware { - if telemetry.TelemetryFacade == nil { - color.Warningln("[Telemetry] Facade not initialized. HTTP middleware disabled.") - return func(ctx http.Context) { ctx.Request().Next() } + h := &MiddlewareHandler{ + opts: opts, } - if telemetry.ConfigFacade == nil { - color.Warningln("[Telemetry] Config facade not initialized. HTTP middleware disabled.") - return func(ctx http.Context) { ctx.Request().Next() } - } + return h.Handle +} + +type MiddlewareHandler struct { + opts []Option + once sync.Once + disabled bool + + // Telemetry components + tracer trace.Tracer + propagator propagation.TextMapPropagator + durationHist metric.Float64Histogram + requestSizeHist metric.Int64Histogram + responseSizeHist metric.Int64Histogram + + cfg ServerConfig + excludedPaths map[string]bool + excludedMethods map[string]bool +} - var cfg ServerConfig - _ = telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &cfg) +func (r *MiddlewareHandler) Handle(ctx http.Context) { + r.once.Do(r.init) - for _, opt := range opts { - opt(&cfg) + if r.disabled { + ctx.Request().Next() + return } - if !cfg.Enabled { - return func(ctx http.Context) { ctx.Request().Next() } + req := ctx.Request() + + routePath := req.OriginPath() + if routePath == "" { + routePath = req.Path() } - if cfg.SpanNameFormatter == nil { - cfg.SpanNameFormatter = defaultSpanNameFormatter + if r.excludedPaths[routePath] || r.excludedMethods[req.Method()] { + req.Next() + return } - tracer := telemetry.TelemetryFacade.Tracer(instrumentationName) - meter := telemetry.TelemetryFacade.Meter(instrumentationName) - propagator := telemetry.TelemetryFacade.Propagator() + for _, f := range r.cfg.Filters { + if !f(ctx) { + req.Next() + return + } + } - durationHist, _ := meter.Float64Histogram(metricRequestDuration, metric.WithUnit(unitSeconds), metric.WithDescription("Duration of HTTP server requests")) - requestSizeHist, _ := meter.Int64Histogram(metricRequestBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server request body")) - responseSizeHist, _ := meter.Int64Histogram(metricResponseBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server response body")) + start := time.Now() + parentCtx := r.propagator.Extract(ctx.Context(), propagation.HeaderCarrier(req.Headers())) + spanName := r.cfg.SpanNameFormatter(routePath, ctx) - excludedPaths := make(map[string]bool, len(cfg.ExcludedPaths)) - for _, p := range cfg.ExcludedPaths { - excludedPaths[p] = true + scheme := "http" + if proto := req.Header("X-Forwarded-Proto"); proto != "" { + scheme = proto } - excludedMethods := make(map[string]bool, len(cfg.ExcludedMethods)) - for _, m := range cfg.ExcludedMethods { - excludedMethods[m] = true + + baseAttrs := []telemetry.KeyValue{ + semconv.HTTPRequestMethodKey.String(req.Method()), + semconv.HTTPRoute(routePath), + semconv.URLScheme(scheme), + semconv.ServerAddress(req.Host()), + semconv.ClientAddress(req.Ip()), + semconv.UserAgentOriginal(req.Header("User-Agent")), } - return func(ctx http.Context) { - req := ctx.Request() + baseAttrs = append(baseAttrs, r.cfg.MetricAttributes...) - routePath := req.OriginPath() - if routePath == "" { - routePath = req.Path() - } + spanCtx, span := r.tracer.Start(parentCtx, spanName, + telemetry.WithAttributes(baseAttrs...), + telemetry.WithSpanKind(telemetry.SpanKindServer), + ) - if excludedPaths[routePath] || excludedMethods[req.Method()] { - req.Next() - return - } + ctx.WithContext(spanCtx) - for _, f := range cfg.Filters { - if !f(ctx) { - req.Next() - return - } - } + func() { + defer func() { + if rec := recover(); rec != nil { + err := fmt.Errorf("panic: %v", rec) + span.RecordError(err) + span.SetStatus(codes.Error, "Internal Server Error") - start := time.Now() - parentCtx := propagator.Extract(ctx.Context(), propagation.HeaderCarrier(req.Headers())) - spanName := cfg.SpanNameFormatter(routePath, ctx) + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(500))...) - scheme := "http" - if proto := req.Header("X-Forwarded-Proto"); proto != "" { - scheme = proto - } + r.durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + r.requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) + r.responseSizeHist.Record(spanCtx, 0, metricAttrs) - baseAttrs := []telemetry.KeyValue{ - semconv.HTTPRequestMethodKey.String(req.Method()), - semconv.HTTPRoute(routePath), - semconv.URLScheme(scheme), - semconv.ServerAddress(req.Host()), - semconv.ClientAddress(req.Ip()), - semconv.UserAgentOriginal(req.Header("User-Agent")), - } + span.End() + panic(rec) + } + }() + req.Next() + }() - baseAttrs = append(baseAttrs, cfg.MetricAttributes...) + status := ctx.Response().Origin().Status() - spanCtx, span := tracer.Start(parentCtx, spanName, - telemetry.WithAttributes(baseAttrs...), - telemetry.WithSpanKind(telemetry.SpanKindServer), - ) + span.SetAttributes(semconv.HTTPResponseStatusCode(status)) - ctx.WithContext(spanCtx) + if status >= 500 { + span.SetStatus(codes.Error, "") + } else { + span.SetStatus(codes.Ok, "") + } - func() { - defer func() { - if r := recover(); r != nil { - err := fmt.Errorf("panic: %v", r) - span.RecordError(err) - span.SetStatus(codes.Error, "Internal Server Error") + if err := ctx.Err(); err != nil { + span.RecordError(err) + } - metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(500))...) + span.End() - durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) - requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) - responseSizeHist.Record(spanCtx, 0, metricAttrs) + metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) - span.End() - panic(r) - } - }() - req.Next() - }() + r.durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) + r.requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) + r.responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) +} - status := ctx.Response().Origin().Status() +func (r *MiddlewareHandler) init() { + if telemetry.TelemetryFacade == nil || telemetry.ConfigFacade == nil { + color.Warningln("[Telemetry] Facades not initialized. HTTP middleware disabled.") + r.disabled = true + return + } - span.SetAttributes(semconv.HTTPResponseStatusCode(status)) + if err := telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &r.cfg); err != nil { + color.Errorf("[Telemetry] Failed to load HTTP server config: %v. HTTP middleware disabled.", err) + r.disabled = true + return + } - if status >= 500 { - span.SetStatus(codes.Error, "") - } else { - span.SetStatus(codes.Ok, "") - } + for _, opt := range r.opts { + opt(&r.cfg) + } - if err := ctx.Err(); err != nil { - span.RecordError(err) - } + if !r.cfg.Enabled { + r.disabled = true + return + } - span.End() + if r.cfg.SpanNameFormatter == nil { + r.cfg.SpanNameFormatter = defaultSpanNameFormatter + } - metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) + r.tracer = telemetry.TelemetryFacade.Tracer(instrumentationName) + r.propagator = telemetry.TelemetryFacade.Propagator() + meter := telemetry.TelemetryFacade.Meter(instrumentationName) - durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) - requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) - responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) + r.durationHist, _ = meter.Float64Histogram(metricRequestDuration, metric.WithUnit(unitSeconds), metric.WithDescription("Duration of HTTP server requests")) + r.requestSizeHist, _ = meter.Int64Histogram(metricRequestBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server request body")) + r.responseSizeHist, _ = meter.Int64Histogram(metricResponseBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server response body")) + + r.excludedPaths = make(map[string]bool, len(r.cfg.ExcludedPaths)) + for _, p := range r.cfg.ExcludedPaths { + r.excludedPaths[p] = true + } + r.excludedMethods = make(map[string]bool, len(r.cfg.ExcludedMethods)) + for _, m := range r.cfg.ExcludedMethods { + r.excludedMethods[m] = true } } diff --git a/telemetry/instrumentation/http/middleware_test.go b/telemetry/instrumentation/http/middleware_test.go index 3d4e6b34d..aa18acb97 100644 --- a/telemetry/instrumentation/http/middleware_test.go +++ b/telemetry/instrumentation/http/middleware_test.go @@ -132,26 +132,30 @@ func (s *MiddlewareTestSuite) TestTelemetry() { tt.telemetrySetup(mockTelemetry) handler := testMiddleware(tt.handler) - server := httptest.NewServer(handler) - defer server.Close() - - client := &nethttp.Client{} - - action := func() { - _, err := client.Get(server.URL + tt.requestPath) - if !tt.expectPanic { - s.NoError(err) + if tt.expectPanic { + req := httptest.NewRequest("GET", tt.requestPath, nil) + w := httptest.NewRecorder() + s.Panics(func() { + handler.ServeHTTP(w, req) + }) + } else { + server := httptest.NewServer(handler) + defer server.Close() + + client := &nethttp.Client{} + resp, err := client.Get(server.URL + tt.requestPath) + s.NoError(err) + if resp != nil { + s.NoError(resp.Body.Close()) } } - - action() }) } } func testMiddleware(next nethttp.Handler) nethttp.Handler { + mw := Telemetry() return nethttp.HandlerFunc(func(w nethttp.ResponseWriter, r *nethttp.Request) { - mw := Telemetry() ctx := NewTestContext(r.Context(), next, w, r) mw(ctx) }) @@ -261,8 +265,6 @@ func (r *TestResponse) Origin() contractshttp.ResponseOrigin { return r.ctx.writer } -// --- TestResponseWriter --- - type TestResponseWriter struct { nethttp.ResponseWriter status int diff --git a/telemetry/instrumentation/http/transport.go b/telemetry/instrumentation/http/transport.go index bd60720a3..183121f04 100644 --- a/telemetry/instrumentation/http/transport.go +++ b/telemetry/instrumentation/http/transport.go @@ -2,6 +2,7 @@ package http import ( "net/http" + "sync" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -21,21 +22,39 @@ func NewTransport(base http.RoundTripper) http.RoundTripper { base = http.DefaultTransport } - // If telemetry.ConfigFacade is missing or set to false, return the original transport immediately. - // We use "telemetry.instrumentation.http_client" as the standard key for outgoing HTTP. - if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.http_client", true) { - return base + return &TransportProxy{ + base: base, } +} + +type TransportProxy struct { + base http.RoundTripper + otelTransport http.RoundTripper + once sync.Once +} + +func (t *TransportProxy) RoundTrip(req *http.Request) (*http.Response, error) { + t.once.Do(func() { + if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.http_client", true) { + return + } + + if telemetry.TelemetryFacade == nil { + color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") + return + } + + t.otelTransport = otelhttp.NewTransport( + t.base, + otelhttp.WithTracerProvider(telemetry.TelemetryFacade.TracerProvider()), + otelhttp.WithMeterProvider(telemetry.TelemetryFacade.MeterProvider()), + otelhttp.WithPropagators(telemetry.TelemetryFacade.Propagator()), + ) + }) - if telemetry.TelemetryFacade == nil { - color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") - return base + if t.otelTransport != nil { + return t.otelTransport.RoundTrip(req) } - return otelhttp.NewTransport( - base, - otelhttp.WithTracerProvider(telemetry.TelemetryFacade.TracerProvider()), - otelhttp.WithMeterProvider(telemetry.TelemetryFacade.MeterProvider()), - otelhttp.WithPropagators(telemetry.TelemetryFacade.Propagator()), - ) + return t.base.RoundTrip(req) } diff --git a/telemetry/instrumentation/http/transport_test.go b/telemetry/instrumentation/http/transport_test.go index 599d76e87..dff654133 100644 --- a/telemetry/instrumentation/http/transport_test.go +++ b/telemetry/instrumentation/http/transport_test.go @@ -3,8 +3,10 @@ package http import ( "io" "net/http" + "net/http/httptest" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" metricnoop "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/propagation" @@ -38,68 +40,43 @@ func TestTransportTestSuite(t *testing.T) { suite.Run(t, new(TransportTestSuite)) } -func (s *TransportTestSuite) TestNewTransport() { +func (s *TransportTestSuite) TestRoundTrip() { + req := httptest.NewRequest("GET", "http://example.com", nil) + tests := []struct { - name string - setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) - base http.RoundTripper - assert func(res http.RoundTripper) + name string + setup func(*mockstelemetry.Telemetry, *mocksconfig.Config, *MockRoundTripper) }{ { - name: "fallback: returns base when ConfigFacade is nil", - setup: func(_ *mockstelemetry.Telemetry, _ *mocksconfig.Config) { + name: "Fallback: Base used when ConfigFacade is nil", + setup: func(_ *mockstelemetry.Telemetry, _ *mocksconfig.Config, baseMock *MockRoundTripper) { telemetry.ConfigFacade = nil - }, - base: http.DefaultTransport, - assert: func(res http.RoundTripper) { - s.Equal(http.DefaultTransport, res) + baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() }, }, { - name: "kill switch: returns base when http_client is disabled in config", - setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + name: "Kill Switch: Base used when http_client is disabled", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { telemetry.ConfigFacade = mockConfig mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(false).Once() - }, - base: http.DefaultTransport, - assert: func(res http.RoundTripper) { - s.Equal(http.DefaultTransport, res) - }, - }, - { - name: "fallback: returns base when TelemetryFacade is nil (even if config enabled)", - setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = nil - mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() - }, - base: http.DefaultTransport, - assert: func(res http.RoundTripper) { - s.Equal(http.DefaultTransport, res) + baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() }, }, { - name: "success: returns wrapped transport when enabled and facades exist", - setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + name: "Fallback: Base used (with warning) when TelemetryFacade is nil", + setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry + telemetry.TelemetryFacade = nil mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() - mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() - mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() - mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - }, - base: http.DefaultTransport, - assert: func(res http.RoundTripper) { - s.NotNil(res) - s.NotEqual(http.DefaultTransport, res) + baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() }, }, { - name: "success: handles nil base automatically (wraps DefaultTransport)", - setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { + name: "Success: OTel Transport initialized and used", + setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { telemetry.ConfigFacade = mockConfig telemetry.TelemetryFacade = mockTelemetry @@ -107,13 +84,7 @@ func (s *TransportTestSuite) TestNewTransport() { mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - }, - base: nil, - assert: func(res http.RoundTripper) { - // otelhttp.NewTransport(nil) will wrap DefaultTransport. - // So result should be NotNil. - s.NotNil(res) - s.NotEqual(http.DefaultTransport, res) + baseMock.On("RoundTrip", mock.Anything).Return(&http.Response{}, nil).Once() }, }, } @@ -122,15 +93,31 @@ func (s *TransportTestSuite) TestNewTransport() { s.Run(test.name, func() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) mockConfig := mocksconfig.NewConfig(s.T()) + mockBase := &MockRoundTripper{} + + test.setup(mockTelemetry, mockConfig, mockBase) - test.setup(mockTelemetry, mockConfig) + transport := NewTransport(mockBase) - var res http.RoundTripper color.CaptureOutput(func(w io.Writer) { - res = NewTransport(test.base) + _, _ = transport.RoundTrip(req) }) - test.assert(res) + mockTelemetry.AssertExpectations(s.T()) + mockConfig.AssertExpectations(s.T()) + mockBase.AssertExpectations(s.T()) }) } } + +type MockRoundTripper struct { + mock.Mock +} + +func (m *MockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + args := m.Called(req) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*http.Response), args.Error(1) +} From 76021c29c5ec5a89730e1e498450c5ba8bc8b1c4 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 5 Jan 2026 00:29:19 +0530 Subject: [PATCH 17/29] optimise channel test --- telemetry/instrumentation/log/channel_test.go | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index bf90d296f..2c0867d51 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -6,11 +6,10 @@ import ( "time" "github.com/stretchr/testify/suite" - "go.opentelemetry.io/otel/log/noop" + lognoop "go.opentelemetry.io/otel/log/noop" contractslog "github.com/goravel/framework/contracts/log" mocksconfig "github.com/goravel/framework/mocks/config" - mockslog "github.com/goravel/framework/mocks/log" mockstelemetry "github.com/goravel/framework/mocks/telemetry" "github.com/goravel/framework/telemetry" ) @@ -19,7 +18,6 @@ type TelemetryChannelTestSuite struct { suite.Suite mockConfig *mocksconfig.Config mockTelemetry *mockstelemetry.Telemetry - mockEntry *mockslog.Entry } func TestTelemetryChannelTestSuite(t *testing.T) { @@ -29,7 +27,6 @@ func TestTelemetryChannelTestSuite(t *testing.T) { func (s *TelemetryChannelTestSuite) SetupTest() { s.mockConfig = mocksconfig.NewConfig(s.T()) s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) - s.mockEntry = mockslog.NewEntry(s.T()) telemetry.TelemetryFacade = s.mockTelemetry } @@ -95,24 +92,19 @@ func (s *TelemetryChannelTestSuite) TestHandle_Runtime_LazyLoading_TriggersTelem s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(noop.NewLoggerProvider().Logger("test")).Once() + s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(lognoop.NewLoggerProvider().Logger("test")).Once() - s.mockEntry.On("Context").Return(context.Background()) - s.mockEntry.On("Time").Return(time.Now()) - s.mockEntry.On("Message").Return("test message") - s.mockEntry.On("Level").Return(contractslog.InfoLevel) - s.mockEntry.On("Code").Return("") - s.mockEntry.On("Domain").Return("") - s.mockEntry.On("Hint").Return("") - s.mockEntry.On("Owner").Return(nil) - s.mockEntry.On("User").Return(nil) - s.mockEntry.On("With").Return(map[string]any{}) - s.mockEntry.On("Data").Return(map[string]any{}) + entry := &TestEntry{ + ctx: context.Background(), + level: contractslog.LevelInfo, + time: time.Now(), + message: "test message", + } channel := NewTelemetryChannel(s.mockConfig) h, err := channel.Handle(channelPath) s.NoError(err) - s.NoError(h.Handle(s.mockEntry)) + s.NoError(h.Handle(entry)) s.mockTelemetry.AssertExpectations(s.T()) } From eb97e1b88e4dc556151c163389103feffa30158d Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 5 Jan 2026 00:30:32 +0530 Subject: [PATCH 18/29] optimise channel test --- telemetry/instrumentation/log/handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index 821a23547..3913ef54c 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -21,7 +21,7 @@ type HandlerTestSuite struct { loggerName string ctx context.Context now time.Time - mockTelemetry *mockstelemetry.Telemetry // Added for lazy test + mockTelemetry *mockstelemetry.Telemetry } func TestHandlerTestSuite(t *testing.T) { From e4faad6b403bc420fa78574d20e319c6a51ed575 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Sun, 18 Jan 2026 20:31:12 +0530 Subject: [PATCH 19/29] accept telemetry facade as an input instead of using global instance --- telemetry/instrumentation/log/channel.go | 16 ++- telemetry/instrumentation/log/channel_test.go | 123 +++++++----------- telemetry/instrumentation/log/handler.go | 12 +- telemetry/instrumentation/log/handler_test.go | 22 ++-- 4 files changed, 81 insertions(+), 92 deletions(-) diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index d71c54ab5..abc8db014 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -1,29 +1,33 @@ package log import ( - "github.com/goravel/framework/contracts/config" - "github.com/goravel/framework/contracts/log" + contractsconfig "github.com/goravel/framework/contracts/config" + contractslog "github.com/goravel/framework/contracts/log" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" ) const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/log" type TelemetryChannel struct { - config config.Config + config contractsconfig.Config + telemetry contractstelemetry.Telemetry } -func NewTelemetryChannel(config config.Config) *TelemetryChannel { +func NewTelemetryChannel(config contractsconfig.Config, telemetry contractstelemetry.Telemetry) *TelemetryChannel { return &TelemetryChannel{ - config: config, + config: config, + telemetry: telemetry, } } -func (r *TelemetryChannel) Handle(channelPath string) (log.Handler, error) { +func (r *TelemetryChannel) Handle(channelPath string) (contractslog.Handler, error) { if !r.config.GetBool("telemetry.instrumentation.log", true) { return &handler{enabled: false}, nil } instrumentName := r.config.GetString(channelPath+".instrument_name", defaultInstrumentationName) return &handler{ + telemetry: r.telemetry, enabled: true, instrumentName: instrumentName, }, nil diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index 2c0867d51..4f6f8015e 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -1,17 +1,13 @@ package log import ( - "context" "testing" - "time" "github.com/stretchr/testify/suite" - lognoop "go.opentelemetry.io/otel/log/noop" contractslog "github.com/goravel/framework/contracts/log" mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" - "github.com/goravel/framework/telemetry" ) type TelemetryChannelTestSuite struct { @@ -27,84 +23,65 @@ func TestTelemetryChannelTestSuite(t *testing.T) { func (s *TelemetryChannelTestSuite) SetupTest() { s.mockConfig = mocksconfig.NewConfig(s.T()) s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) - - telemetry.TelemetryFacade = s.mockTelemetry -} - -func (s *TelemetryChannelTestSuite) TearDownTest() { - telemetry.TelemetryFacade = nil } -func (s *TelemetryChannelTestSuite) TestHandle_Factory_Success_DefaultName() { +func (s *TelemetryChannelTestSuite) TestHandle() { channelPath := "logging.channels.otel" - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() - s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - - channel := NewTelemetryChannel(s.mockConfig) - h, err := channel.Handle(channelPath) - - s.NoError(err) - s.NotNil(h) - - impl, ok := h.(*handler) - s.True(ok) - s.True(impl.enabled) - s.Equal(defaultInstrumentationName, impl.instrumentName) -} - -func (s *TelemetryChannelTestSuite) TestHandle_Factory_Success_CustomName() { - channelPath := "logging.channels.otel" - customName := "my-service-logs" - - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() - s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(customName).Once() - - channel := NewTelemetryChannel(s.mockConfig) - h, err := channel.Handle(channelPath) - - s.NoError(err) - s.NotNil(h) - - impl, ok := h.(*handler) - s.True(ok) - s.Equal(customName, impl.instrumentName) -} - -func (s *TelemetryChannelTestSuite) TestHandle_Factory_Disabled() { - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(false).Once() - - channel := NewTelemetryChannel(s.mockConfig) - h, err := channel.Handle("logging.channels.otel") + tests := []struct { + name string + configSetup func() + expectedEnabled bool + expectedInstName string + }{ + { + name: "Success: Enabled with default instrumentation name", + configSetup: func() { + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() + s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() + }, + expectedEnabled: true, + expectedInstName: defaultInstrumentationName, + }, + { + name: "Success: Enabled with custom instrumentation name", + configSetup: func() { + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() + s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return("my-custom-logger").Once() + }, + expectedEnabled: true, + expectedInstName: "my-custom-logger", + }, + { + name: "Success: Disabled via config", + configSetup: func() { + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(false).Once() + }, + expectedEnabled: false, + }, + } - s.NoError(err) - s.NotNil(h) + for _, tt := range tests { + s.Run(tt.name, func() { + tt.configSetup() - impl, ok := h.(*handler) - s.True(ok) - s.False(impl.enabled) - s.False(h.Enabled(contractslog.LevelInfo)) -} + channel := NewTelemetryChannel(s.mockConfig, s.mockTelemetry) -func (s *TelemetryChannelTestSuite) TestHandle_Runtime_LazyLoading_TriggersTelemetry() { - channelPath := "logging.channels.otel" + h, err := channel.Handle(channelPath) - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() - s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() + s.NoError(err) + s.NotNil(h) - s.mockTelemetry.On("Logger", defaultInstrumentationName).Return(lognoop.NewLoggerProvider().Logger("test")).Once() + impl, ok := h.(*handler) + s.True(ok, "Handler should be of type *handler") + s.Equal(tt.expectedEnabled, impl.enabled) - entry := &TestEntry{ - ctx: context.Background(), - level: contractslog.LevelInfo, - time: time.Now(), - message: "test message", + if tt.expectedEnabled { + s.Equal(tt.expectedInstName, impl.instrumentName) + s.Equal(s.mockTelemetry, impl.telemetry) + } else { + s.False(h.Enabled(contractslog.LevelInfo)) + } + }) } - - channel := NewTelemetryChannel(s.mockConfig) - h, err := channel.Handle(channelPath) - s.NoError(err) - s.NoError(h.Handle(entry)) - - s.mockTelemetry.AssertExpectations(s.T()) } diff --git a/telemetry/instrumentation/log/handler.go b/telemetry/instrumentation/log/handler.go index cc81217fd..6dc92c07d 100644 --- a/telemetry/instrumentation/log/handler.go +++ b/telemetry/instrumentation/log/handler.go @@ -5,19 +5,21 @@ import ( "sync" "time" - "github.com/goravel/framework/telemetry" otellog "go.opentelemetry.io/otel/log" contractslog "github.com/goravel/framework/contracts/log" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" ) var _ contractslog.Handler = (*handler)(nil) type handler struct { + telemetry contractstelemetry.Telemetry enabled bool instrumentName string - logger otellog.Logger - mu sync.Mutex + + logger otellog.Logger + mu sync.Mutex } func (r *handler) Enabled(level contractslog.Level) bool { @@ -52,8 +54,8 @@ func (r *handler) getLogger() otellog.Logger { return r.logger } - if telemetry.TelemetryFacade != nil { - r.logger = telemetry.TelemetryFacade.Logger(r.instrumentName) + if r.telemetry != nil { + r.logger = r.telemetry.Logger(r.instrumentName) } return r.logger diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index 3913ef54c..8adfebbc7 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -11,7 +11,6 @@ import ( contractslog "github.com/goravel/framework/contracts/log" mockstelemetry "github.com/goravel/framework/mocks/telemetry" - "github.com/goravel/framework/telemetry" ) type HandlerTestSuite struct { @@ -32,9 +31,11 @@ func (s *HandlerTestSuite) SetupTest() { s.loggerName = "test-logger" s.recorder = logtest.NewRecorder() s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) + s.handler = &handler{ enabled: true, instrumentName: s.loggerName, + telemetry: s.mockTelemetry, logger: s.recorder.Logger(s.loggerName), } s.now = time.Date(2024, 1, 1, 10, 0, 0, 0, time.UTC) @@ -44,12 +45,11 @@ func (s *HandlerTestSuite) SetupTest() { } func (s *HandlerTestSuite) TearDownTest() { - telemetry.TelemetryFacade = nil } -func (s *HandlerTestSuite) TestHandle_Lazy_Success() { +func (s *HandlerTestSuite) TestHandle_InitLogger_Success() { s.handler.logger = nil - telemetry.TelemetryFacade = s.mockTelemetry + s.mockTelemetry.On("Logger", s.loggerName).Return(s.recorder.Logger(s.loggerName)).Once() entry := &TestEntry{ @@ -57,22 +57,27 @@ func (s *HandlerTestSuite) TestHandle_Lazy_Success() { level: contractslog.LevelInfo, time: s.now, } + err := s.handler.Handle(entry) + s.NoError(err) s.mockTelemetry.AssertExpectations(s.T()) - s.NotNil(s.handler.logger) + s.NotNil(s.handler.logger, "Logger should have been initialized") } -func (s *HandlerTestSuite) TestHandle_Lazy_FacadeNotSet() { +func (s *HandlerTestSuite) TestHandle_InitLogger_TelemetryNil() { s.handler.logger = nil - telemetry.TelemetryFacade = nil + s.handler.telemetry = nil + entry := &TestEntry{ ctx: context.Background(), level: contractslog.LevelInfo, } + err := s.handler.Handle(entry) + s.NoError(err) - s.Nil(s.handler.logger) + s.Nil(s.handler.logger, "Logger should remain nil if telemetry is missing") } func (s *HandlerTestSuite) TestEnabled() { @@ -230,6 +235,7 @@ func (s *HandlerTestSuite) TestHandle() { s.handler = &handler{ enabled: true, instrumentName: s.loggerName, + telemetry: s.mockTelemetry, logger: s.recorder.Logger(s.loggerName), } From 5141d18b864f5452181a070ae46cfe90c61954c9 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 19 Jan 2026 00:12:05 +0530 Subject: [PATCH 20/29] use a callback to resolve the telemetry facade instance --- contracts/telemetry/telemetry.go | 2 + log/application.go | 37 +++++----- log/service_provider.go | 6 +- telemetry/instrumentation/log/channel.go | 16 ++-- telemetry/instrumentation/log/channel_test.go | 74 ++++++++----------- telemetry/instrumentation/log/handler.go | 12 ++- telemetry/instrumentation/log/handler_test.go | 45 +++++------ 7 files changed, 95 insertions(+), 97 deletions(-) diff --git a/contracts/telemetry/telemetry.go b/contracts/telemetry/telemetry.go index 9fa4e175e..903ae5ba4 100644 --- a/contracts/telemetry/telemetry.go +++ b/contracts/telemetry/telemetry.go @@ -9,6 +9,8 @@ import ( oteltrace "go.opentelemetry.io/otel/trace" ) +type Resolver = func() Telemetry + type Telemetry interface { // Logger returns a log.Logger instance for emitting structured log records under the given instrumentation name. // Optional log.LoggerOption parameters allow customization of logger behavior. diff --git a/log/application.go b/log/application.go index a1cb51f18..30f839450 100644 --- a/log/application.go +++ b/log/application.go @@ -5,6 +5,7 @@ import ( "log/slog" "sync" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" slogmulti "github.com/samber/slog-multi" "github.com/goravel/framework/contracts/config" @@ -20,13 +21,14 @@ var channelToHandlers sync.Map type Application struct { log.Writer - ctx context.Context - channels []string - config config.Config - json foundation.Json + ctx context.Context + channels []string + config config.Config + json foundation.Json + telemetryResolver contractstelemetry.Resolver } -func NewApplication(ctx context.Context, channels []string, config config.Config, json foundation.Json) (*Application, error) { +func NewApplication(ctx context.Context, channels []string, config config.Config, json foundation.Json, resolver contractstelemetry.Resolver) (*Application, error) { var handlers []slog.Handler if len(channels) == 0 && config != nil { @@ -36,7 +38,7 @@ func NewApplication(ctx context.Context, channels []string, config config.Config } for _, channel := range channels { - channelHandlers, err := getHandlers(config, json, channel) + channelHandlers, err := getHandlers(config, json, resolver, channel) if err != nil { return nil, err } @@ -47,11 +49,12 @@ func NewApplication(ctx context.Context, channels []string, config config.Config slogLogger := slog.New(slogmulti.Fanout(handlers...)) return &Application{ - ctx: ctx, - channels: channels, - config: config, - json: json, - Writer: NewWriter(ctx, slogLogger), + ctx: ctx, + channels: channels, + config: config, + json: json, + telemetryResolver: resolver, + Writer: NewWriter(ctx, slogLogger), }, nil } @@ -60,7 +63,7 @@ func (r *Application) WithContext(ctx context.Context) log.Log { ctx = httpCtx.Context() } - app, err := NewApplication(ctx, r.channels, r.config, r.json) + app, err := NewApplication(ctx, r.channels, r.config, r.json, r.telemetryResolver) if err != nil { r.Error(err) @@ -75,7 +78,7 @@ func (r *Application) Channel(channel string) log.Log { return r } - app, err := NewApplication(r.ctx, []string{channel}, r.config, r.json) + app, err := NewApplication(r.ctx, []string{channel}, r.config, r.json, r.telemetryResolver) if err != nil { r.Error(err) @@ -90,7 +93,7 @@ func (r *Application) Stack(channels []string) log.Log { return r } - app, err := NewApplication(r.ctx, channels, r.config, r.json) + app, err := NewApplication(r.ctx, channels, r.config, r.json, r.telemetryResolver) if err != nil { r.Error(err) @@ -101,7 +104,7 @@ func (r *Application) Stack(channels []string) log.Log { } // getHandlers returns slog log handlers for the specified channel. -func getHandlers(config config.Config, json foundation.Json, channel string) ([]slog.Handler, error) { +func getHandlers(config config.Config, json foundation.Json, telemetryResolver contractstelemetry.Resolver, channel string) ([]slog.Handler, error) { var handlers []slog.Handler handlersAny, ok := channelToHandlers.Load(channel) if ok { @@ -123,7 +126,7 @@ func getHandlers(config config.Config, json foundation.Json, channel string) ([] return nil, errors.LogDriverCircularReference.Args("stack") } - channelHandlers, err := getHandlers(config, json, stackChannel) + channelHandlers, err := getHandlers(config, json, telemetryResolver, stackChannel) if err != nil { return nil, err } @@ -156,7 +159,7 @@ func getHandlers(config config.Config, json foundation.Json, channel string) ([] handlers = append(handlers, HandlerToSlogHandler(logger.NewConsoleHandler(config, json, level, formatter))) } case log.DriverOtel: - logLogger := telemetrylog.NewTelemetryChannel(config) + logLogger := telemetrylog.NewLazyTelemetryChannel(config, telemetryResolver) handler, err := logLogger.Handle(channelPath) if err != nil { return nil, err diff --git a/log/service_provider.go b/log/service_provider.go index 2284e4ce4..31b97c832 100644 --- a/log/service_provider.go +++ b/log/service_provider.go @@ -5,6 +5,7 @@ import ( "github.com/goravel/framework/contracts/binding" "github.com/goravel/framework/contracts/foundation" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" ) @@ -32,7 +33,10 @@ func (r *ServiceProvider) Register(app foundation.Application) { if json == nil { return nil, errors.JSONParserNotSet.SetModule(errors.ModuleLog) } - return NewApplication(context.Background(), nil, config, json) + return NewApplication(context.Background(), nil, config, json, func() contractstelemetry.Telemetry { + // todo: check if we can omit the warning published by MakeTelemetry method + return app.MakeTelemetry() + }) }) } diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index abc8db014..27b51e52b 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -9,14 +9,20 @@ import ( const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/log" type TelemetryChannel struct { - config contractsconfig.Config - telemetry contractstelemetry.Telemetry + config contractsconfig.Config + resolver contractstelemetry.Resolver } func NewTelemetryChannel(config contractsconfig.Config, telemetry contractstelemetry.Telemetry) *TelemetryChannel { + return NewLazyTelemetryChannel(config, func() contractstelemetry.Telemetry { + return telemetry + }) +} + +func NewLazyTelemetryChannel(config contractsconfig.Config, resolver contractstelemetry.Resolver) *TelemetryChannel { return &TelemetryChannel{ - config: config, - telemetry: telemetry, + config: config, + resolver: resolver, } } @@ -27,7 +33,7 @@ func (r *TelemetryChannel) Handle(channelPath string) (contractslog.Handler, err instrumentName := r.config.GetString(channelPath+".instrument_name", defaultInstrumentationName) return &handler{ - telemetry: r.telemetry, + resolver: r.resolver, enabled: true, instrumentName: instrumentName, }, nil diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index 4f6f8015e..186cd7c98 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -12,75 +12,65 @@ import ( type TelemetryChannelTestSuite struct { suite.Suite - mockConfig *mocksconfig.Config - mockTelemetry *mockstelemetry.Telemetry } func TestTelemetryChannelTestSuite(t *testing.T) { suite.Run(t, new(TelemetryChannelTestSuite)) } -func (s *TelemetryChannelTestSuite) SetupTest() { - s.mockConfig = mocksconfig.NewConfig(s.T()) - s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) -} - func (s *TelemetryChannelTestSuite) TestHandle() { - channelPath := "logging.channels.otel" + const ( + channelPath = "logging.channels.otel" + telemetryKey = "telemetry.instrumentation.log" + ) tests := []struct { name string - configSetup func() - expectedEnabled bool + setup func(m *mocksconfig.Config) + shouldBeEnabled bool expectedInstName string }{ { - name: "Success: Enabled with default instrumentation name", - configSetup: func() { - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() - s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return(defaultInstrumentationName).Once() - }, - expectedEnabled: true, - expectedInstName: defaultInstrumentationName, - }, - { - name: "Success: Enabled with custom instrumentation name", - configSetup: func() { - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() - s.mockConfig.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName).Return("my-custom-logger").Once() + name: "Success: Telemetry enabled with custom name", + setup: func(m *mocksconfig.Config) { + m.EXPECT().GetBool(telemetryKey, true).Return(true).Once() + m.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName). + Return("custom-app-logger").Once() }, - expectedEnabled: true, - expectedInstName: "my-custom-logger", + shouldBeEnabled: true, + expectedInstName: "custom-app-logger", }, { - name: "Success: Disabled via config", - configSetup: func() { - s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(false).Once() + name: "Success: Telemetry disabled via config", + setup: func(m *mocksconfig.Config) { + m.EXPECT().GetBool(telemetryKey, true).Return(false).Once() }, - expectedEnabled: false, + shouldBeEnabled: false, }, } for _, tt := range tests { s.Run(tt.name, func() { - tt.configSetup() - - channel := NewTelemetryChannel(s.mockConfig, s.mockTelemetry) + mockConfig := mocksconfig.NewConfig(s.T()) + mockTelemetry := mockstelemetry.NewTelemetry(s.T()) - h, err := channel.Handle(channelPath) + tt.setup(mockConfig) + channel := NewTelemetryChannel(mockConfig, mockTelemetry) + channelHandler, err := channel.Handle(channelPath) s.NoError(err) - s.NotNil(h) + s.NotNil(channelHandler) + + s.Equal(tt.shouldBeEnabled, channelHandler.Enabled(contractslog.LevelInfo)) + + if tt.shouldBeEnabled { + impl, ok := channelHandler.(*handler) + s.True(ok, "Returned handler must be of type *handler") - impl, ok := h.(*handler) - s.True(ok, "Handler should be of type *handler") - s.Equal(tt.expectedEnabled, impl.enabled) + s.Equal(tt.expectedInstName, impl.instrumentName, "Instrumentation name should match config") - if tt.expectedEnabled { - s.Equal(tt.expectedInstName, impl.instrumentName) - s.Equal(s.mockTelemetry, impl.telemetry) - } else { - s.False(h.Enabled(contractslog.LevelInfo)) + s.NotNil(impl.resolver, "Handler resolver should not be nil") + s.Equal(mockTelemetry, impl.resolver(), "Resolver should return the injected telemetry service") } }) } diff --git a/telemetry/instrumentation/log/handler.go b/telemetry/instrumentation/log/handler.go index 6dc92c07d..8e6e8a864 100644 --- a/telemetry/instrumentation/log/handler.go +++ b/telemetry/instrumentation/log/handler.go @@ -14,12 +14,12 @@ import ( var _ contractslog.Handler = (*handler)(nil) type handler struct { - telemetry contractstelemetry.Telemetry + resolver contractstelemetry.Resolver // The un-executed function + telemetry contractstelemetry.Telemetry // The cached instance enabled bool instrumentName string - - logger otellog.Logger - mu sync.Mutex + logger otellog.Logger + mu sync.Mutex } func (r *handler) Enabled(level contractslog.Level) bool { @@ -54,6 +54,10 @@ func (r *handler) getLogger() otellog.Logger { return r.logger } + if r.telemetry == nil && r.resolver != nil { + r.telemetry = r.resolver() + } + if r.telemetry != nil { r.logger = r.telemetry.Logger(r.instrumentName) } diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index 8adfebbc7..d1c1521d5 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -10,6 +10,7 @@ import ( "go.opentelemetry.io/otel/log/logtest" contractslog "github.com/goravel/framework/contracts/log" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" mockstelemetry "github.com/goravel/framework/mocks/telemetry" ) @@ -35,8 +36,10 @@ func (s *HandlerTestSuite) SetupTest() { s.handler = &handler{ enabled: true, instrumentName: s.loggerName, - telemetry: s.mockTelemetry, - logger: s.recorder.Logger(s.loggerName), + resolver: func() contractstelemetry.Telemetry { + return s.mockTelemetry + }, + logger: s.recorder.Logger(s.loggerName), } s.now = time.Date(2024, 1, 1, 10, 0, 0, 0, time.UTC) @@ -44,11 +47,9 @@ func (s *HandlerTestSuite) SetupTest() { s.ctx = context.WithValue(context.Background(), ctxKey("request_id"), "req-123") } -func (s *HandlerTestSuite) TearDownTest() { -} - -func (s *HandlerTestSuite) TestHandle_InitLogger_Success() { +func (s *HandlerTestSuite) TestHandle_Lazy_Success() { s.handler.logger = nil + s.handler.telemetry = nil s.mockTelemetry.On("Logger", s.loggerName).Return(s.recorder.Logger(s.loggerName)).Once() @@ -59,15 +60,19 @@ func (s *HandlerTestSuite) TestHandle_InitLogger_Success() { } err := s.handler.Handle(entry) - s.NoError(err) + s.mockTelemetry.AssertExpectations(s.T()) - s.NotNil(s.handler.logger, "Logger should have been initialized") + s.NotNil(s.handler.logger, "Logger should be initialized") + s.NotNil(s.handler.telemetry, "Telemetry instance should be cached") } -func (s *HandlerTestSuite) TestHandle_InitLogger_TelemetryNil() { +func (s *HandlerTestSuite) TestHandle_Lazy_Nil() { s.handler.logger = nil s.handler.telemetry = nil + s.handler.resolver = func() contractstelemetry.Telemetry { + return nil + } entry := &TestEntry{ ctx: context.Background(), @@ -77,7 +82,8 @@ func (s *HandlerTestSuite) TestHandle_InitLogger_TelemetryNil() { err := s.handler.Handle(entry) s.NoError(err) - s.Nil(s.handler.logger, "Logger should remain nil if telemetry is missing") + s.Nil(s.handler.logger, "Logger should remain nil") + s.Nil(s.handler.telemetry, "Telemetry should remain nil") } func (s *HandlerTestSuite) TestEnabled() { @@ -158,7 +164,6 @@ func (s *HandlerTestSuite) TestHandle() { }, data: map[string]any{ "user_id": 42, - "active": true, }, }, expected: logtest.Record{ @@ -170,7 +175,6 @@ func (s *HandlerTestSuite) TestHandle() { Attributes: []log.KeyValue{ log.String("foo", "bar"), log.Int64("user_id", 42), - log.Bool("active", true), }, }, }, @@ -181,14 +185,9 @@ func (s *HandlerTestSuite) TestHandle() { level: contractslog.LevelWarning, time: s.now, user: map[string]any{ - "id": 1, "role": "admin", }, tags: []string{"critical", "auth"}, - request: map[string]any{ - "method": "GET", - "url": "/login", - }, }, expected: logtest.Record{ Context: s.ctx, @@ -198,17 +197,12 @@ func (s *HandlerTestSuite) TestHandle() { Body: log.StringValue(""), Attributes: []log.KeyValue{ log.Map("user", - log.Int64("id", 1), log.String("role", "admin"), ), log.Slice("tags", log.StringValue("critical"), log.StringValue("auth"), ), - log.Map("request", - log.String("method", "GET"), - log.String("url", "/login"), - ), }, }, }, @@ -232,12 +226,7 @@ func (s *HandlerTestSuite) TestHandle() { for _, tt := range tests { s.Run(tt.name, func() { s.recorder = logtest.NewRecorder() - s.handler = &handler{ - enabled: true, - instrumentName: s.loggerName, - telemetry: s.mockTelemetry, - logger: s.recorder.Logger(s.loggerName), - } + s.handler.logger = s.recorder.Logger(s.loggerName) err := s.handler.Handle(tt.entry) s.NoError(err) From 5a2697151f60d560b91aa00c78269f6fc12eedce Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 19 Jan 2026 00:52:06 +0530 Subject: [PATCH 21/29] optimise grpc handler to remove usage of telemetry and config facade --- telemetry/instrumentation/grpc/handler.go | 17 ++- .../instrumentation/grpc/handler_test.go | 126 +++++------------- 2 files changed, 39 insertions(+), 104 deletions(-) diff --git a/telemetry/instrumentation/grpc/handler.go b/telemetry/instrumentation/grpc/handler.go index 746e51493..261c2200e 100644 --- a/telemetry/instrumentation/grpc/handler.go +++ b/telemetry/instrumentation/grpc/handler.go @@ -1,21 +1,21 @@ package grpc import ( + contractsconfig "github.com/goravel/framework/contracts/config" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc/stats" - "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" ) // NewServerStatsHandler creates an OTel stats handler for the server. -func NewServerStatsHandler(opts ...Option) stats.Handler { - if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.grpc_server", true) { +func NewServerStatsHandler(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, opts ...Option) stats.Handler { + if config == nil || !config.GetBool("telemetry.instrumentation.grpc_server", true) { return nil } - if telemetry.TelemetryFacade == nil { - color.Warningln("[Telemetry] Facade not initialized. gRPC server stats instrumentation is disabled.") + if telemetry == nil { return nil } @@ -25,13 +25,12 @@ func NewServerStatsHandler(opts ...Option) stats.Handler { } // NewClientStatsHandler creates an OTel stats handler for the client. -func NewClientStatsHandler(opts ...Option) stats.Handler { - if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.grpc_client", true) { +func NewClientStatsHandler(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, opts ...Option) stats.Handler { + if config == nil || !config.GetBool("telemetry.instrumentation.grpc_client", true) { return nil } - if telemetry.TelemetryFacade == nil { - color.Warningln("[Telemetry] Facade not initialized. gRPC client stats instrumentation is disabled.") + if telemetry == nil { return nil } diff --git a/telemetry/instrumentation/grpc/handler_test.go b/telemetry/instrumentation/grpc/handler_test.go index 2e39643a0..0e7d4b111 100644 --- a/telemetry/instrumentation/grpc/handler_test.go +++ b/telemetry/instrumentation/grpc/handler_test.go @@ -1,7 +1,6 @@ package grpc import ( - "io" "testing" "github.com/stretchr/testify/suite" @@ -10,28 +9,13 @@ import ( tracenoop "go.opentelemetry.io/otel/trace/noop" "google.golang.org/grpc/stats" - contractsconfig "github.com/goravel/framework/contracts/config" - contractstelemetry "github.com/goravel/framework/contracts/telemetry" mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" - "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" ) type HandlerTestSuite struct { suite.Suite - originalTelemetryFacade contractstelemetry.Telemetry - originalConfigFacade contractsconfig.Config -} - -func (s *HandlerTestSuite) SetupTest() { - s.originalTelemetryFacade = telemetry.TelemetryFacade - s.originalConfigFacade = telemetry.ConfigFacade -} - -func (s *HandlerTestSuite) TearDownTest() { - telemetry.TelemetryFacade = s.originalTelemetryFacade - telemetry.ConfigFacade = s.originalConfigFacade } func TestHandlerTestSuite(t *testing.T) { @@ -42,66 +26,48 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { tests := []struct { name string setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) - assert func() + assert func(*mockstelemetry.Telemetry, *mocksconfig.Config) }{ { - name: "returns nil immediately if config is disabled", + name: "Returns nil if config is disabled", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(false).Once() }, - assert: func() { - s.Nil(NewServerStatsHandler()) + assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.Nil(NewServerStatsHandler(c, t)) }, }, { - name: "returns nil and logs warning when config enabled but telemetry facade is nil", + name: "Returns nil (no warning) if telemetry is nil", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = nil - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() }, - assert: func() { - var handler stats.Handler - out := color.CaptureOutput(func(w io.Writer) { - handler = NewServerStatsHandler() - }) - - s.Nil(handler) - s.Contains(out, "[Telemetry] Facade not initialized. gRPC server stats instrumentation is disabled.") + assert: func(_ *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.Nil(NewServerStatsHandler(c, nil)) }, }, { - name: "returns handler when enabled and facade is set", + name: "Returns handler when enabled and dependencies set", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() - mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() }, - assert: func() { - s.NotNil(NewServerStatsHandler()) + assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.NotNil(NewServerStatsHandler(c, t)) }, }, { - name: "accepts options", + name: "Accepts options", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() - mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() }, - assert: func() { - handler := NewServerStatsHandler( + assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { + handler := NewServerStatsHandler(c, t, WithFilter(func(info *stats.RPCTagInfo) bool { return true }), WithMessageEvents(ReceivedEvents, SentEvents), WithMetricAttributes(telemetry.String("key", "value")), @@ -116,8 +82,10 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) mockConfig := mocksconfig.NewConfig(s.T()) - test.setup(mockTelemetry, mockConfig) - test.assert() + if test.setup != nil { + test.setup(mockTelemetry, mockConfig) + } + test.assert(mockTelemetry, mockConfig) }) } } @@ -126,70 +94,36 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { tests := []struct { name string setup func(*mockstelemetry.Telemetry, *mocksconfig.Config) - assert func() + assert func(*mockstelemetry.Telemetry, *mocksconfig.Config) }{ { - name: "returns nil immediately if config is disabled", + name: "Returns nil if config is disabled", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(false).Once() }, - assert: func() { - s.Nil(NewClientStatsHandler()) + assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.Nil(NewClientStatsHandler(c, t)) }, }, { - name: "returns nil and logs warning when telemetry facade is nil", + name: "Returns nil (no warning) if telemetry is nil", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = nil - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() }, - assert: func() { - var handler stats.Handler - out := color.CaptureOutput(func(w io.Writer) { - handler = NewClientStatsHandler() - }) - - s.Nil(handler) - s.Contains(out, "[Telemetry] Facade not initialized. gRPC client stats instrumentation is disabled.") + assert: func(_ *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.Nil(NewClientStatsHandler(c, nil)) }, }, { - name: "returns handler when telemetry facade is set", + name: "Returns handler when dependencies set", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() - mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() }, - assert: func() { - s.NotNil(NewClientStatsHandler()) - }, - }, - { - name: "accepts options", - setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry - - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() - - mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() - mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() - mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - }, - assert: func() { - handler := NewClientStatsHandler( - WithSpanAttributes(), - WithMetricAttributes(), - ) - s.NotNil(handler) + assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { + s.NotNil(NewClientStatsHandler(c, t)) }, }, } @@ -199,8 +133,10 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { mockTelemetry := mockstelemetry.NewTelemetry(s.T()) mockConfig := mocksconfig.NewConfig(s.T()) - test.setup(mockTelemetry, mockConfig) - test.assert() + if test.setup != nil { + test.setup(mockTelemetry, mockConfig) + } + test.assert(mockTelemetry, mockConfig) }) } } From 5a5d9fc7b2e1b7dbf2403947c04b6c492a067684 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 19 Jan 2026 00:58:50 +0530 Subject: [PATCH 22/29] optimise the grpc handler --- telemetry/instrumentation/grpc/handler.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/telemetry/instrumentation/grpc/handler.go b/telemetry/instrumentation/grpc/handler.go index 261c2200e..01fbd30d1 100644 --- a/telemetry/instrumentation/grpc/handler.go +++ b/telemetry/instrumentation/grpc/handler.go @@ -5,8 +5,6 @@ import ( contractstelemetry "github.com/goravel/framework/contracts/telemetry" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc/stats" - - "github.com/goravel/framework/telemetry" ) // NewServerStatsHandler creates an OTel stats handler for the server. @@ -19,7 +17,7 @@ func NewServerStatsHandler(config contractsconfig.Config, telemetry contractstel return nil } - finalOpts := append(getCommonOptions(), opts...) + finalOpts := append(getCommonOptions(telemetry), opts...) return otelgrpc.NewServerHandler(finalOpts...) } @@ -34,16 +32,16 @@ func NewClientStatsHandler(config contractsconfig.Config, telemetry contractstel return nil } - finalOpts := append(getCommonOptions(), opts...) + finalOpts := append(getCommonOptions(telemetry), opts...) return otelgrpc.NewClientHandler(finalOpts...) } -func getCommonOptions() []otelgrpc.Option { +func getCommonOptions(telemetry contractstelemetry.Telemetry) []otelgrpc.Option { return []otelgrpc.Option{ - otelgrpc.WithTracerProvider(telemetry.TelemetryFacade.TracerProvider()), - otelgrpc.WithMeterProvider(telemetry.TelemetryFacade.MeterProvider()), - otelgrpc.WithPropagators(telemetry.TelemetryFacade.Propagator()), + otelgrpc.WithTracerProvider(telemetry.TracerProvider()), + otelgrpc.WithMeterProvider(telemetry.MeterProvider()), + otelgrpc.WithPropagators(telemetry.Propagator()), otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents), } } From 09f47456917dd03ecfee66f98acb5eadc9cd2db3 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Tue, 20 Jan 2026 11:22:17 +0530 Subject: [PATCH 23/29] use telemetry transport if enabled --- http/client/factory.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/http/client/factory.go b/http/client/factory.go index a1f649d6f..cd8379f28 100644 --- a/http/client/factory.go +++ b/http/client/factory.go @@ -7,6 +7,7 @@ import ( "github.com/goravel/framework/contracts/foundation" "github.com/goravel/framework/contracts/http/client" "github.com/goravel/framework/errors" + telemetryhttp "github.com/goravel/framework/telemetry/instrumentation/http" ) var _ client.Factory = (*Factory)(nil) @@ -198,6 +199,10 @@ func (r *Factory) resolveClient(name string, state *FakeState) (*http.Client, er baseTransport.IdleConnTimeout = cfg.IdleConnTimeout var transport http.RoundTripper = baseTransport + if cfg.EnableTelemetry { + transport = telemetryhttp.NewTransport(transport) + } + if state != nil { // If testing mode is active, wrap the real transport with our interceptor. transport = NewFakeTransport(state, baseTransport, r.json) From 9793141f2a211efdad1a95076ebfd387c863f5dd Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Wed, 21 Jan 2026 01:46:31 +0530 Subject: [PATCH 24/29] optimise http auto instrumentation --- http/client/factory.go | 38 ++++-- http/client/factory_test.go | 100 +++++++++++++--- http/service_provider.go | 7 +- telemetry/instrumentation/http/middleware.go | 111 +++++++++--------- .../instrumentation/http/middleware_test.go | 67 ++++++----- telemetry/instrumentation/http/transport.go | 54 +++------ .../instrumentation/http/transport_test.go | 76 +++++------- telemetry/service_provider.go | 9 -- 8 files changed, 248 insertions(+), 214 deletions(-) diff --git a/http/client/factory.go b/http/client/factory.go index cd8379f28..55ff9d03f 100644 --- a/http/client/factory.go +++ b/http/client/factory.go @@ -4,8 +4,10 @@ import ( "net/http" "sync" + contractsconfig "github.com/goravel/framework/contracts/config" "github.com/goravel/framework/contracts/foundation" "github.com/goravel/framework/contracts/http/client" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" telemetryhttp "github.com/goravel/framework/telemetry/instrumentation/http" ) @@ -15,8 +17,11 @@ var _ client.Factory = (*Factory)(nil) type Factory struct { client.Request - json foundation.Json - config *FactoryConfig + factoryConfig *FactoryConfig + config contractsconfig.Config + json foundation.Json + telemetryResolver contractstelemetry.Resolver + clients sync.Map mu sync.RWMutex @@ -25,14 +30,21 @@ type Factory struct { stray []string } -func NewFactory(config *FactoryConfig, json foundation.Json) (*Factory, error) { - if config == nil { +func NewFactory( + factoryConfig *FactoryConfig, + config contractsconfig.Config, + json foundation.Json, + telemetryResolver contractstelemetry.Resolver, +) (*Factory, error) { + if factoryConfig == nil { return nil, errors.HttpClientConfigNotSet } factory := &Factory{ - config: config, - json: json, + factoryConfig: factoryConfig, + config: config, + json: json, + telemetryResolver: telemetryResolver, } if err := factory.bindDefault(); err != nil { @@ -82,7 +94,7 @@ func (r *Factory) AssertSentCount(count int) bool { } func (r *Factory) Client(names ...string) client.Request { - name := r.config.Default + name := r.factoryConfig.Default if len(names) > 0 && names[0] != "" { name = names[0] } @@ -96,7 +108,7 @@ func (r *Factory) Client(names ...string) client.Request { return newRequestWithError(err) } - cfg := r.config.Clients[name] + cfg := r.factoryConfig.Clients[name] return NewRequest(httpClient, r.json, cfg.BaseUrl, name) } @@ -152,7 +164,7 @@ func (r *Factory) Sequence() client.FakeSequence { } func (r *Factory) bindDefault() error { - name := r.config.Default + name := r.factoryConfig.Default c, err := r.resolveClient(name, r.fakeState) if err != nil { return err @@ -160,7 +172,7 @@ func (r *Factory) bindDefault() error { // Bind the default client to the embedded Request implementation // so that methods like Http.Get() use the default configuration. - r.Request = NewRequest(c, r.json, r.config.Clients[name].BaseUrl, name) + r.Request = NewRequest(c, r.json, r.factoryConfig.Clients[name].BaseUrl, name) return nil } @@ -185,7 +197,7 @@ func (r *Factory) resolveClient(name string, state *FakeState) (*http.Client, er return val.(*http.Client), nil } - cfg, ok := r.config.Clients[name] + cfg, ok := r.factoryConfig.Clients[name] if !ok { return nil, errors.HttpClientConnectionNotFound.Args(name) } @@ -199,8 +211,8 @@ func (r *Factory) resolveClient(name string, state *FakeState) (*http.Client, er baseTransport.IdleConnTimeout = cfg.IdleConnTimeout var transport http.RoundTripper = baseTransport - if cfg.EnableTelemetry { - transport = telemetryhttp.NewTransport(transport) + if cfg.EnableTelemetry && r.telemetryResolver != nil { + transport = telemetryhttp.NewTransport(r.config, r.telemetryResolver(), transport) } if state != nil { diff --git a/http/client/factory_test.go b/http/client/factory_test.go index 4737d8013..55cdcdb52 100644 --- a/http/client/factory_test.go +++ b/http/client/factory_test.go @@ -10,18 +10,28 @@ import ( "time" "github.com/stretchr/testify/suite" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + metricnoop "go.opentelemetry.io/otel/metric/noop" + "go.opentelemetry.io/otel/propagation" + tracenoop "go.opentelemetry.io/otel/trace/noop" "github.com/goravel/framework/contracts/foundation" "github.com/goravel/framework/contracts/http/client" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" "github.com/goravel/framework/foundation/json" + mocksconfig "github.com/goravel/framework/mocks/config" + mockstelemetry "github.com/goravel/framework/mocks/telemetry" ) type FactoryTestSuite struct { suite.Suite - json foundation.Json - factory *Factory - config *FactoryConfig + json foundation.Json + factory *Factory + factoryConfig *FactoryConfig + mockConfig *mocksconfig.Config + mockTelemetry *mockstelemetry.Telemetry + mockResolver contractstelemetry.Resolver } func TestFactoryTestSuite(t *testing.T) { @@ -30,7 +40,14 @@ func TestFactoryTestSuite(t *testing.T) { func (s *FactoryTestSuite) SetupTest() { s.json = json.New() - s.config = &FactoryConfig{ + s.mockConfig = mocksconfig.NewConfig(s.T()) + s.mockTelemetry = mockstelemetry.NewTelemetry(s.T()) + + s.mockResolver = func() contractstelemetry.Telemetry { + return s.mockTelemetry + } + + s.factoryConfig = &FactoryConfig{ Default: "main", Clients: map[string]Config{ "main": { @@ -48,7 +65,7 @@ func (s *FactoryTestSuite) SetupTest() { }, } var err error - s.factory, err = NewFactory(s.config, s.json) + s.factory, err = NewFactory(s.factoryConfig, s.mockConfig, s.json, s.mockResolver) s.NoError(err) } @@ -77,8 +94,8 @@ func (s *FactoryTestSuite) TestClient_Resolution() { } func (s *FactoryTestSuite) TestErrorHandling() { - s.Run("handles nil config safely", func() { - f, err := NewFactory(nil, s.json) + s.Run("handles nil factoryConfig safely", func() { + f, err := NewFactory(nil, s.mockConfig, s.json, s.mockResolver) s.Nil(f) s.ErrorIs(err, errors.HttpClientConfigNotSet) }) @@ -93,12 +110,12 @@ func (s *FactoryTestSuite) TestErrorHandling() { s.Contains(err.Error(), "[missing_client]") }) - s.Run("returns lazy error when default is empty in config", func() { + s.Run("returns lazy error when default is empty in factoryConfig", func() { cfg := &FactoryConfig{ Default: "", Clients: map[string]Config{"main": {}}, } - f, err := NewFactory(cfg, s.json) + f, err := NewFactory(cfg, s.mockConfig, s.json, s.mockResolver) s.ErrorIs(err, errors.HttpClientDefaultNotSet) s.Nil(f) }) @@ -113,7 +130,7 @@ func (s *FactoryTestSuite) TestConcurrency() { "new2": {BaseUrl: "https://new2.com", Timeout: 3 * time.Second}, }, } - f, err := NewFactory(cfg, s.json) + f, err := NewFactory(cfg, s.mockConfig, s.json, s.mockResolver) s.NoError(err) var wg sync.WaitGroup @@ -163,7 +180,7 @@ func (s *FactoryTestSuite) TestBaseUrl_Override() { "main": {BaseUrl: "https://wrong-url.com"}, }, } - f, err := NewFactory(cfg, s.json) + f, err := NewFactory(cfg, s.mockConfig, s.json, s.mockResolver) s.NoError(err) s.Run("overrides config base url", func() { @@ -182,7 +199,12 @@ func (s *FactoryTestSuite) TestProxy_HttpMethods() { })) defer server.Close() - f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.json) + f, err := NewFactory( + &FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, + s.mockConfig, + s.json, + s.mockResolver, + ) s.NoError(err) tests := []struct { @@ -229,7 +251,12 @@ func (s *FactoryTestSuite) TestProxy_Headers() { })) defer server.Close() - f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.json) + f, err := NewFactory( + &FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, + s.mockConfig, + s.json, + s.mockResolver, + ) s.NoError(err) s.Run("WithHeader & WithHeaders", func() { @@ -306,7 +333,7 @@ func (s *FactoryTestSuite) TestProxy_QueryParameters() { })) defer server.Close() - f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.json) + f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.mockConfig, s.json, s.mockResolver) s.NoError(err) s.Run("WithQueryParameter", func() { @@ -342,7 +369,7 @@ func (s *FactoryTestSuite) TestProxy_UrlParameters() { })) defer server.Close() - f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.json) + f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.mockConfig, s.json, s.mockResolver) s.NoError(err) s.Run("WithUrlParameter", func() { @@ -371,7 +398,7 @@ func (s *FactoryTestSuite) TestProxy_Cookies() { })) defer server.Close() - f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.json) + f, err := NewFactory(&FactoryConfig{Default: "test", Clients: map[string]Config{"test": {BaseUrl: server.URL}}}, s.mockConfig, s.json, s.mockResolver) s.NoError(err) s.Run("WithCookie", func() { @@ -423,7 +450,7 @@ func (s *FactoryTestSuite) TestRouting_Integration() { "server_b": {BaseUrl: serverB.URL}, }, } - f, err := NewFactory(cfg, s.json) + f, err := NewFactory(cfg, s.mockConfig, s.json, s.mockResolver) s.NoError(err) s.Run("proxy methods hit default server", func() { @@ -443,6 +470,41 @@ func (s *FactoryTestSuite) TestRouting_Integration() { }) } +func (s *FactoryTestSuite) TestTelemetry_Integration() { + factoryConfig := &FactoryConfig{ + Default: "telemetry_client", + Clients: map[string]Config{ + "telemetry_client": { + BaseUrl: "https://example.com", + EnableTelemetry: true, + }, + }, + } + + s.mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true). + Return(true).Once() + + s.mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() + s.mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() + s.mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() + + factory, err := NewFactory(factoryConfig, s.mockConfig, s.json, s.mockResolver) + s.NoError(err) + + req := factory.Client("telemetry_client") + s.NotNil(req) + + concreteReq, ok := req.(*Request) + s.True(ok, "Request should be of type *client.Request") + + httpClient := concreteReq.HttpClient() + s.NotNil(httpClient.Transport) + s.IsType(&otelhttp.Transport{}, httpClient.Transport) + + _, isPlainTransport := httpClient.Transport.(*http.Transport) + s.False(isPlainTransport) +} + func (s *FactoryTestSuite) TestFake_GithubUserProfile() { userMap := map[string]any{ "login": "goravel", @@ -547,7 +609,7 @@ func (s *FactoryTestSuite) TestFactory_AllowStrayRequests() { })) defer server.Close() - s.factory.config.Clients["github"] = Config{BaseUrl: server.URL} + s.factory.factoryConfig.Clients["github"] = Config{BaseUrl: server.URL} s.factory.Reset() s.factory.Fake(nil).PreventStrayRequests().AllowStrayRequests([]string{server.URL + "/*"}) @@ -593,7 +655,7 @@ func (s *FactoryTestSuite) TestIntegration_RealServer() { "local": {BaseUrl: server.URL}, }, } - factory, err := NewFactory(cfg, s.json) + factory, err := NewFactory(cfg, s.mockConfig, s.json, s.mockResolver) s.NoError(err) resp, err := factory.Post("/repos", nil) diff --git a/http/service_provider.go b/http/service_provider.go index 1efae02b8..93baaae52 100644 --- a/http/service_provider.go +++ b/http/service_provider.go @@ -4,6 +4,7 @@ import ( contractsbinding "github.com/goravel/framework/contracts/binding" contractsconsole "github.com/goravel/framework/contracts/console" "github.com/goravel/framework/contracts/foundation" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" "github.com/goravel/framework/http/client" "github.com/goravel/framework/http/console" @@ -40,7 +41,7 @@ func (r *ServiceProvider) Register(app foundation.Application) { return nil, errors.ConfigFacadeNotSet.SetModule(errors.ModuleHttp) } - j := app.GetJson() + j := app.Json() if j == nil { return nil, errors.JSONParserNotSet.SetModule(errors.ModuleHttp) } @@ -50,7 +51,9 @@ func (r *ServiceProvider) Register(app foundation.Application) { return nil, err } - return client.NewFactory(factoryConfig, j) + return client.NewFactory(factoryConfig, configFacade, j, func() contractstelemetry.Telemetry { + return app.MakeTelemetry() + }) }) } diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index 70dcc5224..14b3deeee 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -2,7 +2,6 @@ package http import ( "fmt" - "sync" "time" "go.opentelemetry.io/otel/codes" @@ -11,7 +10,9 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.37.0" "go.opentelemetry.io/otel/trace" + contractsconfig "github.com/goravel/framework/contracts/config" "github.com/goravel/framework/contracts/http" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/support/color" "github.com/goravel/framework/telemetry" ) @@ -34,19 +35,64 @@ const ( // context, records spans and metrics when telemetry is enabled, and otherwise // transparently passes requests through when telemetry is disabled or not // initialized. -func Telemetry(opts ...Option) http.Middleware { +func Telemetry(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, opts ...Option) http.Middleware { + if config == nil || telemetry == nil { + return func(ctx http.Context) { + ctx.Request().Next() + } + } + + var cfg ServerConfig + if err := config.UnmarshalKey("telemetry.instrumentation.http_server", &cfg); err != nil { + color.Warningln("Failed to load http server telemetry instrumentation config:", err) + return func(ctx http.Context) { + ctx.Request().Next() + } + } + + for _, opt := range opts { + opt(&cfg) + } + + if !cfg.Enabled { + return func(ctx http.Context) { + ctx.Request().Next() + } + } + + if cfg.SpanNameFormatter == nil { + cfg.SpanNameFormatter = defaultSpanNameFormatter + } + + meter := telemetry.Meter(instrumentationName) + durationHist, _ := meter.Float64Histogram(metricRequestDuration, metric.WithUnit(unitSeconds), metric.WithDescription("Duration of HTTP server requests")) + requestSizeHist, _ := meter.Int64Histogram(metricRequestBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server request body")) + responseSizeHist, _ := meter.Int64Histogram(metricResponseBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server response body")) + + excludedPaths := make(map[string]bool, len(cfg.ExcludedPaths)) + for _, p := range cfg.ExcludedPaths { + excludedPaths[p] = true + } + excludedMethods := make(map[string]bool, len(cfg.ExcludedMethods)) + for _, m := range cfg.ExcludedMethods { + excludedMethods[m] = true + } + h := &MiddlewareHandler{ - opts: opts, + tracer: telemetry.Tracer(instrumentationName), + propagator: telemetry.Propagator(), + durationHist: durationHist, + requestSizeHist: requestSizeHist, + responseSizeHist: responseSizeHist, + cfg: cfg, + excludedPaths: excludedPaths, + excludedMethods: excludedMethods, } return h.Handle } type MiddlewareHandler struct { - opts []Option - once sync.Once - disabled bool - // Telemetry components tracer trace.Tracer propagator propagation.TextMapPropagator @@ -60,13 +106,6 @@ type MiddlewareHandler struct { } func (r *MiddlewareHandler) Handle(ctx http.Context) { - r.once.Do(r.init) - - if r.disabled { - ctx.Request().Next() - return - } - req := ctx.Request() routePath := req.OriginPath() @@ -156,50 +195,6 @@ func (r *MiddlewareHandler) Handle(ctx http.Context) { r.responseSizeHist.Record(spanCtx, int64(ctx.Response().Origin().Size()), metricAttrs) } -func (r *MiddlewareHandler) init() { - if telemetry.TelemetryFacade == nil || telemetry.ConfigFacade == nil { - color.Warningln("[Telemetry] Facades not initialized. HTTP middleware disabled.") - r.disabled = true - return - } - - if err := telemetry.ConfigFacade.UnmarshalKey("telemetry.instrumentation.http_server", &r.cfg); err != nil { - color.Errorf("[Telemetry] Failed to load HTTP server config: %v. HTTP middleware disabled.", err) - r.disabled = true - return - } - - for _, opt := range r.opts { - opt(&r.cfg) - } - - if !r.cfg.Enabled { - r.disabled = true - return - } - - if r.cfg.SpanNameFormatter == nil { - r.cfg.SpanNameFormatter = defaultSpanNameFormatter - } - - r.tracer = telemetry.TelemetryFacade.Tracer(instrumentationName) - r.propagator = telemetry.TelemetryFacade.Propagator() - meter := telemetry.TelemetryFacade.Meter(instrumentationName) - - r.durationHist, _ = meter.Float64Histogram(metricRequestDuration, metric.WithUnit(unitSeconds), metric.WithDescription("Duration of HTTP server requests")) - r.requestSizeHist, _ = meter.Int64Histogram(metricRequestBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server request body")) - r.responseSizeHist, _ = meter.Int64Histogram(metricResponseBodySize, metric.WithUnit(unitBytes), metric.WithDescription("Size of HTTP server response body")) - - r.excludedPaths = make(map[string]bool, len(r.cfg.ExcludedPaths)) - for _, p := range r.cfg.ExcludedPaths { - r.excludedPaths[p] = true - } - r.excludedMethods = make(map[string]bool, len(r.cfg.ExcludedMethods)) - for _, m := range r.cfg.ExcludedMethods { - r.excludedMethods[m] = true - } -} - func getRequestSize(req http.ContextRequest) int64 { size := req.Origin().ContentLength if size < 0 { diff --git a/telemetry/instrumentation/http/middleware_test.go b/telemetry/instrumentation/http/middleware_test.go index aa18acb97..5409048e6 100644 --- a/telemetry/instrumentation/http/middleware_test.go +++ b/telemetry/instrumentation/http/middleware_test.go @@ -14,24 +14,15 @@ import ( "go.opentelemetry.io/otel/propagation" tracenoop "go.opentelemetry.io/otel/trace/noop" + contractsconfig "github.com/goravel/framework/contracts/config" contractshttp "github.com/goravel/framework/contracts/http" contractstelemetry "github.com/goravel/framework/contracts/telemetry" - configmocks "github.com/goravel/framework/mocks/config" - telemetrymocks "github.com/goravel/framework/mocks/telemetry" - "github.com/goravel/framework/telemetry" + mocksconfig "github.com/goravel/framework/mocks/config" + mockstelemetry "github.com/goravel/framework/mocks/telemetry" ) type MiddlewareTestSuite struct { suite.Suite - originalTelemetry contractstelemetry.Telemetry -} - -func (s *MiddlewareTestSuite) SetupTest() { - s.originalTelemetry = telemetry.TelemetryFacade -} - -func (s *MiddlewareTestSuite) TearDownTest() { - telemetry.TelemetryFacade = s.originalTelemetry } func TestMiddlewareTestSuite(t *testing.T) { @@ -39,7 +30,7 @@ func TestMiddlewareTestSuite(t *testing.T) { } func (s *MiddlewareTestSuite) TestTelemetry() { - defaultTelemetrySetup := func(mockTelemetry *telemetrymocks.Telemetry) { + defaultTelemetrySetup := func(mockTelemetry *mockstelemetry.Telemetry) { mockTelemetry.EXPECT().Tracer(instrumentationName).Return(tracenoop.NewTracerProvider().Tracer("test")).Once() mockTelemetry.EXPECT().Meter(instrumentationName).Return(metricnoop.NewMeterProvider().Meter("test")).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() @@ -47,8 +38,9 @@ func (s *MiddlewareTestSuite) TestTelemetry() { tests := []struct { name string - configSetup func(*configmocks.Config) - telemetrySetup func(*telemetrymocks.Telemetry) + configSetup func(*mocksconfig.Config) + telemetrySetup func(*mockstelemetry.Telemetry) + opts []Option handler nethttp.HandlerFunc requestPath string expectPanic bool @@ -56,7 +48,7 @@ func (s *MiddlewareTestSuite) TestTelemetry() { { name: "Success: Request is traced and metrics recorded", requestPath: "/users", - configSetup: func(mockConfig *configmocks.Config) { + configSetup: func(mockConfig *mocksconfig.Config) { mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). Run(func(_ string, cfg any) { c := cfg.(*ServerConfig) @@ -69,10 +61,31 @@ func (s *MiddlewareTestSuite) TestTelemetry() { _, _ = w.Write([]byte("OK")) }, }, + { + name: "Success: Custom Filter blocks tracing", + requestPath: "/admin", + opts: []Option{ + WithFilter(func(ctx contractshttp.Context) bool { + // Don't trace admin routes + return ctx.Request().OriginPath() != "/admin" + }), + }, + configSetup: func(mockConfig *mocksconfig.Config) { + mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). + Run(func(_ string, cfg any) { + c := cfg.(*ServerConfig) + c.Enabled = true + }).Return(nil).Once() + }, + telemetrySetup: defaultTelemetrySetup, + handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { + w.WriteHeader(nethttp.StatusOK) + }, + }, { name: "Ignored: Excluded path is skipped", requestPath: "/health", - configSetup: func(mockConfig *configmocks.Config) { + configSetup: func(mockConfig *mocksconfig.Config) { mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). Run(func(_ string, cfg interface{}) { c := cfg.(*ServerConfig) @@ -88,14 +101,14 @@ func (s *MiddlewareTestSuite) TestTelemetry() { { name: "Ignored: Disabled via config", requestPath: "/users", - configSetup: func(mockConfig *configmocks.Config) { + configSetup: func(mockConfig *mocksconfig.Config) { mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). Run(func(_ string, cfg interface{}) { c := cfg.(*ServerConfig) c.Enabled = false }).Return(nil).Once() }, - telemetrySetup: func(mockTelemetry *telemetrymocks.Telemetry) { + telemetrySetup: func(mockTelemetry *mockstelemetry.Telemetry) { // If disabled, Tracer/Meter should NOT be initialized }, handler: func(w nethttp.ResponseWriter, r *nethttp.Request) { @@ -106,7 +119,7 @@ func (s *MiddlewareTestSuite) TestTelemetry() { name: "Panic: Metrics recorded as 500 and panic re-thrown", requestPath: "/crash", expectPanic: true, - configSetup: func(mockConfig *configmocks.Config) { + configSetup: func(mockConfig *mocksconfig.Config) { mockConfig.EXPECT().UnmarshalKey("telemetry.instrumentation.http_server", mock.Anything). Run(func(_ string, cfg any) { c := cfg.(*ServerConfig) @@ -122,16 +135,14 @@ func (s *MiddlewareTestSuite) TestTelemetry() { for _, tt := range tests { s.Run(tt.name, func() { - mockConfig := configmocks.NewConfig(s.T()) - mockTelemetry := telemetrymocks.NewTelemetry(s.T()) - - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry + mockConfig := mocksconfig.NewConfig(s.T()) + mockTelemetry := mockstelemetry.NewTelemetry(s.T()) tt.configSetup(mockConfig) tt.telemetrySetup(mockTelemetry) - handler := testMiddleware(tt.handler) + handler := testMiddleware(mockConfig, mockTelemetry, tt.handler, tt.opts...) + if tt.expectPanic { req := httptest.NewRequest("GET", tt.requestPath, nil) w := httptest.NewRecorder() @@ -153,8 +164,8 @@ func (s *MiddlewareTestSuite) TestTelemetry() { } } -func testMiddleware(next nethttp.Handler) nethttp.Handler { - mw := Telemetry() +func testMiddleware(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, next nethttp.Handler, opts ...Option) nethttp.Handler { + mw := Telemetry(config, telemetry, opts...) return nethttp.HandlerFunc(func(w nethttp.ResponseWriter, r *nethttp.Request) { ctx := NewTestContext(r.Context(), next, w, r) mw(ctx) diff --git a/telemetry/instrumentation/http/transport.go b/telemetry/instrumentation/http/transport.go index 183121f04..56c8ee0f3 100644 --- a/telemetry/instrumentation/http/transport.go +++ b/telemetry/instrumentation/http/transport.go @@ -2,59 +2,37 @@ package http import ( "net/http" - "sync" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" - "github.com/goravel/framework/support/color" - "github.com/goravel/framework/telemetry" + contractsconfig "github.com/goravel/framework/contracts/config" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" ) // NewTransport returns an http.RoundTripper instrumented with OpenTelemetry. // It wraps the provided base RoundTripper with otelhttp using the configured // telemetry facade's tracer provider, meter provider, and propagator. // -// If telemetry.TelemetryFacade is nil, a warning is logged and no -// instrumentation is applied. In that case, http.DefaultTransport is returned -// when base is nil; otherwise the provided base RoundTripper is returned. -func NewTransport(base http.RoundTripper) http.RoundTripper { +// If telemetry is nil, no instrumentation is applied. In that case, +// http.DefaultTransport is returned when base is nil; otherwise the provided +// base RoundTripper is returned. +func NewTransport(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, base http.RoundTripper) http.RoundTripper { if base == nil { base = http.DefaultTransport } - return &TransportProxy{ - base: base, + if config == nil || telemetry == nil { + return base } -} - -type TransportProxy struct { - base http.RoundTripper - otelTransport http.RoundTripper - once sync.Once -} - -func (t *TransportProxy) RoundTrip(req *http.Request) (*http.Response, error) { - t.once.Do(func() { - if telemetry.ConfigFacade == nil || !telemetry.ConfigFacade.GetBool("telemetry.instrumentation.http_client", true) { - return - } - - if telemetry.TelemetryFacade == nil { - color.Warningln("[Telemetry] Facade not initialized. HTTP client instrumentation is disabled.") - return - } - - t.otelTransport = otelhttp.NewTransport( - t.base, - otelhttp.WithTracerProvider(telemetry.TelemetryFacade.TracerProvider()), - otelhttp.WithMeterProvider(telemetry.TelemetryFacade.MeterProvider()), - otelhttp.WithPropagators(telemetry.TelemetryFacade.Propagator()), - ) - }) - if t.otelTransport != nil { - return t.otelTransport.RoundTrip(req) + if !config.GetBool("telemetry.instrumentation.http_client", true) { + return base } - return t.base.RoundTrip(req) + return otelhttp.NewTransport( + base, + otelhttp.WithTracerProvider(telemetry.TracerProvider()), + otelhttp.WithMeterProvider(telemetry.MeterProvider()), + otelhttp.WithPropagators(telemetry.Propagator()), + ) } diff --git a/telemetry/instrumentation/http/transport_test.go b/telemetry/instrumentation/http/transport_test.go index dff654133..01c69215e 100644 --- a/telemetry/instrumentation/http/transport_test.go +++ b/telemetry/instrumentation/http/transport_test.go @@ -1,7 +1,6 @@ package http import ( - "io" "net/http" "net/http/httptest" "testing" @@ -16,24 +15,10 @@ import ( contractstelemetry "github.com/goravel/framework/contracts/telemetry" mocksconfig "github.com/goravel/framework/mocks/config" mockstelemetry "github.com/goravel/framework/mocks/telemetry" - "github.com/goravel/framework/support/color" - "github.com/goravel/framework/telemetry" ) type TransportTestSuite struct { suite.Suite - originalTelemetryFacade contractstelemetry.Telemetry - originalConfigFacade contractsconfig.Config -} - -func (s *TransportTestSuite) SetupTest() { - s.originalTelemetryFacade = telemetry.TelemetryFacade - s.originalConfigFacade = telemetry.ConfigFacade -} - -func (s *TransportTestSuite) TearDownTest() { - telemetry.TelemetryFacade = s.originalTelemetryFacade - telemetry.ConfigFacade = s.originalConfigFacade } func TestTransportTestSuite(t *testing.T) { @@ -45,67 +30,64 @@ func (s *TransportTestSuite) TestRoundTrip() { tests := []struct { name string - setup func(*mockstelemetry.Telemetry, *mocksconfig.Config, *MockRoundTripper) + setup func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) }{ { - name: "Fallback: Base used when ConfigFacade is nil", - setup: func(_ *mockstelemetry.Telemetry, _ *mocksconfig.Config, baseMock *MockRoundTripper) { - telemetry.ConfigFacade = nil + name: "Fallback: Base used when Config is nil", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + baseMock := &MockRoundTripper{} baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() + return mockstelemetry.NewTelemetry(t), nil, baseMock }, }, { - name: "Kill Switch: Base used when http_client is disabled", - setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { - telemetry.ConfigFacade = mockConfig - mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(false).Once() - + name: "Fallback: Base used when Telemetry is nil", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + baseMock := &MockRoundTripper{} baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() + + return nil, mocksconfig.NewConfig(t), baseMock }, }, { - name: "Fallback: Base used (with warning) when TelemetryFacade is nil", - setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = nil - - mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() + name: "Kill Switch: Base used when http_client is disabled", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + mockConfig := mocksconfig.NewConfig(t) + mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(false).Once() + baseMock := &MockRoundTripper{} baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() + + return mockstelemetry.NewTelemetry(t), mockConfig, baseMock }, }, { name: "Success: OTel Transport initialized and used", - setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config, baseMock *MockRoundTripper) { - telemetry.ConfigFacade = mockConfig - telemetry.TelemetryFacade = mockTelemetry - + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + mockConfig := mocksconfig.NewConfig(t) mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() + + mockTelemetry := mockstelemetry.NewTelemetry(t) mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() + + baseMock := &MockRoundTripper{} baseMock.On("RoundTrip", mock.Anything).Return(&http.Response{}, nil).Once() + + return mockTelemetry, mockConfig, baseMock }, }, } for _, test := range tests { s.Run(test.name, func() { - mockTelemetry := mockstelemetry.NewTelemetry(s.T()) - mockConfig := mocksconfig.NewConfig(s.T()) - mockBase := &MockRoundTripper{} - - test.setup(mockTelemetry, mockConfig, mockBase) - - transport := NewTransport(mockBase) + telemetry, config, baseMock := test.setup(s.T()) + transport := NewTransport(config, telemetry, baseMock) - color.CaptureOutput(func(w io.Writer) { - _, _ = transport.RoundTrip(req) - }) + _, _ = transport.RoundTrip(req) - mockTelemetry.AssertExpectations(s.T()) - mockConfig.AssertExpectations(s.T()) - mockBase.AssertExpectations(s.T()) + baseMock.AssertExpectations(s.T()) }) } } diff --git a/telemetry/service_provider.go b/telemetry/service_provider.go index 0c811cee7..a8f8fa063 100644 --- a/telemetry/service_provider.go +++ b/telemetry/service_provider.go @@ -2,17 +2,10 @@ package telemetry import ( "github.com/goravel/framework/contracts/binding" - "github.com/goravel/framework/contracts/config" "github.com/goravel/framework/contracts/foundation" - "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" ) -var ( - TelemetryFacade telemetry.Telemetry - ConfigFacade config.Config -) - type ServiceProvider struct { } @@ -43,6 +36,4 @@ func (r *ServiceProvider) Register(app foundation.Application) { } func (r *ServiceProvider) Boot(app foundation.Application) { - TelemetryFacade = app.MakeTelemetry() - ConfigFacade = app.MakeConfig() } From bd0645b3a2d06a9eff94bb8c6c6b836460d71ba4 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Wed, 21 Jan 2026 02:16:44 +0530 Subject: [PATCH 25/29] optimize log test cases --- log/application_test.go | 88 +++++++++++-------- log/service_provider.go | 3 +- log/writer_test.go | 60 ++++++------- telemetry/instrumentation/log/channel.go | 4 +- telemetry/instrumentation/log/channel_test.go | 2 +- 5 files changed, 86 insertions(+), 71 deletions(-) diff --git a/log/application_test.go b/log/application_test.go index 3d05b0d73..da520ecf4 100644 --- a/log/application_test.go +++ b/log/application_test.go @@ -6,10 +6,13 @@ import ( "github.com/stretchr/testify/assert" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" "github.com/goravel/framework/foundation/json" mocksconfig "github.com/goravel/framework/mocks/config" + mockstelemetry "github.com/goravel/framework/mocks/telemetry" "github.com/goravel/framework/support/file" + telemetrylog "github.com/goravel/framework/telemetry/instrumentation/log" ) // clearChannelCache clears all entries from the channel cache @@ -22,7 +25,11 @@ func clearChannelCache() { func TestNewApplication(t *testing.T) { j := json.New() - app, err := NewApplication(context.Background(), nil, nil, j) + telemetryResolver := func() contractstelemetry.Telemetry { + return mockstelemetry.NewTelemetry(t) + } + + app, err := NewApplication(context.Background(), nil, nil, j, telemetryResolver) assert.Nil(t, err) assert.NotNil(t, app) @@ -33,7 +40,7 @@ func TestNewApplication(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test.level").Return("debug").Times(2) // Called for file handler and console handler when print=true mockConfig.EXPECT().GetString("logging.channels.test.formatter", "text").Return("text").Times(2) // Called for file handler and console handler when print=true mockConfig.EXPECT().GetBool("logging.channels.test.print").Return(true).Once() - app, err = NewApplication(context.Background(), nil, mockConfig, j) + app, err = NewApplication(context.Background(), nil, mockConfig, j, telemetryResolver) assert.Nil(t, err) assert.NotNil(t, app) @@ -44,7 +51,7 @@ func TestNewApplication(t *testing.T) { mockConfig.EXPECT().GetString("logging.default").Return("test").Once() mockConfig.EXPECT().GetString("logging.channels.test.driver").Return("test").Once() - app, err = NewApplication(context.Background(), nil, mockConfig, j) + app, err = NewApplication(context.Background(), nil, mockConfig, j, telemetryResolver) assert.EqualError(t, err, errors.LogDriverNotSupported.Args("test").Error()) assert.Nil(t, app) @@ -65,7 +72,7 @@ func TestApplication_Channel(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test.formatter", "text").Return("text").Times(2) // Called for file handler and console handler mockConfig.EXPECT().GetBool("logging.channels.test.print").Return(true).Once() - app, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + app, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, app) assert.NotNil(t, app.Channel("")) @@ -101,7 +108,7 @@ func TestApplication_Stack(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test.level").Return("debug").Times(2) // Called for file handler and console handler mockConfig.EXPECT().GetString("logging.channels.test.formatter", "text").Return("text").Times(2) // Called for file handler and console handler mockConfig.EXPECT().GetBool("logging.channels.test.print").Return(true).Once() - app, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + app, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, app) @@ -137,7 +144,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test-single.formatter", "text").Return("text").Once() mockConfig.EXPECT().GetBool("logging.channels.test-single.print").Return(false).Once() - handlers, err := getHandlers(mockConfig, j, "test-single") + handlers, err := getHandlers(mockConfig, j, nil, "test-single") assert.NoError(t, err) assert.Len(t, handlers, 1) @@ -154,7 +161,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test-single-print.formatter", "text").Return("text").Times(2) mockConfig.EXPECT().GetBool("logging.channels.test-single-print.print").Return(true).Once() - handlers, err := getHandlers(mockConfig, j, "test-single-print") + handlers, err := getHandlers(mockConfig, j, nil, "test-single-print") assert.NoError(t, err) assert.Len(t, handlers, 2) // file handler + console handler @@ -172,7 +179,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetBool("logging.channels.test-daily.print").Return(false).Once() mockConfig.EXPECT().GetInt("logging.channels.test-daily.days").Return(7).Once() - handlers, err := getHandlers(mockConfig, j, "test-daily") + handlers, err := getHandlers(mockConfig, j, nil, "test-daily") assert.NoError(t, err) assert.Len(t, handlers, 1) @@ -189,7 +196,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetBool("logging.channels.test-daily-print.print").Return(true).Once() mockConfig.EXPECT().GetInt("logging.channels.test-daily-print.days").Return(3).Once() - handlers, err := getHandlers(mockConfig, j, "test-daily-print") + handlers, err := getHandlers(mockConfig, j, nil, "test-daily-print") assert.NoError(t, err) assert.Len(t, handlers, 2) @@ -197,37 +204,46 @@ func TestGetHandlers(t *testing.T) { clearChannelCache() }) - t.Run("otel driver", func(t *testing.T) { + t.Run("OTeL driver", func(t *testing.T) { mockConfig := mocksconfig.NewConfig(t) + mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() mockConfig.EXPECT().GetString("logging.channels.test-otel.driver").Return("otel").Once() + mockConfig.EXPECT().GetString("logging.channels.test-otel.instrument_name", telemetrylog.DefaultInstrumentationName). + Return("goravel").Once() + mockConfig.EXPECT().GetBool("logging.channels.test-otel.print").Return(false).Once() - handlers, err := getHandlers(mockConfig, j, "test-otel") - // In unit tests, telemetry facade is not initialized - // so we expect an error or empty handlers depending on implementation - if err != nil { - assert.Error(t, err) - } else { - assert.Len(t, handlers, 0) + mockTelemetry := mockstelemetry.NewTelemetry(t) + resolver := func() contractstelemetry.Telemetry { + return mockTelemetry } - // Cleanup + handlers, err := getHandlers(mockConfig, j, resolver, "test-otel") + assert.NoError(t, err) + assert.Len(t, handlers, 1) + clearChannelCache() }) - t.Run("otel driver with print enabled", func(t *testing.T) { + t.Run("OTeL driver with print enabled", func(t *testing.T) { mockConfig := mocksconfig.NewConfig(t) + mockConfig.EXPECT().GetBool("telemetry.instrumentation.log", true).Return(true).Once() mockConfig.EXPECT().GetString("logging.channels.test-otel-print.driver").Return("otel").Once() - - handlers, err := getHandlers(mockConfig, j, "test-otel-print") - // In unit tests, telemetry facade is not initialized - // so we expect an error or empty handlers depending on implementation - if err != nil { - assert.Error(t, err) - } else { - assert.Len(t, handlers, 0) + mockConfig.EXPECT().GetString("logging.channels.test-otel-print.instrument_name", telemetrylog.DefaultInstrumentationName). + Return("goravel").Once() + mockConfig.EXPECT().GetString("logging.channels.test-otel-print.level").Return("debug").Once() + mockConfig.EXPECT().GetBool("logging.channels.test-otel-print.print").Return(true).Once() + mockConfig.EXPECT().GetString("logging.channels.test-otel-print.formatter", "text"). + Return("text").Once() + + mockTelemetry := mockstelemetry.NewTelemetry(t) + resolver := func() contractstelemetry.Telemetry { + return mockTelemetry } - // Cleanup + handlers, err := getHandlers(mockConfig, j, resolver, "test-otel-print") + assert.NoError(t, err) + assert.Len(t, handlers, 2) + clearChannelCache() }) @@ -250,7 +266,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.stack-single2.formatter", "text").Return("text").Once() mockConfig.EXPECT().GetBool("logging.channels.stack-single2.print").Return(false).Once() - handlers, err := getHandlers(mockConfig, j, "test-stack") + handlers, err := getHandlers(mockConfig, j, nil, "test-stack") assert.NoError(t, err) assert.Len(t, handlers, 2) // One from each channel @@ -265,7 +281,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test-circular.driver").Return("stack").Once() mockConfig.EXPECT().Get("logging.channels.test-circular.channels").Return([]string{"test-circular"}).Once() - handlers, err := getHandlers(mockConfig, j, "test-circular") + handlers, err := getHandlers(mockConfig, j, nil, "test-circular") assert.Error(t, err) assert.EqualError(t, err, errors.LogDriverCircularReference.Args("stack").Error()) assert.Nil(t, handlers) @@ -279,7 +295,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test-badstack.driver").Return("stack").Once() mockConfig.EXPECT().Get("logging.channels.test-badstack.channels").Return("not-a-slice").Once() - handlers, err := getHandlers(mockConfig, j, "test-badstack") + handlers, err := getHandlers(mockConfig, j, nil, "test-badstack") assert.Error(t, err) assert.EqualError(t, err, errors.LogChannelNotFound.Args("test-badstack").Error()) assert.Nil(t, handlers) @@ -295,7 +311,7 @@ func TestGetHandlers(t *testing.T) { customLogger := &CustomLogger{} mockConfig.EXPECT().Get("logging.channels.test-custom.via").Return(customLogger).Once() - handlers, err := getHandlers(mockConfig, j, "test-custom") + handlers, err := getHandlers(mockConfig, j, nil, "test-custom") assert.NoError(t, err) assert.Len(t, handlers, 1) @@ -308,7 +324,7 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.test-badcustom.driver").Return("custom").Once() mockConfig.EXPECT().Get("logging.channels.test-badcustom.via").Return(nil).Once() - handlers, err := getHandlers(mockConfig, j, "test-badcustom") + handlers, err := getHandlers(mockConfig, j, nil, "test-badcustom") assert.Error(t, err) assert.EqualError(t, err, errors.LogChannelUnimplemented.Args("test-badcustom").Error()) assert.Nil(t, handlers) @@ -321,7 +337,7 @@ func TestGetHandlers(t *testing.T) { mockConfig := mocksconfig.NewConfig(t) mockConfig.EXPECT().GetString("logging.channels.test-unknown.driver").Return("unknown").Once() - handlers, err := getHandlers(mockConfig, j, "test-unknown") + handlers, err := getHandlers(mockConfig, j, nil, "test-unknown") assert.Error(t, err) assert.EqualError(t, err, errors.LogDriverNotSupported.Args("test-unknown").Error()) assert.Nil(t, handlers) @@ -339,12 +355,12 @@ func TestGetHandlers(t *testing.T) { mockConfig.EXPECT().GetBool("logging.channels.test-cached.print").Return(false).Once() // First call should use mock config - handlers1, err := getHandlers(mockConfig, j, "test-cached") + handlers1, err := getHandlers(mockConfig, j, nil, "test-cached") assert.NoError(t, err) assert.Len(t, handlers1, 1) // Second call should use cached handlers (no mock expectations needed) - handlers2, err := getHandlers(mockConfig, j, "test-cached") + handlers2, err := getHandlers(mockConfig, j, nil, "test-cached") assert.NoError(t, err) assert.Len(t, handlers2, 1) assert.Equal(t, handlers1, handlers2) diff --git a/log/service_provider.go b/log/service_provider.go index 31b97c832..086144b69 100644 --- a/log/service_provider.go +++ b/log/service_provider.go @@ -29,12 +29,11 @@ func (r *ServiceProvider) Register(app foundation.Application) { return nil, errors.ConfigFacadeNotSet.SetModule(errors.ModuleLog) } - json := app.GetJson() + json := app.Json() if json == nil { return nil, errors.JSONParserNotSet.SetModule(errors.ModuleLog) } return NewApplication(context.Background(), nil, config, json, func() contractstelemetry.Telemetry { - // todo: check if we can omit the warning published by MakeTelemetry method return app.MakeTelemetry() }) }) diff --git a/log/writer_test.go b/log/writer_test.go index a6a1db50a..24b77908c 100644 --- a/log/writer_test.go +++ b/log/writer_test.go @@ -54,7 +54,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Debug("Debug Goravel") }, assert: func() { @@ -67,7 +67,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Debugf("Goravel: %s", "World") }, assert: func() { @@ -80,7 +80,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Info("Goravel") }, assert: func() { @@ -93,7 +93,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Infof("Goravel: %s", "World") }, assert: func() { @@ -106,7 +106,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Warning("Goravel") }, assert: func() { @@ -119,7 +119,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Warningf("Goravel: %s", "World") }, assert: func() { @@ -132,7 +132,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Error("Goravel") }, assert: func() { @@ -145,7 +145,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Errorf("Goravel: %s", "World") }, assert: func() { @@ -158,7 +158,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) }, assert: func() { assert.Panics(t, func() { @@ -173,7 +173,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) }, assert: func() { assert.Panics(t, func() { @@ -188,7 +188,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Code("code").Info("Goravel") }, assert: func() { @@ -201,7 +201,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Hint("hint").Info("Goravel") }, assert: func() { @@ -214,7 +214,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.In("domain").Info("Goravel") }, assert: func() { @@ -227,7 +227,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Owner("team@goravel.dev").Info("Goravel") }, assert: func() { @@ -240,7 +240,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Request(&TestRequest{}).Info("Goravel") }, assert: func() { @@ -265,7 +265,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Response(&TestResponse{}).Info("Goravel") }, assert: func() { @@ -289,7 +289,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Tags("tag").Info("Goravel") }, assert: func() { @@ -302,7 +302,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.User(map[string]any{"name": "kkumar-gcc"}).Info("Goravel") }, assert: func() { @@ -315,7 +315,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.With(map[string]any{"key": "value"}).Info("Goravel") }, assert: func() { @@ -328,7 +328,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.WithTrace().Info("Goravel") }, assert: func() { @@ -341,7 +341,7 @@ func TestWriter(t *testing.T) { setup: func() { mockConfig.EXPECT().GetString("app.env").Return("test").Times(4) - log, err = NewApplication(context.Background(), nil, mockConfig, j) + log, err = NewApplication(context.Background(), nil, mockConfig, j, nil) log.Error("test error") log.Info("test info") }, @@ -392,7 +392,7 @@ func TestWriter_WithContext(t *testing.T) { // app.env is called twice per log write (once for each handler: single + daily) mockConfig.EXPECT().GetString("app.env").Return("test").Twice() - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) ctx := context.Background() @@ -426,7 +426,7 @@ func TestWriter_LevelNotMatch(t *testing.T) { mockConfig.EXPECT().GetString("logging.channels.single.level").Return("info").Once() mockConfig.EXPECT().GetString("logging.channels.single.formatter", "text").Return("text").Once() - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) log.Debug("No Debug Goravel") @@ -444,7 +444,7 @@ func TestWriter_DailyLogWithDifferentDays(t *testing.T) { // We log twice in this test, so 2 * 2 = 4 calls mockConfig.EXPECT().GetString("app.env").Return("test").Times(4) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) @@ -482,7 +482,7 @@ func TestWriterWithCustomLogger(t *testing.T) { filename := "custom.log" - logger, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + logger, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, logger) @@ -507,7 +507,7 @@ func TestWriter_Fatal(t *testing.T) { clearChannelCache() mockConfig := initMockConfig(t) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) @@ -532,7 +532,7 @@ func TestWriter_Fatalf(t *testing.T) { clearChannelCache() mockConfig := initMockConfig(t) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) @@ -574,7 +574,7 @@ func TestWriter_ConcurrentAccess(t *testing.T) { // Mock app.env for all log entries (goroutines * iterations) mockConfig.EXPECT().GetString("app.env").Return("test").Times(goroutines * iterations) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) @@ -666,7 +666,7 @@ func TestWriter_NoEntryContamination(t *testing.T) { mockConfig := initMockConfig(t) mockConfig.EXPECT().GetString("app.env").Return("test").Times(2) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) @@ -689,7 +689,7 @@ func TestWriter_FluentChainIsolation(t *testing.T) { mockConfig := initMockConfig(t) mockConfig.EXPECT().GetString("app.env").Return("test").Times(4) - log, err := NewApplication(context.Background(), nil, mockConfig, json.New()) + log, err := NewApplication(context.Background(), nil, mockConfig, json.New(), nil) assert.Nil(t, err) assert.NotNil(t, log) diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index 27b51e52b..815eb6f80 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -6,7 +6,7 @@ import ( contractstelemetry "github.com/goravel/framework/contracts/telemetry" ) -const defaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/log" +const DefaultInstrumentationName = "github.com/goravel/framework/telemetry/instrumentation/log" type TelemetryChannel struct { config contractsconfig.Config @@ -31,7 +31,7 @@ func (r *TelemetryChannel) Handle(channelPath string) (contractslog.Handler, err return &handler{enabled: false}, nil } - instrumentName := r.config.GetString(channelPath+".instrument_name", defaultInstrumentationName) + instrumentName := r.config.GetString(channelPath+".instrument_name", DefaultInstrumentationName) return &handler{ resolver: r.resolver, enabled: true, diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index 186cd7c98..72f0bf643 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -34,7 +34,7 @@ func (s *TelemetryChannelTestSuite) TestHandle() { name: "Success: Telemetry enabled with custom name", setup: func(m *mocksconfig.Config) { m.EXPECT().GetBool(telemetryKey, true).Return(true).Once() - m.EXPECT().GetString(channelPath+".instrument_name", defaultInstrumentationName). + m.EXPECT().GetString(channelPath+".instrument_name", DefaultInstrumentationName). Return("custom-app-logger").Once() }, shouldBeEnabled: true, From f7e34b8ad798275c695ad367488948bf7279aa0a Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Thu, 22 Jan 2026 00:19:24 +0530 Subject: [PATCH 26/29] optimise --- telemetry/instrumentation/http/middleware.go | 6 +----- telemetry/setup/stubs.go | 11 +++++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/telemetry/instrumentation/http/middleware.go b/telemetry/instrumentation/http/middleware.go index 14b3deeee..25ea9cfd9 100644 --- a/telemetry/instrumentation/http/middleware.go +++ b/telemetry/instrumentation/http/middleware.go @@ -149,6 +149,7 @@ func (r *MiddlewareHandler) Handle(ctx http.Context) { telemetry.WithAttributes(baseAttrs...), telemetry.WithSpanKind(telemetry.SpanKindServer), ) + defer span.End() ctx.WithContext(spanCtx) @@ -165,7 +166,6 @@ func (r *MiddlewareHandler) Handle(ctx http.Context) { r.requestSizeHist.Record(spanCtx, getRequestSize(req), metricAttrs) r.responseSizeHist.Record(spanCtx, 0, metricAttrs) - span.End() panic(rec) } }() @@ -178,16 +178,12 @@ func (r *MiddlewareHandler) Handle(ctx http.Context) { if status >= 500 { span.SetStatus(codes.Error, "") - } else { - span.SetStatus(codes.Ok, "") } if err := ctx.Err(); err != nil { span.RecordError(err) } - span.End() - metricAttrs := metric.WithAttributes(append(baseAttrs, semconv.HTTPResponseStatusCode(status))...) r.durationHist.Record(spanCtx, time.Since(start).Seconds(), metricAttrs) diff --git a/telemetry/setup/stubs.go b/telemetry/setup/stubs.go index 1ee3169d6..ac2e84193 100644 --- a/telemetry/setup/stubs.go +++ b/telemetry/setup/stubs.go @@ -146,10 +146,13 @@ func init() { // HTTP Client Instrumentation // - // Configures the instrumentation for outgoing HTTP requests made via - // the application's HTTP client Facade. This acts as a global kill switch - // for all clients. To disable telemetry for specific clients, configure - // "enable_telemetry" within the "http.clients.{client_name}" property. + // Configures instrumentation for outgoing HTTP requests made through the + // application's HTTP client facade. This acts as a global kill switch for + // HTTP client telemetry across all clients. + // + // To disable telemetry for a specific client, set + // "http.clients.{client_name}.enable_telemetry" to false for the + // corresponding client configuration. "http_client": map[string]any{ "enabled": config.Env("OTEL_HTTP_CLIENT_ENABLED", true), }, From 777ce250e2a25782b09ee3bdaf1bdb29ee7cc903 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Thu, 22 Jan 2026 01:05:40 +0530 Subject: [PATCH 27/29] optimise --- telemetry/instrumentation/log/channel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index 815eb6f80..127920145 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -27,7 +27,7 @@ func NewLazyTelemetryChannel(config contractsconfig.Config, resolver contractste } func (r *TelemetryChannel) Handle(channelPath string) (contractslog.Handler, error) { - if !r.config.GetBool("telemetry.instrumentation.log", true) { + if r.config == nil || !r.config.GetBool("telemetry.instrumentation.log", true) { return &handler{enabled: false}, nil } From 43a7d8f46f78469408118722e9db18bd32d9b4c3 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 26 Jan 2026 23:35:36 +0530 Subject: [PATCH 28/29] optimise --- log/application.go | 2 +- telemetry/instrumentation/grpc/handler.go | 9 +-- .../instrumentation/grpc/handler_test.go | 14 ++-- telemetry/instrumentation/http/config.go | 12 ++-- .../instrumentation/http/transport_test.go | 72 +++++++------------ telemetry/instrumentation/log/channel.go | 2 +- telemetry/instrumentation/log/channel_test.go | 4 +- telemetry/instrumentation/log/handler_test.go | 2 +- telemetry/setup/stubs.go | 6 +- 9 files changed, 50 insertions(+), 73 deletions(-) diff --git a/log/application.go b/log/application.go index 30f839450..3ccfac066 100644 --- a/log/application.go +++ b/log/application.go @@ -5,13 +5,13 @@ import ( "log/slog" "sync" - contractstelemetry "github.com/goravel/framework/contracts/telemetry" slogmulti "github.com/samber/slog-multi" "github.com/goravel/framework/contracts/config" "github.com/goravel/framework/contracts/foundation" "github.com/goravel/framework/contracts/http" "github.com/goravel/framework/contracts/log" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" "github.com/goravel/framework/errors" "github.com/goravel/framework/log/logger" telemetrylog "github.com/goravel/framework/telemetry/instrumentation/log" diff --git a/telemetry/instrumentation/grpc/handler.go b/telemetry/instrumentation/grpc/handler.go index 01fbd30d1..f7667a849 100644 --- a/telemetry/instrumentation/grpc/handler.go +++ b/telemetry/instrumentation/grpc/handler.go @@ -1,15 +1,16 @@ package grpc import ( - contractsconfig "github.com/goravel/framework/contracts/config" - contractstelemetry "github.com/goravel/framework/contracts/telemetry" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc/stats" + + contractsconfig "github.com/goravel/framework/contracts/config" + contractstelemetry "github.com/goravel/framework/contracts/telemetry" ) // NewServerStatsHandler creates an OTel stats handler for the server. func NewServerStatsHandler(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, opts ...Option) stats.Handler { - if config == nil || !config.GetBool("telemetry.instrumentation.grpc_server", true) { + if config == nil || !config.GetBool("telemetry.instrumentation.grpc_server") { return nil } @@ -24,7 +25,7 @@ func NewServerStatsHandler(config contractsconfig.Config, telemetry contractstel // NewClientStatsHandler creates an OTel stats handler for the client. func NewClientStatsHandler(config contractsconfig.Config, telemetry contractstelemetry.Telemetry, opts ...Option) stats.Handler { - if config == nil || !config.GetBool("telemetry.instrumentation.grpc_client", true) { + if config == nil || !config.GetBool("telemetry.instrumentation.grpc_client") { return nil } diff --git a/telemetry/instrumentation/grpc/handler_test.go b/telemetry/instrumentation/grpc/handler_test.go index 0e7d4b111..d5db1f961 100644 --- a/telemetry/instrumentation/grpc/handler_test.go +++ b/telemetry/instrumentation/grpc/handler_test.go @@ -31,7 +31,7 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { { name: "Returns nil if config is disabled", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(false).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server").Return(false).Once() }, assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { s.Nil(NewServerStatsHandler(c, t)) @@ -40,7 +40,7 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { { name: "Returns nil (no warning) if telemetry is nil", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server").Return(true).Once() }, assert: func(_ *mockstelemetry.Telemetry, c *mocksconfig.Config) { s.Nil(NewServerStatsHandler(c, nil)) @@ -49,7 +49,7 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { { name: "Returns handler when enabled and dependencies set", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server").Return(true).Once() mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() @@ -61,7 +61,7 @@ func (s *HandlerTestSuite) TestServerStatsHandler() { { name: "Accepts options", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server", true).Return(true).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_server").Return(true).Once() mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() @@ -99,7 +99,7 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { { name: "Returns nil if config is disabled", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(false).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client").Return(false).Once() }, assert: func(t *mockstelemetry.Telemetry, c *mocksconfig.Config) { s.Nil(NewClientStatsHandler(c, t)) @@ -108,7 +108,7 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { { name: "Returns nil (no warning) if telemetry is nil", setup: func(_ *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client").Return(true).Once() }, assert: func(_ *mockstelemetry.Telemetry, c *mocksconfig.Config) { s.Nil(NewClientStatsHandler(c, nil)) @@ -117,7 +117,7 @@ func (s *HandlerTestSuite) TestClientStatsHandler() { { name: "Returns handler when dependencies set", setup: func(mockTelemetry *mockstelemetry.Telemetry, mockConfig *mocksconfig.Config) { - mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client", true).Return(true).Once() + mockConfig.EXPECT().GetBool("telemetry.instrumentation.grpc_client").Return(true).Once() mockTelemetry.EXPECT().TracerProvider().Return(tracenoop.NewTracerProvider()).Once() mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() diff --git a/telemetry/instrumentation/http/config.go b/telemetry/instrumentation/http/config.go index 991800a7f..3b2ad0fc2 100644 --- a/telemetry/instrumentation/http/config.go +++ b/telemetry/instrumentation/http/config.go @@ -19,12 +19,12 @@ type Option func(*ServerConfig) // ServerConfig maps to "telemetry.instrumentation.http_server". type ServerConfig struct { - Enabled bool `mapstructure:"enabled"` - ExcludedPaths []string `mapstructure:"excluded_paths"` - ExcludedMethods []string `mapstructure:"excluded_methods"` - Filters []Filter `mapstructure:"-"` - SpanNameFormatter SpanNameFormatter `mapstructure:"-"` - MetricAttributes []attribute.KeyValue `mapstructure:"-"` + Enabled bool `json:"enabled"` + ExcludedPaths []string `json:"excluded_paths"` + ExcludedMethods []string `json:"excluded_methods"` + Filters []Filter `json:"-"` + SpanNameFormatter SpanNameFormatter `json:"-"` + MetricAttributes []attribute.KeyValue `json:"-"` } func WithFilter(f Filter) Option { diff --git a/telemetry/instrumentation/http/transport_test.go b/telemetry/instrumentation/http/transport_test.go index 01c69215e..8bb9b8014 100644 --- a/telemetry/instrumentation/http/transport_test.go +++ b/telemetry/instrumentation/http/transport_test.go @@ -2,10 +2,8 @@ package http import ( "net/http" - "net/http/httptest" "testing" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" metricnoop "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/propagation" @@ -25,45 +23,40 @@ func TestTransportTestSuite(t *testing.T) { suite.Run(t, new(TransportTestSuite)) } -func (s *TransportTestSuite) TestRoundTrip() { - req := httptest.NewRequest("GET", "http://example.com", nil) +func (s *TransportTestSuite) TestNewTransport() { + baseTransport := &http.Transport{} tests := []struct { - name string - setup func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) + name string + setup func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config) + expectWrapped bool }{ { - name: "Fallback: Base used when Config is nil", - setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { - baseMock := &MockRoundTripper{} - baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() - return mockstelemetry.NewTelemetry(t), nil, baseMock + name: "Fallback: Returns base when Config is nil", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config) { + return mockstelemetry.NewTelemetry(t), nil }, + expectWrapped: false, }, { - name: "Fallback: Base used when Telemetry is nil", - setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { - baseMock := &MockRoundTripper{} - baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() - - return nil, mocksconfig.NewConfig(t), baseMock + name: "Fallback: Returns base when Telemetry is nil", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config) { + return nil, mocksconfig.NewConfig(t) }, + expectWrapped: false, }, { - name: "Kill Switch: Base used when http_client is disabled", - setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + name: "Kill Switch: Returns base when http_client is disabled", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config) { mockConfig := mocksconfig.NewConfig(t) mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(false).Once() - - baseMock := &MockRoundTripper{} - baseMock.On("RoundTrip", req).Return(&http.Response{}, nil).Once() - - return mockstelemetry.NewTelemetry(t), mockConfig, baseMock + return mockstelemetry.NewTelemetry(t), mockConfig }, + expectWrapped: false, }, { - name: "Success: OTel Transport initialized and used", - setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config, *MockRoundTripper) { + name: "Success: Returns wrapped transport when enabled", + setup: func(t *testing.T) (contractstelemetry.Telemetry, contractsconfig.Config) { mockConfig := mocksconfig.NewConfig(t) mockConfig.EXPECT().GetBool("telemetry.instrumentation.http_client", true).Return(true).Once() @@ -72,34 +65,17 @@ func (s *TransportTestSuite) TestRoundTrip() { mockTelemetry.EXPECT().MeterProvider().Return(metricnoop.NewMeterProvider()).Once() mockTelemetry.EXPECT().Propagator().Return(propagation.NewCompositeTextMapPropagator()).Once() - baseMock := &MockRoundTripper{} - baseMock.On("RoundTrip", mock.Anything).Return(&http.Response{}, nil).Once() - - return mockTelemetry, mockConfig, baseMock + return mockTelemetry, mockConfig }, + expectWrapped: true, }, } for _, test := range tests { s.Run(test.name, func() { - telemetry, config, baseMock := test.setup(s.T()) - transport := NewTransport(config, telemetry, baseMock) - - _, _ = transport.RoundTrip(req) - - baseMock.AssertExpectations(s.T()) + telemetry, config := test.setup(s.T()) + result := NewTransport(config, telemetry, baseTransport) + s.Equal(test.expectWrapped, baseTransport != result, "Transport wrapping mismatch") }) } } - -type MockRoundTripper struct { - mock.Mock -} - -func (m *MockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - args := m.Called(req) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*http.Response), args.Error(1) -} diff --git a/telemetry/instrumentation/log/channel.go b/telemetry/instrumentation/log/channel.go index 127920145..a90593be3 100644 --- a/telemetry/instrumentation/log/channel.go +++ b/telemetry/instrumentation/log/channel.go @@ -27,7 +27,7 @@ func NewLazyTelemetryChannel(config contractsconfig.Config, resolver contractste } func (r *TelemetryChannel) Handle(channelPath string) (contractslog.Handler, error) { - if r.config == nil || !r.config.GetBool("telemetry.instrumentation.log", true) { + if r.config == nil || !r.config.GetBool("telemetry.instrumentation.log", false) { return &handler{enabled: false}, nil } diff --git a/telemetry/instrumentation/log/channel_test.go b/telemetry/instrumentation/log/channel_test.go index 72f0bf643..87da40d43 100644 --- a/telemetry/instrumentation/log/channel_test.go +++ b/telemetry/instrumentation/log/channel_test.go @@ -33,7 +33,7 @@ func (s *TelemetryChannelTestSuite) TestHandle() { { name: "Success: Telemetry enabled with custom name", setup: func(m *mocksconfig.Config) { - m.EXPECT().GetBool(telemetryKey, true).Return(true).Once() + m.EXPECT().GetBool(telemetryKey, false).Return(true).Once() m.EXPECT().GetString(channelPath+".instrument_name", DefaultInstrumentationName). Return("custom-app-logger").Once() }, @@ -43,7 +43,7 @@ func (s *TelemetryChannelTestSuite) TestHandle() { { name: "Success: Telemetry disabled via config", setup: func(m *mocksconfig.Config) { - m.EXPECT().GetBool(telemetryKey, true).Return(false).Once() + m.EXPECT().GetBool(telemetryKey, false).Return(false).Once() }, shouldBeEnabled: false, }, diff --git a/telemetry/instrumentation/log/handler_test.go b/telemetry/instrumentation/log/handler_test.go index d1c1521d5..e8160ef59 100644 --- a/telemetry/instrumentation/log/handler_test.go +++ b/telemetry/instrumentation/log/handler_test.go @@ -51,7 +51,7 @@ func (s *HandlerTestSuite) TestHandle_Lazy_Success() { s.handler.logger = nil s.handler.telemetry = nil - s.mockTelemetry.On("Logger", s.loggerName).Return(s.recorder.Logger(s.loggerName)).Once() + s.mockTelemetry.EXPECT().Logger(s.loggerName).Return(s.recorder.Logger(s.loggerName)).Once() entry := &TestEntry{ ctx: context.Background(), diff --git a/telemetry/setup/stubs.go b/telemetry/setup/stubs.go index ac2e84193..36239998d 100644 --- a/telemetry/setup/stubs.go +++ b/telemetry/setup/stubs.go @@ -58,7 +58,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable tracing. - "exporter": config.Env("OTEL_TRACES_EXPORTER", ""), + "exporter": config.Env("OTEL_TRACES_EXPORTER"), // Sampler Configuration // @@ -89,7 +89,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable metrics. - "exporter": config.Env("OTEL_METRICS_EXPORTER", ""), + "exporter": config.Env("OTEL_METRICS_EXPORTER"), // Reader Configuration // @@ -115,7 +115,7 @@ func init() { // // The name of the exporter definition in the "exporters" section below. // Set to "" to disable OTel logging. - "exporter": config.Env("OTEL_LOGS_EXPORTER", ""), + "exporter": config.Env("OTEL_LOGS_EXPORTER"), // Processor Configuration // From 9d3460c6e189cef333523e746e4ceaec579e3848 Mon Sep 17 00:00:00 2001 From: kkumar-gcc Date: Mon, 26 Jan 2026 23:46:09 +0530 Subject: [PATCH 29/29] optimise --- telemetry/instrumentation/http/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/telemetry/instrumentation/http/config.go b/telemetry/instrumentation/http/config.go index 3b2ad0fc2..d5345a96c 100644 --- a/telemetry/instrumentation/http/config.go +++ b/telemetry/instrumentation/http/config.go @@ -8,7 +8,8 @@ import ( "github.com/goravel/framework/contracts/http" ) -// Filter allows excluding specific requests from being traced. +// Filter determines whether a specific request should be traced. +// Return true to trace the request, or false to exclude it. type Filter func(ctx http.Context) bool // SpanNameFormatter allows customizing the span name.