event: WIP: another try at metrics

The code that is recording the metric is the best place to describe
that metric. For example, an RPC layer knows that number of sent RPCs
is a counter, and that bytes received is a distribution.

We want to keep that information separate from the actual metric
recoding, both to avoid duplication and to amortize any costs
involved.

The idea in this CL is to have a lightweight representation for
metrics. The world seems to have converged on just three kinds:
counter, gauge and distribution in the terminology of this CL.

A package must first create the metrics it wants to record, by calling
NewMetric with the metric kind, a name, and a description. We create a
namespace for the metric using the import path of the caller of
NewMetric.  That can be overridden, but not to the empty string. By
forcing a non-empty the namespace we can prevent ambiguous metric
names, such has having many packages use the name "call_count".

It's up to the package whether it creates the metrics globally and
unconditionally, or each time a value of a certain type is created. A
cache that has just one global instance would use the former, while
and RPC package with a Client type would ask the caller of NewClient
to pass a namespace, and store the created metrics in the Client.

Builder.Metric records a metric value as before. Its first argument is
one of the previously created metrics, from which it can extract
whatever values it needs to build an Event. Its second argument is an
event.Value, allowing any type to be a metric value.

We provide convenience functions for two common cases: Count for
marking a single instance of an event, like an RPC call or cache
probe; and Since for recording the latency since a start time.

Units are a necessary annoyance that we don't fully deal with here.
For time we use time.Duration, which lets the exporter choose the unit
and correctly convert values.

Change-Id: I26d0ac095fa2cd8b19201edc47350704636de89c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/325490
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/event/bench/event_test.go b/event/bench/event_test.go
index 9043530..76c0c9c 100644
--- a/event/bench/event_test.go
+++ b/event/bench/event_test.go
@@ -68,16 +68,17 @@
 		},
 	}
 
+	gauge       = event.NewMetric(event.Gauge, "gauge", "m")
 	eventMetric = Hooks{
 		AStart: func(ctx context.Context, a int) context.Context {
-			event.To(ctx).With(aStat.Of(a)).Metric()
-			event.To(ctx).With(aCount.Of(1)).Metric()
+			event.To(ctx).With(aStat.Of(a)).Metric(gauge, event.Int64Of(1))
+			event.To(ctx).With(aCount.Of(1)).Metric(gauge, event.Int64Of(1))
 			return ctx
 		},
 		AEnd: func(ctx context.Context) {},
 		BStart: func(ctx context.Context, b string) context.Context {
-			event.To(ctx).With(bLength.Of(len(b))).Metric()
-			event.To(ctx).With(bCount.Of(1)).Metric()
+			event.To(ctx).With(bLength.Of(len(b))).Metric(gauge, event.Int64Of(1))
+			event.To(ctx).With(bCount.Of(1)).Metric(gauge, event.Int64Of(1))
 			return ctx
 		},
 		BEnd: func(ctx context.Context) {},
diff --git a/event/builder.go b/event/builder.go
index dda25f0..0d90441 100644
--- a/event/builder.go
+++ b/event/builder.go
@@ -155,7 +155,7 @@
 }
 
 // Metric is a helper that calls Deliver with MetricKind.
