internal/telemetry: convert key from string to struct

This makes keys a pointer, which reduces the allocations during logging, and has
a significant improvement in memory and cpu cost when there is no exporter.
Packing a string into an interface requires a 16 byte allocation, packing a pointer
does not.

name                old time/op    new time/op    delta
/LogNoExporter-8      4.39µs ± 2%    3.83µs ± 2%  -12.74%  (p=0.000 n=18+20)
/Log-8                23.5µs ± 2%    22.6µs ± 1%   -4.20%  (p=0.000 n=19+18)

name                old alloc/op   new alloc/op   delta
/LogNoExporter-8      1.70kB ± 0%    1.38kB ± 0%  -18.78%  (p=0.000 n=20+20)
/Log-8                4.91kB ± 0%    4.91kB ± 0%     ~     (p=1.000 n=20+20)

name                old allocs/op  new allocs/op  delta
/LogNoExporter-8        83.0 ± 0%      63.0 ± 0%  -24.10%  (p=0.000 n=20+20)
/Log-8                   163 ± 0%       163 ± 0%     ~     (all equal)

Change-Id: Iec127f1bff8d5c8f4bd0a6d9f6d8fc4b8bc740b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222599
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/lsp/debug/metrics.go

@@ -6,6 +6,7 @@ package debug
 
 import (
 	"golang.org/x/tools/internal/lsp/telemetry"
+	"golang.org/x/tools/internal/telemetry/event"
 	"golang.org/x/tools/internal/telemetry/metric"
 )
 
@@ -17,33 +18,33 @@ var (
 	receivedBytes = metric.HistogramInt64{
 		Name:        "received_bytes",
 		Description: "Distribution of received bytes, by method.",
-		Keys:        []interface{}{telemetry.RPCDirection, telemetry.Method},
+		Keys:        []*event.Key{telemetry.RPCDirection, telemetry.Method},
 		Buckets:     bytesDistribution,
 	}.Record(telemetry.ReceivedBytes)
 
 	sentBytes = metric.HistogramInt64{
 		Name:        "sent_bytes",
 		Description: "Distribution of sent bytes, by method.",
-		Keys:        []interface{}{telemetry.RPCDirection, telemetry.Method},
+		Keys:        []*event.Key{telemetry.RPCDirection, telemetry.Method},
 		Buckets:     bytesDistribution,
 	}.Record(telemetry.SentBytes)
 
 	latency = metric.HistogramFloat64{
 		Name:        "latency",
 		Description: "Distribution of latency in milliseconds, by method.",
-		Keys:        []interface{}{telemetry.RPCDirection, telemetry.Method},
+		Keys:        []*event.Key{telemetry.RPCDirection, telemetry.Method},
 		Buckets:     millisecondsDistribution,
 	}.Record(telemetry.Latency)
 
 	started = metric.Scalar{
 		Name:        "started",
 		Description: "Count of RPCs started by method.",
-		Keys:        []interface{}{telemetry.RPCDirection, telemetry.Method},
+		Keys:        []*event.Key{telemetry.RPCDirection, telemetry.Method},
 	}.CountInt64(telemetry.Started)
 
 	completed = metric.Scalar{
 		Name:        "completed",
 		Description: "Count of RPCs completed by method and status.",
-		Keys:        []interface{}{telemetry.RPCDirection, telemetry.Method, telemetry.StatusCode},
+		Keys:        []*event.Key{telemetry.RPCDirection, telemetry.Method, telemetry.StatusCode},
 	}.CountFloat64(telemetry.Latency)
 )

internal/lsp/telemetry/telemetry.go

@@ -12,20 +12,20 @@ import (
 	"golang.org/x/tools/internal/telemetry/unit"
 )
 
