internal/telemetry: add a synchronization to telemetry

Some things that used to be safe due to the global
lock now need ther own synchronization primitives.

Fixes golang/go#38102

Change-Id: I03c692682d57620d96fe84b7dc74efae0d3f8f09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226317
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/lsp/debug/trace.go

@@ -93,8 +93,8 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event, tags event.Ta
 			SpanID:   span.ID.SpanID,
 			ParentID: span.ParentID,
 			Name:     span.Name,
-			Start:    span.Start.At,
-			Tags:     renderTags(span.Start.Tags()),
+			Start:    span.Start().At,
+			Tags:     renderTags(span.Start().Tags()),
 		}
 		t.unfinished[span.ID] = td
 		// and wire up parents if we have them
@@ -117,10 +117,11 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event, tags event.Ta
 		}
 		delete(t.unfinished, span.ID)
 
-		td.Finish = span.Finish.At
-		td.Duration = span.Finish.At.Sub(span.Start.At)
-		td.Events = make([]traceEvent, len(span.Events))
-		for i, event := range span.Events {
+		td.Finish = span.Finish().At
+		td.Duration = span.Finish().At.Sub(span.Start().At)
+		events := span.Events()
+		td.Events = make([]traceEvent, len(events))
+		for i, event := range events {
 			td.Events[i] = traceEvent{
 				Time: event.At,
 				Tags: renderTags(event.Tags()),

internal/telemetry/export/metric/exporter.go

@@ -7,6 +7,7 @@ package metric
 
 import (
 	"context"
+	"sync"
 	"time"
 
 	"golang.org/x/tools/internal/telemetry/event"
@@ -28,10 +29,13 @@ func (e *Config) subscribe(key event.Key, s subscriber) {
 }
 
 func (e *Config) Exporter(output event.Exporter) event.Exporter {
+	var mu sync.Mutex
 	return func(ctx context.Context, ev event.Event, tagMap event.TagMap) context.Context {
 		if !ev.IsRecord() {
 			return output(ctx, ev, tagMap)
 		}
+		mu.Lock()
+		defer mu.Unlock()
 		var metrics []Data
 		for it := ev.Tags(); it.Valid(); it.Advance() {
 			tag := it.Tag()

internal/telemetry/export/ocagent/ocagent.go

@@ -198,10 +198,10 @@ func convertSpan(span *export.Span) *wire.Span {
 		ParentSpanID:            span.ParentID[:],
 		Name:                    toTruncatableString(span.Name),
 		Kind:                    wire.UnspecifiedSpanKind,
-		StartTime:               convertTimestamp(span.Start.At),
-		EndTime:                 convertTimestamp(span.Finish.At),
-		Attributes:              convertAttributes(event.Filter(span.Start.Tags(), event.Name)),
-		TimeEvents:              convertEvents(span.Events),
+		StartTime:               convertTimestamp(span.Start().At),
+		EndTime:                 convertTimestamp(span.Finish().At),
+		Attributes:              convertAttributes(event.Filter(span.Start().Tags(), event.Name)),
+		TimeEvents:              convertEvents(span.Events()),
 		SameProcessAsParentSpan: true,
 		//TODO: StackTrace?
 		//TODO: Links?

internal/telemetry/export/trace.go

@@ -7,6 +7,7 @@ package export
 import (
 	"context"
 	"fmt"
+	"sync"
 
 	"golang.org/x/tools/internal/telemetry/event"
 )
@@ -20,9 +21,10 @@ type Span struct {
 	Name     string
 	ID       SpanContext
 	ParentID SpanID
-	Start    event.Event
-	Finish   event.Event
-	Events   []event.Event
+	mu       sync.Mutex
+	start    event.Event
+	finish   event.Event
+	events   []event.Event
 }
 
 type contextKeyType int
@@ -50,12 +52,14 @@ func Spans(output event.Exporter) event.Exporter {
 		switch {
 		case ev.IsLog(), ev.IsLabel():
 			if span := GetSpan(ctx); span != nil {
-				span.Events = append(span.Events, ev)
+				span.mu.Lock()
+				span.events = append(span.events, ev)
+				span.mu.Unlock()
 			}
 		case ev.IsStartSpan():
 			span := &Span{
 				Name:  event.Name.Get(tagMap),
-				Start: ev,
+				start: ev,
 			}
 			if parent := GetSpan(ctx); parent != nil {
 				span.ID.TraceID = parent.ID.TraceID
@@ -67,7 +71,9 @@ func Spans(output event.Exporter) event.Exporter {
 			ctx = context.WithValue(ctx, spanContextKey, span)
 		case ev.IsEndSpan():
 			if span := GetSpan(ctx); span != nil {
-				span.Finish = ev
+				span.mu.Lock()
+				span.finish = ev
+				span.mu.Unlock()
 			}
 		case ev.IsDetach():
 			ctx = context.WithValue(ctx, spanContextKey, nil)
@@ -80,10 +86,29 @@ func (s *SpanContext) Format(f fmt.State, r rune) {
 	fmt.Fprintf(f, "%v:%v", s.TraceID, s.SpanID)
 }
 
+func (s *Span) Start() event.Event {
+	// start never changes after construction, so we dont need to hold the mutex
+	return s.start
+}
+
+func (s *Span) Finish() event.Event {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.finish
+}
+
+func (s *Span) Events() []event.Event {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.events
+}
+
 func (s *Span) Format(f fmt.State, r rune) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	fmt.Fprintf(f, "%v %v", s.Name, s.ID)
 	if s.ParentID.IsValid() {
 		fmt.Fprintf(f, "[%v]", s.ParentID)
 	}
-	fmt.Fprintf(f, " %v->%v", s.Start, s.Finish)
+	fmt.Fprintf(f, " %v->%v", s.start, s.finish)
 }