-func (b Builder) Metric() {
+func (b Builder) Metric(m *Metric, value Value) {
 	if b.data == nil {
 		return
 	}
@@ -163,13 +163,27 @@
 	if b.data.exporter.metricsEnabled() {
 		b.data.exporter.mu.Lock()
 		defer b.data.exporter.mu.Unlock()
-		b.data.Event.Labels = append(b.data.Event.Labels, Metric.Value())
+		if b.data.Event.Namespace == "" {
+			b.data.Event.Namespace = m.Namespace()
+		}
+		b.data.Event.Labels = append(b.data.Event.Labels, MetricValue.Of(value), MetricKey.Of(ValueOf(m)))
 		b.data.exporter.prepare(&b.data.Event)
 		b.data.exporter.handler.Metric(b.ctx, &b.data.Event)
 	}
 	b.done()
 }
 
+func (b Builder) Count(m *Metric) {
+	if m.Kind() != Counter {
+		panic("Builder.Count called on non-counter")
+	}
+	b.Metric(m, Int64Of(1))
+}
+
+func (b Builder) Since(m *Metric, start time.Time) {
+	b.Metric(m, DurationOf(time.Since(start)))
+}
+
 // Annotate is a helper that calls Deliver with AnnotateKind.
 func (b Builder) Annotate() {
 	if b.data == nil {
diff --git a/event/common.go b/event/common.go
index e3fbe16..f7b426e 100644
--- a/event/common.go
+++ b/event/common.go
@@ -8,11 +8,13 @@
 const Name = stringKey("name")
 const Trace = traceKey("trace")
 const End = tagKey("end")
-const Metric = tagKey("metric")
+const MetricKey = valueKey("metric")
+const MetricValue = valueKey("metricValue")
 
 type stringKey string
 type traceKey string
 type tagKey string
+type valueKey string
 
 // Of creates a new message Label.
 func (k stringKey) Of(msg string) Label {
@@ -44,10 +46,8 @@
 }
 
 func (k traceKey) Find(ev *Event) (uint64, bool) {
-	for i := len(ev.Labels) - 1; i >= 0; i-- {
-		if ev.Labels[i].Name == string(k) {
-			return ev.Labels[i].Value.Uint64(), true
-		}
+	if v, ok := lookupValue(string(k), ev.Labels); ok {
+		return v.Uint64(), true
 	}
 	return 0, false
 }
@@ -58,10 +58,29 @@
 }
 
 func (k tagKey) Matches(ev *Event) bool {
-	for i := len(ev.Labels) - 1; i >= 0; i-- {
-		if ev.Labels[i].Name == string(k) {
-			return true
+	_, ok := lookupValue(string(k), ev.Labels)
+	return ok
+}
+
+func (k valueKey) Of(v Value) Label {
+	return Label{Name: string(k), Value: v}
+}
+
+func (k valueKey) Matches(ev *Event) bool {
+	_, found := k.Find(ev)
+	return found
+}
+
+func (k valueKey) Find(ev *Event) (Value, bool) {
+	return lookupValue(string(k), ev.Labels)
+
+}
+
+func lookupValue(name string, labels []Label) (Value, bool) {
+	for i := len(labels) - 1; i >= 0; i-- {
+		if labels[i].Name == name {
+			return labels[i].Value, true
 		}
 	}
-	return false
+	return Value{}, false
 }
diff --git a/event/common_test.go b/event/common_test.go
index 1bbb4e3..a3658c7 100644
--- a/event/common_test.go
+++ b/event/common_test.go
@@ -16,6 +16,7 @@
 func TestCommon(t *testing.T) {
 	h := &catchHandler{}
 	ctx := event.WithExporter(context.Background(), event.NewExporter(h, nil))
+	m := event.NewMetric(event.Counter, "m", "")
 
 	const simple = "simple message"
 	const trace = "a trace"
@@ -24,7 +25,7 @@
 	checkFind(t, h, "Log", event.Message, true, simple)
 	checkFind(t, h, "Log", event.Name, false, "")
 
-	event.To(ctx).Metric()
+	event.To(ctx).Metric(m, event.Int64Of(3))
 	checkFind(t, h, "Metric", event.Message, false, "")
 	checkFind(t, h, "Metric", event.Name, false, "")
 
diff --git a/event/event_test.go b/event/event_test.go
index e929438..cf6e0cb 100644
--- a/event/event_test.go
+++ b/event/event_test.go
@@ -11,6 +11,7 @@
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"golang.org/x/exp/event"
 	"golang.org/x/exp/event/adapter/eventtest"
@@ -19,9 +20,11 @@
 )
 
 var (
-	l1 = keys.Int("l1").Of(1)
-	l2 = keys.Int("l2").Of(2)
-	l3 = keys.Int("l3").Of(3)
+	l1      = keys.Int("l1").Of(1)
+	l2      = keys.Int("l2").Of(2)
+	l3      = keys.Int("l3").Of(3)
+	gauge   = event.NewMetric(event.Gauge, "gauge", "")
+	latency = event.NewMetric(event.Distribution, "latency", "")
 )
 
 func TestPrint(t *testing.T) {
@@ -72,12 +75,14 @@
 time=2020-03-05T14:27:52 parent=1 end
 `}, {
 		name:   "metric",
-		events: func(ctx context.Context) { event.To(ctx).With(l1).Metric() },
-		expect: `time=2020-03-05T14:27:48 l1=1 metric`,
+		events: func(ctx context.Context) { event.To(ctx).With(l1).Metric(gauge, event.Int64Of(2)) },
+		expect: `time=2020-03-05T14:27:48 l1=1 metricValue=2 metric=Gauge("golang.org/x/exp/event_test/gauge")`,
 	}, {
-		name:   "metric 2",
-		events: func(ctx context.Context) { event.To(ctx).With(l1).With(l2).Metric() },
-		expect: `time=2020-03-05T14:27:48 l1=1 l2=2 metric`,
+		name: "metric 2",
+		events: func(ctx context.Context) {
+			event.To(ctx).With(l1).With(l2).Metric(latency, event.DurationOf(3*time.Second))
+		},
+		expect: `time=2020-03-05T14:27:48 l1=1 l2=2 metricValue=3s metric=Distribution("golang.org/x/exp/event_test/latency")`,
 	}, {
 		name:   "annotate",
 		events: func(ctx context.Context) { event.To(ctx).With(l1).Annotate() },
diff --git a/event/export.go b/event/export.go
index e15cd2d..84af70c 100644
--- a/event/export.go
+++ b/event/export.go
@@ -108,26 +108,14 @@
 		// Get the pc of the user function that delivered the event.
 		// This is sensitive to the call stack.
 		// 0: runtime.Callers
-		// 1: Exporter.prepare (this function)
-		// 2: Builder.{Start,End,etc.}
-		// 3: user function
-		var pcs [1]uintptr
-		runtime.Callers(3, pcs[:])
-		pc := pcs[0]
-		if pc != 0 {
-			ns, ok := e.pcToNamespace[pc]
-			if !ok {
-				// If we call runtime.CallersFrames(pcs[:1]) in this function, the
-				// compiler will think the pcs array escapes and will allocate.
-				f := callerFrameFunction(pc)
-				ns = namespace(f)
-				if e.pcToNamespace == nil {
-					e.pcToNamespace = map[uintptr]string{}
-				}
-				e.pcToNamespace[pc] = ns
-			}
-			ev.Namespace = ns
+		// 1: importPath
+		// 2: Exporter.prepare (this function)
+		// 3: Builder.{Start,End,etc.}
+		// 4: user function
+		if e.pcToNamespace == nil {
+			e.pcToNamespace = map[uintptr]string{}
 		}
+		ev.Namespace = importPath(4, e.pcToNamespace)
 	}
 }
 
@@ -136,6 +124,23 @@
 func (e *Exporter) tracingEnabled() bool     { return !e.opts.DisableTracing }
 func (e *Exporter) metricsEnabled() bool     { return !e.opts.DisableMetrics }
 
+func importPath(depth int, cache map[uintptr]string) string {
+	var pcs [1]uintptr
+	runtime.Callers(depth, pcs[:])
+	pc := pcs[0]
+	ns, ok := cache[pc]
+	if !ok {
+		// If we call runtime.CallersFrames(pcs[:1]) in this function, the
+		// compiler will think the pcs array escapes and will allocate.
+		f := callerFrameFunction(pc)
+		ns = namespace(f)
+		if cache != nil {
+			cache[pc] = ns
+		}
+	}
+	return ns
+}
+
 func callerFrameFunction(pc uintptr) string {
 	frame, _ := runtime.CallersFrames([]uintptr{pc}).Next()
 	return frame.Function
diff --git a/event/label.go b/event/label.go
index a867310..71c65fa 100644
--- a/event/label.go
+++ b/event/label.go
@@ -9,6 +9,7 @@
 	"math"
 	"reflect"
 	"strconv"
+	"time"
 	"unsafe"
 )
 
@@ -48,6 +49,9 @@
 // boolKind is used in untyped when the Value is a boolean
 type boolKind struct{}
 
+// durationKind is used in untyped when the Value is a time.Duration
+type durationKind struct{}
+
 // HasValue returns true if the value is set to any type.
 func (v *Value) HasValue() bool { return v.untyped != nil }
 
@@ -70,6 +74,8 @@
 		return v2.IsFloat64() && v1.Float64() == v2.Float64()
 	case v1.IsBool():
 		return v2.IsBool() && v1.packed == v2.packed
+	case v1.IsDuration():
+		return v2.IsDuration() && v1.packed == v2.packed
 	default:
 		return v1.untyped == v2.untyped
 	}
@@ -95,6 +101,8 @@
 		return v.Float64()
 	case v.IsBool():
 		return v.Bool()
+	case v.IsDuration():
+		return v.Duration()
 	default:
 		return v.untyped
 	}
@@ -253,3 +261,19 @@
 	_, ok := v.untyped.(boolKind)
 	return ok
 }
+
+func DurationOf(d time.Duration) Value {
+	return Value{packed: uint64(d), untyped: durationKind{}}
+}
+
+func (v Value) Duration() time.Duration {
+	if !v.IsDuration() {
+		panic("Duration called on non-Duration value")
+	}
+	return time.Duration(v.packed)
+}
+
+func (v Value) IsDuration() bool {
+	_, ok := v.untyped.(durationKind)
+	return ok
+}
diff --git a/event/metric.go b/event/metric.go
new file mode 100644
index 0000000..6e30323
--- /dev/null
+++ b/event/metric.go
@@ -0,0 +1,78 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package event
+
+import "fmt"
+
+// MetricKind represents the kind of a Metric.
+type MetricKind int
+
+const (
+	// A Counter is a metric that always increases, usually by 1.
+	Counter MetricKind = iota
+	// A Gauge is a metric that may go up or down.
+	Gauge
+	// A Distribution is a metric for which a summary of values is tracked.
+	Distribution
+)
+
+func (k MetricKind) String() string {
+	switch k {
+	case Counter:
+		return "Counter"
+	case Gauge:
+		return "Gauge"
+	case Distribution:
+		return "Distribution"
+	default:
+		return "!unknownMetricKind"
+	}
+}
+
+type Metric struct {
+	kind        MetricKind
+	namespace   string
+	name        string
+	description string
+	// For unit, follow otel, or define Go types for common units? We don't need
+	// a time unit because we'll use time.Duration, and the only other unit otel
+	// currently defines (besides dimensionless) is bytes.
+}
+
+func NewMetric(kind MetricKind, name, description string) *Metric {
+	if name == "" {
+		panic("name cannot be empty")
+	}
+	m := &Metric{
+		kind:        kind,
+		name:        name,
+		description: description,
+	}
+	// Set namespace to the caller's import path.
+	// Depth:
+	//   0  runtime.Callers
+	//   1  importPath
+	//   2  this function
+	//   3  caller of this function
+	m.namespace = importPath(3, nil)
+	return m
+}
+
+func (m *Metric) String() string {
+	return fmt.Sprintf("%s(\"%s/%s\")", m.kind, m.namespace, m.name)
+}
+
+func (m *Metric) WithNamespace(ns string) *Metric {
+	if ns == "" {
+		panic("namespace cannot be empty")
+	}
+	m.namespace = ns
+	return m
+}
+
+func (m *Metric) Kind() MetricKind    { return m.kind }
+func (m *Metric) Name() string        { return m.name }
+func (m *Metric) Namespace() string   { return m.namespace }
+func (m *Metric) Description() string { return m.description }