internal/lsp: add the new form of the telemetry tagging package

This maps more directly to the basic telementery tagging requirements and uses
the context package in a way that is more idomatic.

Change-Id: If08c429b897bddfe014224ac2d92d7796a521ab9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184941
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/jsonrpc2/jsonrpc2.go

@@ -103,29 +103,24 @@ func start(ctx context.Context, server bool, method string, id *ID) (context.Con
 		start:  time.Now(),
 	}
 	ctx = context.WithValue(ctx, rpcStatsKey, s)
-	tags := make([]tag.Mutator, 0, 4)
-	tags = append(tags, tag.Upsert(telemetry.KeyMethod, method))
 	mode := telemetry.Outbound
-	spanKind := trace.SpanKindClient
 	if server {
-		spanKind = trace.SpanKindServer
 		mode = telemetry.Inbound
 	}
-	tags = append(tags, tag.Upsert(telemetry.KeyRPCDirection, mode))
-	if id != nil {
-		tags = append(tags, tag.Upsert(telemetry.KeyRPCID, id.String()))
-	}
-	ctx, s.span = trace.StartSpan(ctx, method, trace.WithSpanKind(spanKind))
-	ctx, _ = tag.New(ctx, tags...)
+	ctx, s.span = trace.StartSpan(ctx, method,
+		tag.Tag{Key: telemetry.Method, Value: method},
+		tag.Tag{Key: telemetry.RPCDirection, Value: mode},
+		tag.Tag{Key: telemetry.RPCID, Value: id},
+	)
 	stats.Record(ctx, telemetry.Started.M(1))
 	return ctx, s
 }
 
 func (s *rpcStats) end(ctx context.Context, err *error) {
 	if err != nil && *err != nil {
-		ctx, _ = tag.New(ctx, tag.Upsert(telemetry.KeyStatus, "ERROR"))
+		ctx = telemetry.StatusCode.With(ctx, "ERROR")
 	} else {
-		ctx, _ = tag.New(ctx, tag.Upsert(telemetry.KeyStatus, "OK"))
+		ctx = telemetry.StatusCode.With(ctx, "OK")
 	}
 	elapsedTime := time.Since(s.start)
 	latencyMillis := float64(elapsedTime) / float64(time.Millisecond)
@@ -309,7 +304,7 @@ func (r *Request) Reply(ctx context.Context, result interface{}, err error) erro
 	if r.IsNotify() {
 		return fmt.Errorf("reply not invoked with a valid call")
 	}
-	ctx, st := trace.StartSpan(ctx, r.Method+":reply", trace.WithSpanKind(trace.SpanKindClient))
+	ctx, st := trace.StartSpan(ctx, r.Method+":reply")
 	defer st.End()
 
 	// reply ends the handling phase of a call, so if we are not yet

internal/lsp/telemetry/tag/key.go

@@ -0,0 +1,31 @@
+// Copyright 2019 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 tag provides support for telemetry tagging.
+package tag
+
+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
+
+// 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 {
+	return Tag{Key: k, Value: v}
+}
+
+// With applies sets this key to the supplied value on the context and
+// returns the new context generated.
+// It uses the With package level function so that observers are also notified.
+func (k Key) With(ctx context.Context, v interface{}) context.Context {
+	return With(ctx, Tag{Key: k, Value: v})
+}

internal/lsp/telemetry/tag/tag.go

@@ -2,31 +2,118 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Package tag adds support for telemetry tags.
+// Package tag provides support for telemetry tagging.
+// This package is a thin shim over contexts with the main addition being the
+// the ability to observe when contexts get tagged with new values.
 package tag
 