-const (
+var (
 	// create the tag keys we use
-	Method        = event.Key("method")
-	StatusCode    = event.Key("status.code")
-	StatusMessage = event.Key("status.message")
-	RPCID         = event.Key("id")
-	RPCDirection  = event.Key("direction")
-	File          = event.Key("file")
-	Directory     = event.Key("directory")
-	URI           = event.Key("URI")
-	Package       = event.Key("package")
-	PackagePath   = event.Key("package_path")
-	Query         = event.Key("query")
-	Snapshot      = event.Key("snapshot")
+	Method        = &event.Key{Name: "method"}
+	StatusCode    = &event.Key{Name: "status.code"}
+	StatusMessage = &event.Key{Name: "status.message"}
+	RPCID         = &event.Key{Name: "id"}
+	RPCDirection  = &event.Key{Name: "direction"}
+	File          = &event.Key{Name: "file"}
+	Directory     = &event.Key{Name: "directory"}
+	URI           = &event.Key{Name: "URI"}
+	Package       = &event.Key{Name: "package"}
+	PackagePath   = &event.Key{Name: "package_path"}
+	Query         = &event.Key{Name: "query"}
+	Snapshot      = &event.Key{Name: "snapshot"}
 )
 
 var (

internal/telemetry/bench_test.go

@@ -18,8 +18,8 @@ type Hooks struct {
 }
 
 var (
-	aValue  = event.Key("a")
-	bValue  = event.Key("b")
+	aValue  = &event.Key{Name: "a"}
+	bValue  = &event.Key{Name: "b"}
 	aCount  = stats.Int64("aCount", "Count of time A is called.", unit.Dimensionless)
 	aStat   = stats.Int64("aValue", "A value.", unit.Dimensionless)
 	bCount  = stats.Int64("B", "Count of time B is called.", unit.Dimensionless)

internal/telemetry/event/event.go

@@ -47,6 +47,6 @@ func (e Event) Format(f fmt.State, r rune) {
 		}
 	}
 	for _, tag := range e.Tags {
-		fmt.Fprintf(f, "\n\t%v = %v", tag.Key, tag.Value)
+		fmt.Fprintf(f, "\n\t%s = %v", tag.Key.Name, tag.Value)
 	}
 }

internal/telemetry/event/key.go

@@ -8,32 +8,32 @@ import (
 	"context"
 )
 
-// Key represents the key for a context tag.
-// It is a helper to make use of context tagging slightly easier to read, it is
-// not strictly needed to use it at all.
-// It is intended that your common tagging keys are declared as constants of
-// this type, and then you can use the methods of this type to apply and find
-// those values in the context.
-type Key string
+// 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.
+type Key struct {
+	Name        string
+	Description string
+}
 
 // TagOf returns a Tag for a key and value.
 // This is a trivial helper that makes common logging easier to read.
-func TagOf(key interface{}, value interface{}) Tag {
-	return Tag{Key: key, Value: value}
+func TagOf(name string, value interface{}) Tag {
+	return Tag{Key: &Key{Name: name}, Value: value}
 }
 
 // Of creates a new Tag with this key and the supplied value.
 // You can use this when building a tag list.
-func (k Key) Of(v interface{}) Tag {
+func (k *Key) Of(v interface{}) Tag {
 	return Tag{Key: k, Value: v}
 }
 
 // From can be used to get a tag for the key from a context.
-func (k Key) From(ctx context.Context) Tag {
+func (k *Key) From(ctx context.Context) Tag {
 	return Tag{Key: k, Value: ctx.Value(k)}
 }
 
 // With is a wrapper over the Label package level function for just this key.
-func (k Key) With(ctx context.Context, v interface{}) context.Context {
+func (k *Key) With(ctx context.Context, v interface{}) context.Context {
 	return Label(ctx, Tag{Key: k, Value: v})
 }

internal/telemetry/event/tag.go

@@ -12,7 +12,7 @@ import (
 // Tag holds a key and value pair.
 // It is normally used when passing around lists of tags.
 type Tag struct {
-	Key   interface{}
+	Key   *Key
 	Value interface{}
 }
 
@@ -23,12 +23,11 @@ type TagList []Tag
 
 // Format is used for debug printing of tags.
 func (t Tag) Format(f fmt.State, r rune) {
-	fmt.Fprintf(f, `%v="%v"`, t.Key, t.Value)
+	fmt.Fprintf(f, `%v="%v"`, t.Key.Name, t.Value)
 }
 
-// Tags collects a set of values from the context and returns them as a
-// tag list.
-func Tags(ctx context.Context, keys ...interface{}) TagList {
+// Tags collects a set of values from the context and returns them as a tag list.
+func Tags(ctx context.Context, keys ...*Key) TagList {
 	tags := make(TagList, len(keys))
 	for i, key := range keys {
 		tags[i] = Tag{Key: key, Value: ctx.Value(key)}

internal/telemetry/export/ocagent/metrics.go

@@ -202,11 +202,11 @@ func distributionToPoints(counts []int64, count int64, sum float64, bucketBounds
 
 // infoKeysToLabelKeys returns an array of *wire.LabelKeys containing the
 // string values of the elements of labelKeys.
-func infoKeysToLabelKeys(infoKeys []interface{}) []*wire.LabelKey {
+func infoKeysToLabelKeys(infoKeys []*event.Key) []*wire.LabelKey {
 	labelKeys := make([]*wire.LabelKey, 0, len(infoKeys))
 	for _, key := range infoKeys {
 		labelKeys = append(labelKeys, &wire.LabelKey{
-			Key: fmt.Sprintf("%v", key),
+			Key: fmt.Sprintf("%v", key.Name),
 		})
 	}
 

internal/telemetry/export/ocagent/metrics_test.go

@@ -29,7 +29,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.Scalar{
 					Name:        "int",
 					Description: "int metric",
-					Keys:        []interface{}{"hello"},
+					Keys:        []*event.Key{{Name: "hello"}},
 				},
 				Rows: []int64{
 					1,
@@ -86,7 +86,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.Scalar{
 					Name:        "int-gauge",
 					Description: "int metric gauge",
-					Keys:        []interface{}{"hello"},
+					Keys:        []*event.Key{{Name: "hello"}},
 				},
 				IsGauge: true,
 			},
@@ -109,7 +109,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.Scalar{
 					Name:        "float",
 					Description: "float metric",
-					Keys:        []interface{}{"world"},
+					Keys:        []*event.Key{{Name: "world"}},
 				},
 				Rows: []float64{
 					1.5,
@@ -156,7 +156,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.Scalar{
 					Name:        "float-gauge",
 					Description: "float metric gauge",
-					Keys:        []interface{}{"world"},
+					Keys:        []*event.Key{{Name: "world"}},
 				},
 				IsGauge: true,
 			},
@@ -179,7 +179,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.HistogramInt64{
 					Name:        "histogram int",
 					Description: "histogram int metric",
-					Keys:        []interface{}{"hello"},
+					Keys:        []*event.Key{{Name: "hello"}},
 					Buckets: []int64{
 						0, 5, 10,
 					},
@@ -250,7 +250,7 @@ func TestEncodeMetric(t *testing.T) {
 				Info: &metric.HistogramFloat64{
 					Name:        "histogram float",
 					Description: "histogram float metric",
-					Keys:        []interface{}{"hello"},
+					Keys:        []*event.Key{{Name: "hello"}},
 					Buckets: []float64{
 						0, 5,
 					},

internal/telemetry/export/ocagent/ocagent.go

@@ -233,7 +233,7 @@ func convertAttributes(tags event.TagList) *wire.Attributes {
 	}
 	attributes := make(map[string]wire.Attribute)
 	for _, tag := range tags {
-		attributes[fmt.Sprint(tag.Key)] = convertAttribute(tag.Value)
+		attributes[tag.Key.Name] = convertAttribute(tag.Value)
 	}
 	return &wire.Attributes{AttributeMap: attributes}
 }

internal/telemetry/metric/metric.go

@@ -21,7 +21,7 @@ type Scalar struct {
 	// Description can be used by observers to describe the metric to users.
 	Description string
 	// Keys is the set of tags that collectively describe rows of the metric.
-	Keys []interface{}
+	Keys []*event.Key
 }
 
 // HistogramInt64 represents the construction information for an int64 histogram metric.
@@ -31,7 +31,7 @@ type HistogramInt64 struct {
 	// Description can be used by observers to describe the metric to users.
 	Description string
 	// Keys is the set of tags that collectively describe rows of the metric.
-	Keys []interface{}
+	Keys []*event.Key
 	// Buckets holds the inclusive upper bound of each bucket in the histogram.
 	Buckets []int64
 }
@@ -43,7 +43,7 @@ type HistogramFloat64 struct {
 	// Description can be used by observers to describe the metric to users.
 	Description string
 	// Keys is the set of tags that collectively describe rows of the metric.
-	Keys []interface{}
+	Keys []*event.Key
 	// Buckets holds the inclusive upper bound of each bucket in the histogram.
 	Buckets []float64
 }
@@ -201,7 +201,7 @@ type HistogramFloat64Row struct {
 	Max float64
 }
 
-func getGroup(ctx context.Context, g *[]event.TagList, keys []interface{}) (int, bool) {
+func getGroup(ctx context.Context, g *[]event.TagList, keys []*event.Key) (int, bool) {
 	group := event.Tags(ctx, keys...)
 	old := *g
 	index := sort.Search(len(old), func(i int) bool {