internal/lsp: reduce trace package to minimal StartSpan for now

also change the return type to be and end function and not an incomplete span

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

internal/jsonrpc2/jsonrpc2.go

@@ -82,7 +82,7 @@ type Canceler func(context.Context, *Conn, ID)
 type rpcStats struct {
 	server bool
 	method string
-	span   trace.Span
+	close  func()
 	start  time.Time
 }
 
@@ -99,7 +99,7 @@ func start(ctx context.Context, server bool, method string, id *ID) (context.Con
 	if server {
 		mode = telemetry.Inbound
 	}
-	ctx, s.span = trace.StartSpan(ctx, method,
+	ctx, s.close = 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},
@@ -117,7 +117,7 @@ func (s *rpcStats) end(ctx context.Context, err *error) {
 	elapsedTime := time.Since(s.start)
 	latencyMillis := float64(elapsedTime) / float64(time.Millisecond)
 	telemetry.Latency.Record(ctx, latencyMillis)
-	s.span.End()
+	s.close()
 }
 
 // NewErrorf builds a Error struct for the suppied message and code.
@@ -290,8 +290,8 @@ 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")
+	ctx, close := trace.StartSpan(ctx, r.Method+":reply")
-	defer st.End()
+	defer close()
 
 	// reply ends the handling phase of a call, so if we are not yet
 	// parallel we should be now. The go routine is allowed to continue

internal/lsp/cache/check.go

@@ -89,8 +89,8 @@ func (imp *importer) getPkg(ctx context.Context, id packageID) (*pkg, error) {
 }
 
 func (imp *importer) typeCheck(ctx context.Context, id packageID) (*pkg, error) {
-	ctx, ts := trace.StartSpan(ctx, "cache.importer.typeCheck")
+	ctx, done := trace.StartSpan(ctx, "cache.importer.typeCheck")
-	defer ts.End()
+	defer done()
 	meta, ok := imp.view.mcache.packages[id]
 	if !ok {
 		return nil, fmt.Errorf("no metadata for %v", id)

internal/lsp/cache/external.go

@@ -51,8 +51,8 @@ func (h *nativeFileHandle) Kind() source.FileKind {
 }
 
 func (h *nativeFileHandle) Read(ctx context.Context) ([]byte, string, error) {
-	ctx, ts := trace.StartSpan(ctx, "cache.nativeFileHandle.Read")
+	ctx, done := trace.StartSpan(ctx, "cache.nativeFileHandle.Read")
-	defer ts.End()
+	defer done()
 	//TODO: this should fail if the version is not the same as the handle
 	data, err := ioutil.ReadFile(h.identity.URI.Filename())
 	if err != nil {

internal/lsp/cache/parse.go

@@ -74,8 +74,8 @@ func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, error) {
 }
 
 func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (*ast.File, error) {
-	ctx, ts := trace.StartSpan(ctx, "cache.parseGo")
+	ctx, done := trace.StartSpan(ctx, "cache.parseGo")
-	defer ts.End()
+	defer done()
 	buf, _, err := fh.Read(ctx)
 	if err != nil {
 		return nil, err

internal/lsp/protocol/protocol.go

@@ -18,8 +18,8 @@ const defaultRejectIfOverloaded = false
 
 func canceller(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.ID) {
 	ctx = xcontext.Detach(ctx)
-	ctx, span := trace.StartSpan(ctx, "protocol.canceller")
+	ctx, done := trace.StartSpan(ctx, "protocol.canceller")
-	defer span.End()
+	defer done()
 	conn.Notify(ctx, "$/cancelRequest", &CancelParams{ID: id})
 }
 

internal/lsp/source/analysis.go

@@ -24,8 +24,8 @@ import (
 )
 
 func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis.Analyzer) ([]*Action, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.analyze")
+	ctx, done := trace.StartSpan(ctx, "source.analyze")
-	defer ts.End()
+	defer done()
 	if ctx.Err() != nil {
 		return nil, ctx.Err()
 	}

internal/lsp/source/completion.go

@@ -278,8 +278,8 @@ type CompletionOptions struct {
 // the client to score the quality of the completion. For instance, some clients
 // may tolerate imperfect matches as valid completion results, since users may make typos.
 func Completion(ctx context.Context, view View, f GoFile, pos token.Pos, opts CompletionOptions) ([]CompletionItem, *Selection, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Completion")
+	ctx, done := trace.StartSpan(ctx, "source.Completion")
-	defer ts.End()
+	defer done()
 	file := f.GetAST(ctx)
 	if file == nil {
 		return nil, nil, fmt.Errorf("no AST for %s", f.URI())

internal/lsp/source/format.go

@@ -22,8 +22,8 @@ import (
 
 // Format formats a file with a given range.
 func Format(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Format")
+	ctx, done := trace.StartSpan(ctx, "source.Format")
-	defer ts.End()
+	defer done()
 	file := f.GetAST(ctx)
 	if file == nil {
 		return nil, fmt.Errorf("no AST for %s", f.URI())
@@ -53,8 +53,8 @@ func Format(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) {
 
 // Imports formats a file using the goimports tool.
 func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]TextEdit, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Imports")
+	ctx, done := trace.StartSpan(ctx, "source.Imports")
-	defer ts.End()
+	defer done()
 	data, _, err := f.Handle(ctx).Read(ctx)
 	if err != nil {
 		return nil, err
@@ -133,8 +133,8 @@ func buildProcessEnv(ctx context.Context, view View) *imports.ProcessEnv {
 }
 
 func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
-	ctx, ts := trace.StartSpan(ctx, "source.computeTextEdits")
+	ctx, done := trace.StartSpan(ctx, "source.computeTextEdits")
-	defer ts.End()
+	defer done()
 	data, _, err := file.Handle(ctx).Read(ctx)
 	if err != nil {
 		file.View().Session().Logger().Errorf(ctx, "Cannot compute text edits: %v", err)

internal/lsp/source/highlight.go

@@ -16,8 +16,8 @@ import (
 )
 
 func Highlight(ctx context.Context, f GoFile, pos token.Pos) ([]span.Span, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Highlight")
+	ctx, done := trace.StartSpan(ctx, "source.Highlight")
-	defer ts.End()
+	defer done()
 	file := f.GetAST(ctx)
 	if file == nil {
 		return nil, fmt.Errorf("no AST for %s", f.URI())

internal/lsp/source/hover.go

@@ -33,8 +33,8 @@ const (
 )
 
 func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hoverKind HoverKind) (string, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Hover")
+	ctx, done := trace.StartSpan(ctx, "source.Hover")
-	defer ts.End()
+	defer done()
 	h, err := i.decl.hover(ctx)
 	if err != nil {
 		return "", err
@@ -80,8 +80,8 @@ func (i *IdentifierInfo) Documentation(ctx context.Context, hoverKind HoverKind)
 }
 
 func (d declaration) hover(ctx context.Context) (*documentation, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.hover")
+	ctx, done := trace.StartSpan(ctx, "source.hover")
-	defer ts.End()
+	defer done()
 	obj := d.obj
 	switch node := d.node.(type) {
 	case *ast.ImportSpec:

internal/lsp/source/identifier.go

@@ -63,8 +63,8 @@ func Identifier(ctx context.Context, view View, f GoFile, pos token.Pos) (*Ident
 
 // identifier checks a single position for a potential identifier.
 func identifier(ctx context.Context, view View, f GoFile, pos token.Pos) (*IdentifierInfo, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.identifier")
+	ctx, done := trace.StartSpan(ctx, "source.identifier")
-	defer ts.End()
+	defer done()
 	file := f.GetAST(ctx)
 	if file == nil {
 		return nil, fmt.Errorf("no AST for %s", f.URI())

internal/lsp/source/references.go

@@ -27,8 +27,8 @@ type ReferenceInfo struct {
 // References returns a list of references for a given identifier within the packages
 // containing i.File. Declarations appear first in the result.
 func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.References")
+	ctx, done := trace.StartSpan(ctx, "source.References")
-	defer ts.End()
+	defer done()
 	var references []*ReferenceInfo
 
 	// If the object declaration is nil, assume it is an import spec and do not look for references.

internal/lsp/source/rename.go

@@ -37,8 +37,8 @@ type renamer struct {
 
 // Rename returns a map of TextEdits for each file modified when renaming a given identifier within a package.
 func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.URI][]TextEdit, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.Rename")
+	ctx, done := trace.StartSpan(ctx, "source.Rename")
-	defer ts.End()
+	defer done()
 	if i.Name == newName {
 		return nil, fmt.Errorf("old and new names are the same: %s", newName)
 	}

internal/lsp/source/signature_help.go

@@ -26,8 +26,8 @@ type ParameterInformation struct {
 }
 
 func SignatureHelp(ctx context.Context, f GoFile, pos token.Pos) (*SignatureInformation, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.SignatureHelp")
+	ctx, done := trace.StartSpan(ctx, "source.SignatureHelp")
-	defer ts.End()
+	defer done()
 	file := f.GetAST(ctx)
 	if file == nil {
 		return nil, fmt.Errorf("no AST for %s", f.URI())

internal/lsp/source/symbols.go

@@ -42,8 +42,8 @@ type Symbol struct {
 }
 
 func DocumentSymbols(ctx context.Context, f GoFile) ([]Symbol, error) {
-	ctx, ts := trace.StartSpan(ctx, "source.DocumentSymbols")
+	ctx, done := trace.StartSpan(ctx, "source.DocumentSymbols")
-	defer ts.End()
+	defer done()
 	fset := f.FileSet()
 	file := f.GetAST(ctx)
 	if file == nil {

internal/lsp/symbols.go

@@ -14,8 +14,8 @@ import (
 )
 
 func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSymbolParams) ([]protocol.DocumentSymbol, error) {
-	ctx, ts := trace.StartSpan(ctx, "lsp.Server.documentSymbol")
+	ctx, done := trace.StartSpan(ctx, "lsp.Server.documentSymbol")
-	defer ts.End()
+	defer done()
 	uri := span.NewURI(params.TextDocument.URI)
 	view := s.session.ViewOf(uri)
 	f, m, err := getGoFile(ctx, view, uri)

internal/lsp/telemetry/trace/trace.go

@@ -2,59 +2,15 @@
 // 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 tracins.
+// Package tag adds support for telemetry tracing.
 package trace
 
 import (
 	"context"
+
+	"golang.org/x/tools/internal/lsp/telemetry/tag"
 )
 
-type Span interface {
+func StartSpan(ctx context.Context, name string, tags ...tag.Tag) (context.Context, func()) {
-	AddAttributes(attributes ...Attribute)
+	return tag.With(ctx, tags...), func() {}
-	AddMessageReceiveEvent(messageID, uncompressedByteSize, compressedByteSize int64)
-	AddMessageSendEvent(messageID, uncompressedByteSize, compressedByteSize int64)
-	Annotate(attributes []Attribute, str string)
-	Annotatef(attributes []Attribute, format string, a ...interface{})
-	End()
-	IsRecordingEvents() bool
-	SetName(name string)
-	SetStatus(status Status)
 }
-
-type Attribute interface{}
-
-type Status struct {
-	Code    int32
-	Message string
-}
-
-type nullSpan struct{}
-
-func (nullSpan) AddAttributes(attributes ...Attribute)                                            {}
-func (nullSpan) AddMessageReceiveEvent(messageID, uncompressedByteSize, compressedByteSize int64) {}
-func (nullSpan) AddMessageSendEvent(messageID, uncompressedByteSize, compressedByteSize int64)    {}
-func (nullSpan) Annotate(attributes []Attribute, str string)                                      {}
-func (nullSpan) Annotatef(attributes []Attribute, format string, a ...interface{})                {}
-func (nullSpan) End()                                                                             {}
-func (nullSpan) IsRecordingEvents() bool                                                          { return false }
-func (nullSpan) SetName(name string)                                                              {}
-func (nullSpan) SetStatus(status Status)                                                          {}
-
-var (
-	FromContext = func(ctx context.Context) Span { return nullSpan{} }
-	NewContext  = func(ctx context.Context, span Span) context.Context { return ctx }
-	StartSpan   = func(ctx context.Context, name string, options ...interface{}) (context.Context, Span) {
-		return ctx, nullSpan{}
-	}
-	BoolAttribute    = func(key string, value bool) Attribute { return nil }
-	Float64Attribute = func(key string, value float64) Attribute { return nil }
-	Int64Attribute   = func(key string, value int64) Attribute { return nil }
-	StringAttribute  = func(key string, value string) Attribute { return nil }
-	WithSpanKind     = func(spanKind int) interface{} { return nil }
-)
-
-const (
-	SpanKindUnspecified = iota
-	SpanKindServer
-	SpanKindClient
-)

internal/lsp/text_synchronization.go

@@ -66,8 +66,8 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo
 	go func() {
 		ctx := view.BackgroundContext()
 		//TODO: connect the remote span?
-		ctx, ts := trace.StartSpan(ctx, "lsp:background-worker")
+		ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
-		defer ts.End()
+		defer done()
 		s.Diagnostics(ctx, view, uri)
 	}()
 	return nil