Add default metrics to othttp instrumentation Fixes #542 (#861)

* Add default metrics to othttp instrumentation

* Changed metrics names, add tests, add standard labels to metrics

* Initialization global error handling, remove requests count metric, tuneup test

* Apply suggestions from code review

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

api/standard/http.go

@@ -148,6 +148,15 @@ func HTTPClientAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue {
 
 func httpCommonAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue {
 	attrs := []kv.KeyValue{}
+	if ua := request.UserAgent(); ua != "" {
+		attrs = append(attrs, HTTPUserAgentKey.String(ua))
+	}
+
+	return append(attrs, httpBasicAttributesFromHTTPRequest(request)...)
+}
+
+func httpBasicAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue {
+	attrs := []kv.KeyValue{}
 
 	if request.TLS != nil {
 		attrs = append(attrs, HTTPSchemeHTTPS)
@@ -159,10 +168,6 @@ func httpCommonAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue {
 		attrs = append(attrs, HTTPHostKey.String(request.Host))
 	}
 
-	if ua := request.UserAgent(); ua != "" {
-		attrs = append(attrs, HTTPUserAgentKey.String(ua))
-	}
-
 	flavor := ""
 	if request.ProtoMajor == 1 {
 		flavor = fmt.Sprintf("1.%d", request.ProtoMinor)
@@ -176,6 +181,16 @@ func httpCommonAttributesFromHTTPRequest(request *http.Request) []kv.KeyValue {
 	return attrs
 }
 
+// HTTPServerMetricAttributesFromHTTPRequest generates low-cardinality attributes
+// to be used with server-side HTTP metrics.
+func HTTPServerMetricAttributesFromHTTPRequest(serverName string, request *http.Request) []kv.KeyValue {
+	attrs := []kv.KeyValue{}
+	if serverName != "" {
+		attrs = append(attrs, HTTPServerNameKey.String(serverName))
+	}
+	return append(attrs, httpBasicAttributesFromHTTPRequest(request)...)
+}
+
 // HTTPServerAttributesFromHTTPRequest generates attributes of the
 // http namespace as specified by the OpenTelemetry specification for
 // a span on the server side. Currently, only basic authentication is

instrumentation/othttp/common.go

@@ -28,6 +28,14 @@ const (
 	WriteErrorKey = kv.Key("http.write_error") // if an error occurred while writing a reply, the string of the error (io.EOF is not recorded)
 )
 
+// Server HTTP metrics
+const (
+	RequestCount          = "http.server.request_count"           // Incoming request count total
+	RequestContentLength  = "http.server.request_content_length"  // Incoming request bytes total
+	ResponseContentLength = "http.server.response_content_length" // Incoming response bytes total
+	ServerLatency         = "http.server.duration"                // Incoming end to end duration, microseconds
+)
+
 // Filter is a predicate used to determine whether a given http.request should
 // be traced. A Filter must return true if the request should be traced.
 type Filter func(*http.Request) bool

instrumentation/othttp/config.go

@@ -17,6 +17,7 @@ package othttp
 import (
 	"net/http"
 
+	"go.opentelemetry.io/otel/api/metric"
 	"go.opentelemetry.io/otel/api/propagation"
 	"go.opentelemetry.io/otel/api/trace"
 )
@@ -25,6 +26,7 @@ import (
 // and othttp.Transport types.
 type Config struct {
 	Tracer            trace.Tracer
+	Meter             metric.Meter
 	Propagators       propagation.Propagators
 	SpanStartOptions  []trace.StartOption
 	ReadEvent         bool
@@ -63,6 +65,14 @@ func WithTracer(tracer trace.Tracer) Option {
 	})
 }
 
+// WithMeter configures a specific meter. If this option
+// isn't specified then the global meter is used.
+func WithMeter(meter metric.Meter) Option {
+	return OptionFunc(func(c *Config) {
+		c.Meter = meter
+	})
+}
+
 // WithPublicEndpoint configures the Handler to link the span with an incoming
 // span context. If this option is not provided, then the association is a child
 // association instead of a link.

instrumentation/othttp/handler.go

@@ -17,9 +17,11 @@ package othttp
 import (
 	"io"
 	"net/http"
+	"time"
 
 	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/kv"
+	"go.opentelemetry.io/otel/api/metric"
 	"go.opentelemetry.io/otel/api/propagation"
 	"go.opentelemetry.io/otel/api/standard"
 	"go.opentelemetry.io/otel/api/trace"
@@ -36,12 +38,15 @@ type Handler struct {
 	handler   http.Handler
 
 	tracer            trace.Tracer
+	meter             metric.Meter
 	propagators       propagation.Propagators
 	spanStartOptions  []trace.StartOption
 	readEvent         bool
 	writeEvent        bool
 	filters           []Filter
 	spanNameFormatter func(string, *http.Request) string
+	counters          map[string]metric.Int64Counter
+	valueRecorders    map[string]metric.Int64ValueRecorder
 }
 
 func defaultHandlerFormatter(operation string, _ *http.Request) string {
@@ -56,8 +61,11 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han
 		operation: operation,
 	}
 
+	const domain = "go.opentelemetry.io/otel/instrumentation/othttp"
+
 	defaultOpts := []Option{
-		WithTracer(global.Tracer("go.opentelemetry.io/otel/instrumentation/othttp")),
+		WithTracer(global.Tracer(domain)),
+		WithMeter(global.Meter(domain)),
 		WithPropagators(global.Propagators()),
 		WithSpanOptions(trace.WithSpanKind(trace.SpanKindServer)),
 		WithSpanNameFormatter(defaultHandlerFormatter),
@@ -65,12 +73,14 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han
 
 	c := NewConfig(append(defaultOpts, opts...)...)
 	h.configure(c)
+	h.createMeasures()
 
 	return &h
 }
 
 func (h *Handler) configure(c *Config) {
 	h.tracer = c.Tracer
+	h.meter = c.Meter
 	h.propagators = c.Propagators
 	h.spanStartOptions = c.SpanStartOptions
 	h.readEvent = c.ReadEvent
@@ -79,8 +89,33 @@ func (h *Handler) configure(c *Config) {
 	h.spanNameFormatter = c.SpanNameFormatter
 }
 
+func handleErr(err error) {
+	if err != nil {
+		global.Handle(err)
+	}
+}
+
+func (h *Handler) createMeasures() {
+	h.counters = make(map[string]metric.Int64Counter)
+	h.valueRecorders = make(map[string]metric.Int64ValueRecorder)
+
+	requestBytesCounter, err := h.meter.NewInt64Counter(RequestContentLength)
+	handleErr(err)
+
+	responseBytesCounter, err := h.meter.NewInt64Counter(ResponseContentLength)
+	handleErr(err)
+
+	serverLatencyMeasure, err := h.meter.NewInt64ValueRecorder(ServerLatency)
+	handleErr(err)
+
+	h.counters[RequestContentLength] = requestBytesCounter
+	h.counters[ResponseContentLength] = responseBytesCounter
+	h.valueRecorders[ServerLatency] = serverLatencyMeasure
+}
+
 // ServeHTTP serves HTTP requests (http.Handler)
 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	requestStartTime := time.Now()
 	for _, f := range h.filters {
 		if !f(r) {
 			// Simply pass through to the handler if a filter rejects the request
@@ -121,6 +156,17 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err)
 	span.SetStatus(standard.SpanStatusFromHTTPStatusCode(rww.statusCode))
+
+	// Add request metrics
+
+	labels := standard.HTTPServerMetricAttributesFromHTTPRequest(h.operation, r)
+
+	h.counters[RequestContentLength].Add(ctx, bw.read, labels...)
+	h.counters[ResponseContentLength].Add(ctx, rww.written, labels...)
+
+	elapsedTime := time.Since(requestStartTime).Microseconds()
+
+	h.valueRecorders[ServerLatency].Record(ctx, elapsedTime, labels...)
 }
 
 func setAfterServeAttributes(span trace.Span, read, wrote int64, statusCode int, rerr, werr error) {

instrumentation/othttp/handler_example_test.go

@@ -26,6 +26,7 @@ import (
 
 	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/trace"
+	mstdout "go.opentelemetry.io/otel/exporters/metric/stdout"
 	"go.opentelemetry.io/otel/exporters/trace/stdout"
 	"go.opentelemetry.io/otel/instrumentation/othttp"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
@@ -57,7 +58,19 @@ func ExampleNewHandler() {
 	if err != nil {
 		log.Fatal(err)
 	}
+
+	pusher, err := mstdout.NewExportPipeline(mstdout.Config{
+		PrettyPrint:    true,
+		DoNotPrintTime: true, // This makes the output deterministic
+	})
+
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	meterProvider := pusher.Provider()
 	global.SetTraceProvider(tp)
+	global.SetMeterProvider(meterProvider)
 
 	figureOutName := func(ctx context.Context, s string) (string, error) {
 		pp := strings.SplitN(s, "/", 2)

instrumentation/othttp/handler_test.go

@@ -14,34 +14,66 @@
 package othttp
 
 import (
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
+	"go.opentelemetry.io/otel/api/kv"
+	"go.opentelemetry.io/otel/api/standard"
+	mockmeter "go.opentelemetry.io/otel/internal/metric"
 	mocktrace "go.opentelemetry.io/otel/internal/trace"
 )
 
+func assertMetricLabels(t *testing.T, expectedLabels []kv.KeyValue, measurementBatches []mockmeter.Batch) {
+	for _, batch := range measurementBatches {
+		assert.ElementsMatch(t, expectedLabels, batch.Labels)
+	}
+}
+
 func TestHandlerBasics(t *testing.T) {
 	rr := httptest.NewRecorder()
 
 	var id uint64
 	tracer := mocktrace.MockTracer{StartSpanID: &id}
+	meterimpl, meter := mockmeter.NewMeter()
+
+	operation := "test_handler"
 
 	h := NewHandler(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			if _, err := io.WriteString(w, "hello world"); err != nil {
 				t.Fatal(err)
 			}
-		}), "test_handler",
-		WithTracer(&tracer))
+		}), operation,
+		WithTracer(&tracer),
+		WithMeter(meter),
+	)
 
 	r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 	h.ServeHTTP(rr, r)
+
+	if len(meterimpl.MeasurementBatches) == 0 {
+		t.Fatalf("got 0 recorded measurements, expected 1 or more")
+	}
+
+	labelsToVerify := []kv.KeyValue{
+		standard.HTTPServerNameKey.String(operation),
+		standard.HTTPSchemeHTTP,
+		standard.HTTPHostKey.String(r.Host),
+		standard.HTTPFlavorKey.String(fmt.Sprintf("1.%d", r.ProtoMinor)),
+	}
+
+	standard.HTTPServerMetricAttributesFromHTTPRequest(operation, r)
+	assertMetricLabels(t, labelsToVerify, meterimpl.MeasurementBatches)
+
 	if got, expected := rr.Result().StatusCode, http.StatusOK; got != expected {
 		t.Fatalf("got %d, expected %d", got, expected)
 	}