-import "context"
+import (
+	"context"
+	"fmt"
+	"time"
 
-type Map interface{}
-
-type Key interface {
-	Name() string
-}
-
-type Mutator interface {
-	Mutate(Map) (Map, error)
-}
-
-type nullMutator struct{}
-
-func (nullMutator) Mutate(Map) (Map, error) { return nil, nil }
-
-var (
-	New         = func(ctx context.Context, mutator ...Mutator) (context.Context, error) { return ctx, nil }
-	NewContext  = func(ctx context.Context, m Map) context.Context { return ctx }
-	FromContext = func(ctx context.Context) Map { return nil }
-	Delete      = func(k Key) Mutator { return nullMutator{} }
-	Insert      = func(k Key, v string) Mutator { return nullMutator{} }
-	Update      = func(k Key, v string) Mutator { return nullMutator{} }
-	Upsert      = func(k Key, v string) Mutator { return nullMutator{} }
+	"golang.org/x/tools/internal/lsp/telemetry/worker"
 )
+
+//TODO: Do we need to do something more efficient than just store tags
+//TODO: directly on the context?
+
+// Tag holds a key and value pair.
+// It is normally used when passing around lists of tags.
+type Tag struct {
+	Key   interface{}
+	Value interface{}
+}
+
+// List is a way of passing around a collection of key value pairs.
+// It is an alternative to the less efficient and unordered method of using
+// maps.
+type List []Tag
+
+// Observer is the type for a function that wants to be notified when new tags
+// are set on a context.
+// If you use context.WithValue (or equivalent) it will bypass the observers,
+// you must use the setters in this package for tags that should be observed.
+// Register new observers with the Observe function.
+type Observer func(ctx context.Context, at time.Time, tags List)
+
+// With is roughly equivalent to context.WithValue except that it also notifies
+// registered observers.
+// Unlike WithValue, it takes a list of tags so that you can set many values
+// at once if needed. Each call to With results in one invocation of each
+// observer.
+func With(ctx context.Context, tags ...Tag) context.Context {
+	at := time.Now()
+	for _, t := range tags {
+		ctx = context.WithValue(ctx, t.Key, t.Value)
+	}
+	worker.Do(func() {
+		for i := len(observers) - 1; i >= 0; i-- {
+			observers[i](ctx, at, tags)
+		}
+	})
+	return ctx
+}
+
+// Get collects a set of values from the context and returns them as a tag list.
+func Get(ctx context.Context, keys ...interface{}) List {
+	tags := make(List, len(keys))
+	for i, key := range keys {
+		tags[i] = Tag{Key: key, Value: ctx.Value(key)}
+	}
+	return tags
+}
+
+var observers = []Observer{}
+
+// Observe adds a new tag observer to the registered set.
+// There is no way to ever unregister a observer.
+// Observers are free to use context information to control their behavior.
+func Observe(observer Observer) {
+	worker.Do(func() {
+		observers = append(observers, observer)
+	})
+}
+
+// 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)
+}
+
+// Get will get a single key's value from the list.
+func (l List) Get(k interface{}) interface{} {
+	for _, t := range l {
+		if t.Key == k {
+			return t.Value
+		}
+	}
+	return nil
+}
+
+// Format pretty prints a list.
+// It is intended only for debugging.
+func (l List) Format(f fmt.State, r rune) {
+	printed := false
+	for _, t := range l {
+		if t.Value == nil {
+			continue
+		}
+		if printed {
+			fmt.Fprint(f, ",")
+		}
+		fmt.Fprint(f, t)
+		printed = true
+	}
+}
+
+// Equal returns true if two lists are identical.
+func (l List) Equal(other List) bool {
+	//TODO: make this more efficient
+	return fmt.Sprint(l) == fmt.Sprint(other)
+}
+
+// Less is intended only for using tag lists as a sorting key.
+func (l List) Less(other List) bool {
+	//TODO: make this more efficient
+	return fmt.Sprint(l) < fmt.Sprint(other)
+}

internal/lsp/telemetry/telemetry.go

@@ -13,6 +13,17 @@ import (
 	"golang.org/x/tools/internal/lsp/telemetry/tag"
 )
 
+const (
+	// create the tag keys we use
+	Method        = tag.Key("method")
+	StatusCode    = tag.Key("status.code")
+	StatusMessage = tag.Key("status.message")
+	RPCID         = tag.Key("id")
+	RPCDirection  = tag.Key("direction")
+	File          = tag.Key("file")
+	Package       = tag.Key("package")
+)
+
 var (
 	Handle = func(mux *http.ServeMux) {}
 
@@ -20,11 +31,6 @@ var (
 	ReceivedBytes = stats.NullInt64Measure()
 	SentBytes     = stats.NullInt64Measure()
 	Latency       = stats.NullFloat64Measure()
-
-	KeyRPCID        tag.Key
-	KeyMethod       tag.Key
-	KeyStatus       tag.Key
-	KeyRPCDirection tag.Key
 )
 
 const (