internal/lsp: use standardised events for tagging

This means that tags also become cheap if there is no exporter and cleans up the
mess with how spans, tags and logs were related.
Also fixes the currently broken metrics that relied on the span tags.

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

internal/lsp/debug/serve.go

@@ -554,6 +554,7 @@ func (e *exporter) ProcessEvent(ctx context.Context, event telemetry.Event) cont
 	if i == nil {
 		return ctx
 	}
+	ctx = export.Tag(ctx, event)
 	if i.ocagent != nil {
 		ctx = i.ocagent.ProcessEvent(ctx, event)
 	}

internal/telemetry/event.go

@@ -15,6 +15,7 @@ const (
 	EventLog = EventType(iota)
 	EventStartSpan
 	EventEndSpan
+	EventTag
 )
 
 type Event struct {

internal/telemetry/export/export.go

@@ -11,7 +11,6 @@ import (
 	"context"
 	"os"
 	"sync/atomic"
-	"time"
 	"unsafe"
 
 	"golang.org/x/tools/internal/telemetry"
@@ -42,28 +41,6 @@ func SetExporter(e Exporter) {
 	atomic.StorePointer(&exporter, p)
 }
 
-func Tag(ctx context.Context, at time.Time, tags telemetry.TagList) {
-	exporterPtr := (*Exporter)(atomic.LoadPointer(&exporter))
-	if exporterPtr == nil {
-		return
-	}
-	// If context has a span we need to add the tags to it
-	span := GetSpan(ctx)
-	if span == nil {
-		return
-	}
-	if span.Start.IsZero() {
-		// span still being created, tag it directly
-		span.Tags = append(span.Tags, tags...)
-		return
-	}
-	// span in progress, add an event to the span
-	span.Events = append(span.Events, telemetry.Event{
-		At:   at,
-		Tags: tags,
-	})
-}
-
 func ProcessEvent(ctx context.Context, event telemetry.Event) context.Context {
 	exporterPtr := (*Exporter)(atomic.LoadPointer(&exporter))
 	if exporterPtr == nil {

internal/telemetry/export/tag.go

@@ -0,0 +1,25 @@
+// 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 export
+
+import (
+	"context"
+
+	"golang.org/x/tools/internal/telemetry"
+)
+
+// Tag returns a context updated with tag values from the event.
+// It ignores events that are not or type EventTag or EventStartSpan.
+func Tag(ctx context.Context, event telemetry.Event) context.Context {
+	//TODO: Do we need to do something more efficient than just store tags
+	//TODO: directly on the context?
+	switch event.Type {
+	case telemetry.EventTag, telemetry.EventStartSpan:
+		for _, t := range event.Tags {
+			ctx = context.WithValue(ctx, t.Key, t.Value)
+		}
+	}
+	return ctx
+}

internal/telemetry/export/trace.go

@@ -41,9 +41,14 @@ func GetSpan(ctx context.Context) *Span {
 	return v.(*Span)
 }
 
+// ContextSpan is an exporter that maintains hierarchical span structure in the
+// context.
+// It creates new spans on EventStartSpan, adds events to the current span on
+// EventLog or EventTag, and closes the span on EventEndSpan.
+// The span structure can then be used by other exporters.
 func ContextSpan(ctx context.Context, event telemetry.Event) context.Context {
 	switch event.Type {
-	case telemetry.EventLog:
+	case telemetry.EventLog, telemetry.EventTag:
 		if span := GetSpan(ctx); span != nil {
 			span.Events = append(span.Events, event)
 		}

internal/telemetry/tag/tag.go

@@ -15,21 +15,13 @@ import (
 	"golang.org/x/tools/internal/telemetry/export"
 )
 
-//TODO: Do we need to do something more efficient than just store tags
-//TODO: directly on the context?
-
-// 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.
+// With delivers the tag list to the telemetry exporter.
 func With(ctx context.Context, tags ...telemetry.Tag) context.Context {
-	at := time.Now()
-	for _, t := range tags {
-		ctx = context.WithValue(ctx, t.Key, t.Value)
-	}
-	export.Tag(ctx, at, tags)
-	return ctx
+	return export.ProcessEvent(ctx, telemetry.Event{
+		Type: telemetry.EventTag,
+		At:   time.Now(),
+		Tags: tags,
+	})
 }
 
 // Get collects a set of values from the context and returns them as a tag list.