internal/telemetry: change ocagent test to use the standard telemetry methods

Rather than building cusom events and driving the exporter, the test now
registers a custom exporter and then uses the normal higher level methods to
deliver events to it.
This means we are testing the actual information that would arise in a user
program, and also means we no longer have to expose internal features just for
tests.
Metrics are not fully converted yet.

Change-Id: I63248a480fb1b3e6274dce0c2e1d66904d055978
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222849
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
diff --git a/internal/telemetry/event/key.go b/internal/telemetry/event/key.go
index 151436a..d24fff6 100644
--- a/internal/telemetry/event/key.go
+++ b/internal/telemetry/event/key.go
@@ -4,6 +4,11 @@
 
 package event
 
+var (
+	// Err is a key used to add error values to tag lists.
+	Err = Key{Name: "error"}
+)
+
 // Key is used as the identity of a Tag.
 // Keys are intended to be compared by pointer only, the name should be unique
 // for communicating with external systems, but it is not required or enforced.
diff --git a/internal/telemetry/export/ocagent/metrics_test.go b/internal/telemetry/export/ocagent/metrics_test.go
index 7c75d51..372cbef 100644
--- a/internal/telemetry/export/ocagent/metrics_test.go
+++ b/internal/telemetry/export/ocagent/metrics_test.go
@@ -3,40 +3,44 @@
 import (
 	"context"
 	"testing"
-	"time"
 
 	"golang.org/x/tools/internal/telemetry/event"
 	"golang.org/x/tools/internal/telemetry/metric"
 )
 
 func TestEncodeMetric(t *testing.T) {
-	end, _ := time.Parse(time.RFC3339Nano, "1970-01-01T00:00:30Z")
+	exporter := registerExporter()
 	const prefix = testNodeStr + `
 	"metrics":[`
 	const suffix = `]}`
 	tests := []struct {
 		name string
-		data event.MetricData
+		run  func(ctx context.Context)
 		want string
 	}{
 		{
 			name: "nil data",
 			want: prefix + `null` + suffix,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, nil)
+			},
 		},
 		{
 			name: "Int64Data cumulative",
-			data: &metric.Int64Data{
-				Info: &metric.Scalar{
-					Name:        "int",
-					Description: "int metric",
-					Keys:        []*event.Key{{Name: "hello"}},
-				},
-				Rows: []int64{
-					1,
-					2,
-					3,
-				},
-				EndTime: &end,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.Int64Data{
+					Info: &metric.Scalar{
+						Name:        "int",
+						Description: "int metric",
+						Keys:        []*event.Key{keyHello},
+					},
+					Rows: []int64{
+						1,
+						2,
+						3,
+					},
+					EndTime: &exporter.start,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -82,13 +86,15 @@
 		},
 		{
 			name: "Int64Data gauge",
-			data: &metric.Int64Data{
-				Info: &metric.Scalar{
-					Name:        "int-gauge",
-					Description: "int metric gauge",
-					Keys:        []*event.Key{{Name: "hello"}},
-				},
-				IsGauge: true,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.Int64Data{
+					Info: &metric.Scalar{
+						Name:        "int-gauge",
+						Description: "int metric gauge",
+						Keys:        []*event.Key{keyHello},
+					},
+					IsGauge: true,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -105,17 +111,19 @@
 		},
 		{
 			name: "Float64Data cumulative",
-			data: &metric.Float64Data{
-				Info: &metric.Scalar{
-					Name:        "float",
-					Description: "float metric",
-					Keys:        []*event.Key{{Name: "world"}},
-				},
-				Rows: []float64{
-					1.5,
-					4.5,
-				},
-				EndTime: &end,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.Float64Data{
+					Info: &metric.Scalar{
+						Name:        "float",
+						Description: "float metric",
+						Keys:        []*event.Key{keyWorld},
+					},
+					Rows: []float64{
+						1.5,
+						4.5,
+					},
+					EndTime: &exporter.start,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -152,13 +160,15 @@
 		},
 		{
 			name: "Float64Data gauge",
-			data: &metric.Float64Data{
-				Info: &metric.Scalar{
-					Name:        "float-gauge",
-					Description: "float metric gauge",
-					Keys:        []*event.Key{{Name: "world"}},
-				},
-				IsGauge: true,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.Float64Data{
+					Info: &metric.Scalar{
+						Name:        "float-gauge",
+						Description: "float metric gauge",
+						Keys:        []*event.Key{keyWorld},
+					},
+					IsGauge: true,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -175,27 +185,29 @@
 		},
 		{
 			name: "HistogramInt64",
-			data: &metric.HistogramInt64Data{
-				Info: &metric.HistogramInt64{
-					Name:        "histogram int",
-					Description: "histogram int metric",
-					Keys:        []*event.Key{{Name: "hello"}},
-					Buckets: []int64{
-						0, 5, 10,
-					},
-				},
-				Rows: []*metric.HistogramInt64Row{
-					{
-						Count: 6,
-						Sum:   40,
-						Values: []int64{
-							1,
-							2,
-							3,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.HistogramInt64Data{
+					Info: &metric.HistogramInt64{
+						Name:        "histogram int",
+						Description: "histogram int metric",
+						Keys:        []*event.Key{keyHello},
+						Buckets: []int64{
+							0, 5, 10,
 						},
 					},
-				},
-				EndTime: &end,
+					Rows: []*metric.HistogramInt64Row{
+						{
+							Count: 6,
+							Sum:   40,
+							Values: []int64{
+								1,
+								2,
+								3,
+							},
+						},
+					},
+					EndTime: &exporter.start,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -246,26 +258,28 @@
 		},
 		{
 			name: "HistogramFloat64",
-			data: &metric.HistogramFloat64Data{
-				Info: &metric.HistogramFloat64{
-					Name:        "histogram float",
-					Description: "histogram float metric",
-					Keys:        []*event.Key{{Name: "hello"}},
-					Buckets: []float64{
-						0, 5,
-					},
-				},
-				Rows: []*metric.HistogramFloat64Row{
-					{
-						Count: 3,
-						Sum:   10,
-						Values: []int64{
-							1,
-							2,
+			run: func(ctx context.Context) {
+				exporter.Metric(ctx, &metric.HistogramFloat64Data{
+					Info: &metric.HistogramFloat64{
+						Name:        "histogram float",
+						Description: "histogram float metric",
+						Keys:        []*event.Key{keyHello},
+						Buckets: []float64{
+							0, 5,
 						},
 					},
-				},
-				EndTime: &end,
+					Rows: []*metric.HistogramFloat64Row{
+						{
+							Count: 3,
+							Sum:   10,
+							Values: []int64{
+								1,
+								2,
+							},
+						},
+					},
+					EndTime: &exporter.start,
+				})
 			},
 			want: prefix + `{
 				"metric_descriptor": {
@@ -315,9 +329,8 @@
 	ctx := context.TODO()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			exporter.Metric(ctx, tt.data)
-			exporter.Flush()
-			got := sent.get("/v1/metrics")
+			tt.run(ctx)
+			got := exporter.Output("/v1/metrics")
 			checkJSON(t, got, []byte(tt.want))
 		})
 	}
diff --git a/internal/telemetry/export/ocagent/ocagent.go b/internal/telemetry/export/ocagent/ocagent.go
index 4bd605f..1536a34 100644
--- a/internal/telemetry/export/ocagent/ocagent.go
+++ b/internal/telemetry/export/ocagent/ocagent.go
@@ -297,7 +297,7 @@
 	}
 	tags := ev.Tags
 	if ev.Error != nil {
-		tags = append(tags, event.TagOf("error", ev.Error))
+		tags = append(tags, event.Err.Of(ev.Error))
 	}
 	if description == "" && len(tags) == 0 {
 		return nil
diff --git a/internal/telemetry/export/ocagent/ocagent_test.go b/internal/telemetry/export/ocagent/ocagent_test.go
index c89e81b1..ea75305 100644
--- a/internal/telemetry/export/ocagent/ocagent_test.go
+++ b/internal/telemetry/export/ocagent/ocagent_test.go
@@ -6,6 +6,7 @@
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -14,7 +15,10 @@
 	"testing"
 	"time"
 
+	"golang.org/x/tools/internal/telemetry/event"
+	"golang.org/x/tools/internal/telemetry/export"
 	"golang.org/x/tools/internal/telemetry/export/ocagent"
+	"golang.org/x/tools/internal/telemetry/metric"
 )
 
 const testNodeStr = `{
@@ -35,22 +39,87 @@
 	},`
 
 var (
-	exporter *ocagent.Exporter
-	sent     fakeSender
-	start    time.Time
-	at       time.Time
-	end      time.Time
+	keyDB    = &event.Key{Name: "db"}
+	keyHello = &event.Key{Name: "hello"}
+	keyWorld = &event.Key{Name: "world"}
+
+	key1DB = &event.Key{Name: "1_db"}
+
+	key2aAge      = &event.Key{Name: "2a_age"}
+	key2bTTL      = &event.Key{Name: "2b_ttl"}
+	key2cExpiryMS = &event.Key{Name: "2c_expiry_ms"}
+
+	key3aRetry = &event.Key{Name: "3a_retry"}
+	key3bStale = &event.Key{Name: "3b_stale"}
+
+	key4aMax      = &event.Key{Name: "4a_max"}
+	key4bOpcode   = &event.Key{Name: "4b_opcode"}
+	key4cBase     = &event.Key{Name: "4c_base"}
+	key4eChecksum = &event.Key{Name: "4e_checksum"}
+	key4fMode     = &event.Key{Name: "4f_mode"}
+
+	key5aMin     = &event.Key{Name: "5a_min"}
+	key5bMix     = &event.Key{Name: "5b_mix"}
+	key5cPort    = &event.Key{Name: "5c_port"}
+	key5dMinHops = &event.Key{Name: "5d_min_hops"}
+	key5eMaxHops = &event.Key{Name: "5e_max_hops"}
 )
 
-func init() {
+type testExporter struct {
+	ocagent *ocagent.Exporter
+	sent    fakeSender
+	start   time.Time
+	at      time.Time
+	end     time.Time
+}
+
+func registerExporter() *testExporter {
+	exporter := &testExporter{}
 	cfg := ocagent.Config{
 		Host:    "tester",
 		Process: 1,
 		Service: "ocagent-tests",
-		Client:  &http.Client{Transport: &sent},
+		Client:  &http.Client{Transport: &exporter.sent},
 	}
 	cfg.Start, _ = time.Parse(time.RFC3339Nano, "1970-01-01T00:00:00Z")
-	exporter = ocagent.Connect(&cfg)
+	exporter.ocagent = ocagent.Connect(&cfg)
+	exporter.start, _ = time.Parse(time.RFC3339Nano, "1970-01-01T00:00:30Z")
+	exporter.at, _ = time.Parse(time.RFC3339Nano, "1970-01-01T00:00:40Z")
+	exporter.end, _ = time.Parse(time.RFC3339Nano, "1970-01-01T00:00:50Z")
+	event.SetExporter(exporter)
+	return exporter
+}
+
+func (e *testExporter) ProcessEvent(ctx context.Context, ev event.Event) (context.Context, event.Event) {
+	switch {
+	case ev.IsStartSpan():
+		ev.At = e.start
+	case ev.IsEndSpan():
+		ev.At = e.end
+	default:
+		ev.At = e.at
+	}
+	ctx, ev = export.Tag(ctx, ev)
+	ctx, ev = export.ContextSpan(ctx, ev)
+	ctx, ev = e.ocagent.ProcessEvent(ctx, ev)
+	if ev.IsStartSpan() {
+		span := export.GetSpan(ctx)
+		span.ID = export.SpanContext{}
+	}
+	return ctx, ev
+}
+
+func (e *testExporter) Metric(ctx context.Context, data event.MetricData) {
+	switch data := data.(type) {
+	case *metric.Int64Data:
+		data.EndTime = &e.start
+	}
+	e.ocagent.Metric(ctx, data)
+}
+
+func (e *testExporter) Output(route string) []byte {
+	e.ocagent.Flush()
+	return e.sent.get(route)
 }
 
 func checkJSON(t *testing.T, got, want []byte) {
diff --git a/internal/telemetry/export/ocagent/trace_test.go b/internal/telemetry/export/ocagent/trace_test.go
index fb860ab..f565168 100644
--- a/internal/telemetry/export/ocagent/trace_test.go
+++ b/internal/telemetry/export/ocagent/trace_test.go
@@ -8,186 +8,150 @@
 	"context"
 	"errors"
 	"testing"
-	"time"
 
 	"golang.org/x/tools/internal/telemetry/event"
-	"golang.org/x/tools/internal/telemetry/export"
 )
 
 func TestTrace(t *testing.T) {
-	start, _ := time.Parse(time.RFC3339Nano, "1970-01-01T00:00:30Z")
-	at, _ := time.Parse(time.RFC3339Nano, "1970-01-01T00:00:40Z")
-	end, _ := time.Parse(time.RFC3339Nano, "1970-01-01T00:00:50Z")
+	exporter := registerExporter()
 	const prefix = testNodeStr + `
-		"spans":[{
-			"trace_id":"AAAAAAAAAAAAAAAAAAAAAA==",
-			"span_id":"AAAAAAAAAAA=",
-			"parent_span_id":"AAAAAAAAAAA=",
-			"name":{"value":"event span"},
-			"start_time":"1970-01-01T00:00:30Z",
-			"end_time":"1970-01-01T00:00:50Z",
-			"time_events":{
-	`
+	"spans":[{
+		"trace_id":"AAAAAAAAAAAAAAAAAAAAAA==",
+		"span_id":"AAAAAAAAAAA=",
+		"parent_span_id":"AAAAAAAAAAA=",
+		"name":{"value":"event span"},
+		"start_time":"1970-01-01T00:00:30Z",
+		"end_time":"1970-01-01T00:00:50Z",
+		"time_events":{
+`
 	const suffix = `
-			},
-			"same_process_as_parent_span":true
-		}]
-	}`
+		},
+		"same_process_as_parent_span":true
+	}]
+}`
+
 	tests := []struct {
-		name  string
-		event func(ctx context.Context) event.Event
-		want  string
+		name string
+		run  func(ctx context.Context)
+		want string
 	}{
 		{
 			name: "no tags",
-			event: func(ctx context.Context) event.Event {
-				return event.Event{
-					At: at,
-				}
+			run: func(ctx context.Context) {
+				event.Log(ctx)
 			},
 			want: prefix + `
-						"timeEvent":[{"time":"1970-01-01T00:00:40Z"}]
-			` + suffix,
+					"timeEvent":[{"time":"1970-01-01T00:00:40Z"}]
+		` + suffix,
 		},
 		{
 			name: "description no error",
-			event: func(ctx context.Context) event.Event {
-				return event.Event{
-					At:      at,
-					Message: "cache miss",
-					Tags: event.TagList{
-						event.TagOf("db", "godb"),
-					},
-				}
+			run: func(ctx context.Context) {
+				event.Print(ctx, "cache miss", keyDB.Of("godb"))
 			},
 			want: prefix + `"timeEvent":[{"time":"1970-01-01T00:00:40Z","annotation":{
-  "description": { "value": "cache miss" },
-  "attributes": {
-    "attributeMap": {
-      "db": { "stringValue": { "value": "godb" } }
-    }
-  }
+"description": { "value": "cache miss" },
+"attributes": {
+	"attributeMap": {
+		"db": { "stringValue": { "value": "godb" } }
+	}
+}
 }}]` + suffix,
 		},
 
 		{
 			name: "description and error",
-			event: func(ctx context.Context) event.Event {
-				return event.Event{
-					At:      at,
-					Message: "cache miss",
-					Error:   errors.New("no network connectivity"),
-					Tags: event.TagList{
-						event.TagOf("db", "godb"), // must come before e
-					},
-				}
+			run: func(ctx context.Context) {
+				event.Error(ctx, "cache miss",
+					errors.New("no network connectivity"),
+					keyDB.Of("godb"),
+				)
 			},
 			want: prefix + `"timeEvent":[{"time":"1970-01-01T00:00:40Z","annotation":{
-  "description": { "value": "cache miss" },
-  "attributes": {
-    "attributeMap": {
-      "db": { "stringValue": { "value": "godb" } },
-      "error": { "stringValue": { "value": "no network connectivity" } }
-    }
-  }
-	}}]` + suffix,
+"description": { "value": "cache miss" },
+"attributes": {
+	"attributeMap": {
+		"db": { "stringValue": { "value": "godb" } },
+		"error": { "stringValue": { "value": "no network connectivity" } }
+	}
+}
+}}]` + suffix,
 		},
 		{
 			name: "no description, but error",
-			event: func(ctx context.Context) event.Event {
-				return event.Event{
-					At:    at,
-					Error: errors.New("no network connectivity"),
-					Tags: event.TagList{
-						event.TagOf("db", "godb"),
-					},
-				}
+			run: func(ctx context.Context) {
+				event.Error(ctx, "",
+					errors.New("no network connectivity"),
+					keyDB.Of("godb"),
+				)
 			},
 			want: prefix + `"timeEvent":[{"time":"1970-01-01T00:00:40Z","annotation":{
-  "description": { "value": "no network connectivity" },
-  "attributes": {
-    "attributeMap": {
-      "db": { "stringValue": { "value": "godb" } }
-    }
-  }
-	}}]` + suffix,
+"description": { "value": "no network connectivity" },
+"attributes": {
+	"attributeMap": {
+		"db": { "stringValue": { "value": "godb" } }
+	}
+}
+}}]` + suffix,
 		},
 		{
 			name: "enumerate all attribute types",
-			event: func(ctx context.Context) event.Event {
-				return event.Event{
-					At:      at,
-					Message: "cache miss",
-					Tags: event.TagList{
-						event.TagOf("1_db", "godb"),
+			run: func(ctx context.Context) {
+				event.Print(ctx, "cache miss",
+					key1DB.Of("godb"),
 
-						event.TagOf("2a_age", 0.456), // Constant converted into "float64"
-						event.TagOf("2b_ttl", float32(5000)),
-						event.TagOf("2c_expiry_ms", float64(1e3)),
+					key2aAge.Of(0.456), // Constant converted into "float64"
+					key2bTTL.Of(float32(5000)),
+					key2cExpiryMS.Of(float64(1e3)),
 
-						event.TagOf("3a_retry", false),
-						event.TagOf("3b_stale", true),
+					key3aRetry.Of(false),
+					key3bStale.Of(true),
 
-						event.TagOf("4a_max", 0x7fff), // Constant converted into "int"
-						event.TagOf("4b_opcode", int8(0x7e)),
-						event.TagOf("4c_base", int16(1<<9)),
-						event.TagOf("4e_checksum", int32(0x11f7e294)),
-						event.TagOf("4f_mode", int64(0644)),
+					key4aMax.Of(0x7fff), // Constant converted into "int"
+					key4bOpcode.Of(int8(0x7e)),
+					key4cBase.Of(int16(1<<9)),
+					key4eChecksum.Of(int32(0x11f7e294)),
+					key4fMode.Of(int64(0644)),
 
-						event.TagOf("5a_min", uint(1)),
-						event.TagOf("5b_mix", uint8(44)),
-						event.TagOf("5c_port", uint16(55678)),
-						event.TagOf("5d_min_hops", uint32(1<<9)),
-						event.TagOf("5e_max_hops", uint64(0xffffff)),
-					},
-				}
+					key5aMin.Of(uint(1)),
+					key5bMix.Of(uint8(44)),
+					key5cPort.Of(uint16(55678)),
+					key5dMinHops.Of(uint32(1<<9)),
+					key5eMaxHops.Of(uint64(0xffffff)),
+				)
 			},
 			want: prefix + `"timeEvent":[{"time":"1970-01-01T00:00:40Z","annotation":{
-  "description": { "value": "cache miss" },
-  "attributes": {
-    "attributeMap": {
-      "1_db": { "stringValue": { "value": "godb" } },
-      "2a_age": { "doubleValue": 0.456 },
-      "2b_ttl": { "doubleValue": 5000 },
-      "2c_expiry_ms": { "doubleValue": 1000 },
-      "3a_retry": {},
-			"3b_stale": { "boolValue": true },
-      "4a_max": { "intValue": 32767 },
-      "4b_opcode": { "intValue": 126 },
-      "4c_base": { "intValue": 512 },
-      "4e_checksum": { "intValue": 301458068 },
-      "4f_mode": { "intValue": 420 },
-      "5a_min": { "intValue": 1 },
-      "5b_mix": { "intValue": 44 },
-      "5c_port": { "intValue": 55678 },
-      "5d_min_hops": { "intValue": 512 },
-      "5e_max_hops": { "intValue": 16777215 }
-    }
-  }
+"description": { "value": "cache miss" },
+"attributes": {
+	"attributeMap": {
+		"1_db": { "stringValue": { "value": "godb" } },
+		"2a_age": { "doubleValue": 0.456 },
+		"2b_ttl": { "doubleValue": 5000 },
+		"2c_expiry_ms": { "doubleValue": 1000 },
+		"3a_retry": {},
+		"3b_stale": { "boolValue": true },
+		"4a_max": { "intValue": 32767 },
+		"4b_opcode": { "intValue": 126 },
+		"4c_base": { "intValue": 512 },
+		"4e_checksum": { "intValue": 301458068 },
+		"4f_mode": { "intValue": 420 },
+		"5a_min": { "intValue": 1 },
+		"5b_mix": { "intValue": 44 },
+		"5c_port": { "intValue": 55678 },
+		"5d_min_hops": { "intValue": 512 },
+		"5e_max_hops": { "intValue": 16777215 }
+	}
+}
 }}]` + suffix,
 		},
 	}
 	ctx := context.TODO()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			startEvent := event.Event{
-				Type:    event.StartSpanType,
-				Message: "event span",
-				At:      start,
-			}
-			endEvent := event.Event{
-				Type: event.EndSpanType,
-				At:   end,
-			}
-			ctx, _ := export.ContextSpan(ctx, startEvent)
-			span := export.GetSpan(ctx)
-			span.ID = export.SpanContext{}
-			span.Events = []event.Event{tt.event(ctx)}
-			exporter.ProcessEvent(ctx, startEvent)
-			export.ContextSpan(ctx, endEvent)
-			exporter.ProcessEvent(ctx, endEvent)
-			exporter.Flush()
-			got := sent.get("/v1/trace")
+			ctx, done := event.StartSpan(ctx, "event span")
+			tt.run(ctx)
+			done()
+			got := exporter.Output("/v1/trace")
 			checkJSON(t, got, []byte(tt.want))
 		})
 	}