diff --git a/instrumentation/hypertrace/metrics.go b/instrumentation/hypertrace/metrics.go new file mode 100644 index 0000000..1e27821 --- /dev/null +++ b/instrumentation/hypertrace/metrics.go @@ -0,0 +1,7 @@ +package hypertrace // import "github.com/hypertrace/goagent/instrumentation/hypertrace" + +import ( + "github.com/hypertrace/goagent/instrumentation/opentelemetry" +) + +var NewHttpOperationMetricsHandler = opentelemetry.NewHttpOperationMetricsHandler diff --git a/instrumentation/hypertrace/net/hyperhttp/handler.go b/instrumentation/hypertrace/net/hyperhttp/handler.go index 2b95a4f..734fd94 100644 --- a/instrumentation/hypertrace/net/hyperhttp/handler.go +++ b/instrumentation/hypertrace/net/hyperhttp/handler.go @@ -15,8 +15,10 @@ func NewHandler(base http.Handler, operation string, opts ...Option) http.Handle opt(o) } + mh := opentelemetry.NewHttpOperationMetricsHandler(func(_ *http.Request) string { return operation }) + return otelhttp.NewHandler( - sdkhttp.WrapHandler(base, operation, opentelemetry.SpanFromContext, o.toSDKOptions(), map[string]string{}), + sdkhttp.WrapHandler(base, opentelemetry.SpanFromContext, o.toSDKOptions(), map[string]string{}, mh), operation, ) } diff --git a/instrumentation/opencensus/metrics.go b/instrumentation/opencensus/metrics.go new file mode 100644 index 0000000..fa1c7e9 --- /dev/null +++ b/instrumentation/opencensus/metrics.go @@ -0,0 +1,21 @@ +package opencensus // import "github.com/hypertrace/goagent/instrumentation/opencensus" + +import ( + "net/http" + + "github.com/hypertrace/goagent/sdk" +) + +// Will not support metrics for OC instrumentation +type httpOperationMetricsHandler struct { +} + +func NewHttpOperationMetricsHandler() sdk.HttpOperationMetricsHandler { + return &httpOperationMetricsHandler{} +} + +func (mh *httpOperationMetricsHandler) CreateRequestCount() { +} + +func (mh *httpOperationMetricsHandler) AddToRequestCount(n int64, r *http.Request) { +} diff --git a/instrumentation/opencensus/net/hyperhttp/handler.go b/instrumentation/opencensus/net/hyperhttp/handler.go index fb7428e..474364a 100644 --- a/instrumentation/opencensus/net/hyperhttp/handler.go +++ b/instrumentation/opencensus/net/hyperhttp/handler.go @@ -10,6 +10,6 @@ import ( // WrapHandler returns a new http.Handler that should be passed to // the *ochttp.Handler func WrapHandler(delegate http.Handler, options *sdkhttp.Options) http.Handler { - // TODO: If I am doing this then I might have the metrics code in the wrong place. - return sdkhttp.WrapHandler(delegate, "", opencensus.SpanFromContext, options, map[string]string{}) + return sdkhttp.WrapHandler(delegate, opencensus.SpanFromContext, options, map[string]string{}, + opencensus.NewHttpOperationMetricsHandler()) } diff --git a/instrumentation/opentelemetry/github.com/gin-gonic/hypergin/gin.go b/instrumentation/opentelemetry/github.com/gin-gonic/hypergin/gin.go index 438cf65..2a61a7c 100644 --- a/instrumentation/opentelemetry/github.com/gin-gonic/hypergin/gin.go +++ b/instrumentation/opentelemetry/github.com/gin-gonic/hypergin/gin.go @@ -91,8 +91,10 @@ func Middleware(options *sdkhttp.Options) gin.HandlerFunc { ctx := context.WithValue(rc, hyperGinKey, ginRoute{route: ginOperationName}) wrappedHandler.c.Request = wrappedHandler.c.Request.WithContext(ctx) } + + mh := opentelemetry.NewHttpOperationMetricsHandler(func(_ *http.Request) string { return ginOperationName }) return otelhttp.NewHandler( - sdkhttp.WrapHandler(delegate, ginOperationName, opentelemetry.SpanFromContext, options, map[string]string{}), + sdkhttp.WrapHandler(delegate, opentelemetry.SpanFromContext, options, map[string]string{}, mh), "", otelhttp.WithSpanNameFormatter(spanNameFormatter), ) diff --git a/instrumentation/opentelemetry/github.com/gorilla/hypermux/mux.go b/instrumentation/opentelemetry/github.com/gorilla/hypermux/mux.go index e27c772..0e2dd02 100644 --- a/instrumentation/opentelemetry/github.com/gorilla/hypermux/mux.go +++ b/instrumentation/opentelemetry/github.com/gorilla/hypermux/mux.go @@ -9,8 +9,13 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) -func spanNameFormatter(operation string, r *http.Request) (spanName string) { +func spanNameFormatter(operation string, r *http.Request) string { + return getOperationNameFromRoute(r) +} + +func getOperationNameFromRoute(r *http.Request) string { route := mux.CurrentRoute(r) + spanName := "" if route != nil { var err error spanName, err = route.GetPathTemplate() @@ -24,16 +29,15 @@ func spanNameFormatter(operation string, r *http.Request) (spanName string) { // want to use the method as fallback. spanName = r.Method } - - return + return spanName } // NewMiddleware sets up a handler to start tracing the incoming requests. func NewMiddleware(options *sdkhttp.Options) mux.MiddlewareFunc { - // TODO: Get a proper operation name for http gorilla mux + mh := opentelemetry.NewHttpOperationMetricsHandler(getOperationNameFromRoute) return func(delegate http.Handler) http.Handler { return otelhttp.NewHandler( - sdkhttp.WrapHandler(delegate, "", opentelemetry.SpanFromContext, options, map[string]string{}), + sdkhttp.WrapHandler(delegate, opentelemetry.SpanFromContext, options, map[string]string{}, mh), "", otelhttp.WithSpanNameFormatter(spanNameFormatter), ) diff --git a/instrumentation/opentelemetry/init.go b/instrumentation/opentelemetry/init.go index e2ad2c4..2c4252d 100644 --- a/instrumentation/opentelemetry/init.go +++ b/instrumentation/opentelemetry/init.go @@ -246,6 +246,9 @@ func InitWithSpanProcessorWrapper(cfg *config.AgentConfig, wrapper SpanProcessor } } + // Initialize metrics + metricsShutdownFn := initializeMetrics(cfg) + exporterFactory = makeExporterFactory(cfg) exporter, err := exporterFactory() @@ -279,9 +282,6 @@ func InitWithSpanProcessorWrapper(cfg *config.AgentConfig, wrapper SpanProcessor otel.SetTextMapPropagator(makePropagator(cfg.PropagationFormats)) - // Initialize metrics - metricsShutdownFn := initializeMetrics(cfg) - traceProviders = make(map[string]*sdktrace.TracerProvider) globalSampler = sampler initialized = true @@ -421,7 +421,10 @@ func initializeMetrics(cfg *config.AgentConfig) func() { otelmetricglobal.SetMeterProvider(metricsPusher) return func() { - metricsPusher.Stop(context.Background()) + err := metricsPusher.Stop(context.Background()) + if err != nil { + log.Printf("an error while calling metrics pusher stop: %v", err) + } } } diff --git a/instrumentation/opentelemetry/metrics.go b/instrumentation/opentelemetry/metrics.go new file mode 100644 index 0000000..6f81d50 --- /dev/null +++ b/instrumentation/opentelemetry/metrics.go @@ -0,0 +1,57 @@ +package opentelemetry // import "github.com/hypertrace/goagent/instrumentation/opentelemetry" + +import ( + "net/http" + + "github.com/hypertrace/goagent/sdk" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/global" + "go.opentelemetry.io/otel/metric/instrument/syncint64" + semconv "go.opentelemetry.io/otel/semconv/v1.12.0" +) + +// Server HTTP metrics. +const ( + // Pseudo of go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#RequestCount since a metric is not + // created for that one for some reason.(annotated with hypertrace to avoid a duplicate if otel go ever implement + // their own) + RequestCount = "hypertrace.http.server.request_count" // Incoming request count total +) + +type HttpOperationMetricsHandler struct { + operationNameGetter func(*http.Request) string + counters map[string]syncint64.Counter +} + +var _ sdk.HttpOperationMetricsHandler = (*HttpOperationMetricsHandler)(nil) + +func NewHttpOperationMetricsHandler(nameGetter func(*http.Request) string) sdk.HttpOperationMetricsHandler { + return &HttpOperationMetricsHandler{ + operationNameGetter: nameGetter, + counters: make(map[string]syncint64.Counter, 1), + } +} + +func (mh *HttpOperationMetricsHandler) CreateRequestCount() { + mp := global.MeterProvider() + meter := mp.Meter("go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", + metric.WithInstrumentationVersion(otelhttp.SemVersion())) + + requestCountCounter, err := meter.SyncInt64().Counter(RequestCount) + if err != nil { + otel.Handle(err) + } + + mh.counters[RequestCount] = requestCountCounter +} + +func (mh *HttpOperationMetricsHandler) AddToRequestCount(n int64, r *http.Request) { + // Add metrics using the same logic in go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#handler.go + ctx := r.Context() + labeler, _ := otelhttp.LabelerFromContext(ctx) + operationName := mh.operationNameGetter(r) + attributes := append(labeler.Get(), semconv.HTTPServerMetricAttributesFromHTTPRequest(operationName, r)...) + mh.counters[RequestCount].Add(ctx, n, attributes...) +} diff --git a/instrumentation/opentelemetry/net/hyperhttp/handler.go b/instrumentation/opentelemetry/net/hyperhttp/handler.go index c241add..afce2b9 100644 --- a/instrumentation/opentelemetry/net/hyperhttp/handler.go +++ b/instrumentation/opentelemetry/net/hyperhttp/handler.go @@ -10,6 +10,6 @@ import ( // WrapHandler returns a new round tripper instrumented that relies on the // needs to be used with OTel instrumentation. func WrapHandler(delegate http.Handler, options *sdkhttp.Options) http.Handler { - // TODO: Find another way to get the operation name - return sdkhttp.WrapHandler(delegate, "", opentelemetry.SpanFromContext, options, map[string]string{}) + mh := opentelemetry.NewHttpOperationMetricsHandler(func(_ *http.Request) string { return "" }) + return sdkhttp.WrapHandler(delegate, opentelemetry.SpanFromContext, options, map[string]string{}, mh) } diff --git a/sdk/instrumentation/net/http/handler.go b/sdk/instrumentation/net/http/handler.go index d449a9a..997feb6 100644 --- a/sdk/instrumentation/net/http/handler.go +++ b/sdk/instrumentation/net/http/handler.go @@ -12,31 +12,15 @@ import ( "github.com/hypertrace/goagent/sdk/filter" internalconfig "github.com/hypertrace/goagent/sdk/internal/config" "github.com/hypertrace/goagent/sdk/internal/container" - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/metric/global" - "go.opentelemetry.io/otel/metric/instrument/syncint64" - semconv "go.opentelemetry.io/otel/semconv/v1.12.0" -) - -// Server HTTP metrics. -const ( - // Pseudo of go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#RequestCount since a metric is not - // created for that one for some reason.(annotated with hypertrace to avoid a duplicate if otel go ever implement - // their own) - RequestCount = "hypertrace.http.server.request_count" // Incoming request count total ) type handler struct { delegate http.Handler - operation string defaultAttributes map[string]string spanFromContextRetriever sdk.SpanFromContext dataCaptureConfig *config.DataCapture filter filter.Filter - // Some metrics in here. - counters map[string]syncint64.Counter + mh sdk.HttpOperationMetricsHandler } // Options for HTTP handler instrumentation @@ -46,7 +30,8 @@ type Options struct { // WrapHandler wraps an uninstrumented handler (e.g. a handleFunc) and returns a new one // that should be used as base to an instrumented handler -func WrapHandler(delegate http.Handler, operation string, spanFromContext sdk.SpanFromContext, options *Options, spanAttributes map[string]string) http.Handler { +func WrapHandler(delegate http.Handler, spanFromContext sdk.SpanFromContext, options *Options, spanAttributes map[string]string, + mh sdk.HttpOperationMetricsHandler) http.Handler { defaultAttributes := make(map[string]string) for k, v := range spanAttributes { defaultAttributes[k] = v @@ -59,29 +44,17 @@ func WrapHandler(delegate http.Handler, operation string, spanFromContext sdk.Sp f = options.Filter } - mp := global.MeterProvider() - meter := mp.Meter("go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", - metric.WithInstrumentationVersion(otelhttp.SemVersion())) - counters := make(map[string]syncint64.Counter) - - requestCountCounter, err := meter.SyncInt64().Counter(RequestCount) - if err != nil { - otel.Handle(err) - } - - counters[RequestCount] = requestCountCounter + // Create request count metric + mh.CreateRequestCount() - return &handler{delegate, operation, defaultAttributes, spanFromContext, internalconfig.GetConfig().GetDataCapture(), f, counters} + return &handler{delegate, defaultAttributes, spanFromContext, internalconfig.GetConfig().GetDataCapture(), f, mh} } func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Add metrics using the same logic in go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#handler.go ctx := r.Context() - labeler, _ := otelhttp.LabelerFromContext(ctx) - attributes := append(labeler.Get(), semconv.HTTPServerMetricAttributesFromHTTPRequest(h.operation, r)...) - h.counters[RequestCount].Add(ctx, 1, attributes...) + span := h.spanFromContextRetriever(ctx) - span := h.spanFromContextRetriever(r.Context()) + h.mh.AddToRequestCount(1, r) if span.IsNoop() { // isNoop means either the span is not sampled or there was no span diff --git a/sdk/instrumentation/net/http/handler_test.go b/sdk/instrumentation/net/http/handler_test.go index 5ea3fda..4085ddc 100644 --- a/sdk/instrumentation/net/http/handler_test.go +++ b/sdk/instrumentation/net/http/handler_test.go @@ -16,7 +16,16 @@ import ( "github.com/stretchr/testify/assert" ) -const fooOpName string = "/foo" +// A no-op metrics handler +type metricsHandler struct { +} + +func (mh *metricsHandler) CreateRequestCount() { + +} +func (mh *metricsHandler) AddToRequestCount(int64, *http.Request) { + +} var emptyTestConfig = &config.DataCapture{ HttpHeaders: &config.Message{ @@ -52,7 +61,7 @@ func TestServerRequestWithNilBodyIsntChanged(t *testing.T) { assert.Nil(t, r.Body) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{}, map[string]string{}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{}, map[string]string{}, &metricsHandler{}).(*handler) wh.dataCaptureConfig = emptyTestConfig ih := &mockHandler{baseHandler: wh} @@ -76,7 +85,7 @@ func TestServerRequestIsSuccessfullyTraced(t *testing.T) { rw.Write([]byte("ponse_body")) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{}, map[string]string{"foo": "bar"}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{}, map[string]string{"foo": "bar"}, &metricsHandler{}).(*handler) wh.dataCaptureConfig = emptyTestConfig ih := &mockHandler{baseHandler: wh} @@ -105,7 +114,7 @@ func TestHostIsSuccessfullyRecorded(t *testing.T) { assert.Nil(t, r.Body) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{}, map[string]string{}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{}, map[string]string{}, &metricsHandler{}).(*handler) wh.dataCaptureConfig = emptyTestConfig ih := &mockHandler{baseHandler: wh} @@ -141,7 +150,7 @@ func TestServerRequestHeadersAreSuccessfullyRecorded(t *testing.T) { rw.WriteHeader(202) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{}, map[string]string{}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{}, map[string]string{}, &metricsHandler{}).(*handler) ih := &mockHandler{baseHandler: wh} wh.dataCaptureConfig = emptyTestConfig wh.dataCaptureConfig.HttpHeaders = &config.Message{ @@ -287,7 +296,7 @@ func TestServerRecordsRequestAndResponseBodyAccordingly(t *testing.T) { rw.Write([]byte(tCase.responseBody)) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{}, map[string]string{}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{}, map[string]string{}, &metricsHandler{}).(*handler) wh.dataCaptureConfig = emptyTestConfig wh.dataCaptureConfig.HttpBody = &config.Message{ Request: config.Bool(tCase.captureHTTPBodyConfig), @@ -465,7 +474,7 @@ func TestServerRequestFilter(t *testing.T) { rw.WriteHeader(http.StatusOK) }) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, tCase.options, map[string]string{}).(*handler) + wh, _ := WrapHandler(h, mock.SpanFromContext, tCase.options, map[string]string{}, &metricsHandler{}).(*handler) ih := &mockHandler{baseHandler: wh} r, _ := http.NewRequest("POST", tCase.url, strings.NewReader(tCase.body)) for i := 0; i < len(tCase.headerKeys); i++ { @@ -493,14 +502,14 @@ func TestProcessingBodyIsTrimmed(t *testing.T) { h := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {}) - wh, _ := WrapHandler(h, fooOpName, mock.SpanFromContext, &Options{ + wh, _ := WrapHandler(h, mock.SpanFromContext, &Options{ Filter: mock.Filter{ BodyEvaluator: func(span sdk.Span, body []byte, headers map[string][]string) result.FilterResult { assert.Equal(t, "{", string(body)) // body is truncated return result.FilterResult{Block: true, ResponseStatusCode: 403} }, }, - }, map[string]string{}).(*handler) + }, map[string]string{}, &metricsHandler{}).(*handler) wh.dataCaptureConfig = emptyTestConfig wh.dataCaptureConfig.HttpBody.Request = config.Bool(true) wh.dataCaptureConfig.BodyMaxProcessingSizeBytes = config.Int32(int32(bodyMaxProcessingSizeBytes)) diff --git a/sdk/metrics.go b/sdk/metrics.go new file mode 100644 index 0000000..5d73737 --- /dev/null +++ b/sdk/metrics.go @@ -0,0 +1,10 @@ +package sdk // import "github.com/hypertrace/goagent/sdk" + +import ( + "net/http" +) + +type HttpOperationMetricsHandler interface { + CreateRequestCount() + AddToRequestCount(int64, *http.Request) +}