From 4457e4cfd49e5ab5e998d5829ac6530c56bcb5b9 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Fri, 21 Jun 2019 16:28:23 -0400 Subject: [PATCH] internal/lsp: add some trace spans to important functions This uses the new opencensus compatability layer to add telementry to some of the functions in the lsp, in order to allow us to understand their costs and call patterns. Change-Id: I7df820cd4eace7a4840ac6397d5df402369bf0a7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/183419 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/external.go | 3 +++ internal/lsp/cache/parse.go | 3 +++ internal/lsp/source/analysis.go | 3 +++ internal/lsp/source/completion.go | 3 +++ internal/lsp/source/format.go | 7 +++++++ internal/lsp/source/highlight.go | 3 +++ internal/lsp/source/hover.go | 6 ++++++ internal/lsp/source/identifier.go | 3 +++ internal/lsp/source/references.go | 3 +++ internal/lsp/source/rename.go | 3 +++ internal/lsp/source/signature_help.go | 3 +++ internal/lsp/source/symbols.go | 3 +++ internal/lsp/symbols.go | 3 +++ internal/lsp/text_synchronization.go | 4 ++++ 14 files changed, 50 insertions(+) diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go index ddb73675..5047a49f 100644 --- a/internal/lsp/cache/external.go +++ b/internal/lsp/cache/external.go @@ -10,6 +10,7 @@ import ( "os" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -50,6 +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") + defer ts.End() //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 { diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index ed3a2d8b..0d485345 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -13,6 +13,7 @@ import ( "go/token" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/memoize" ) @@ -73,6 +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") + defer ts.End() buf, _, err := fh.Read(ctx) if err != nil { return nil, err diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go index c599e674..754cdedb 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/source/analysis.go @@ -20,9 +20,12 @@ import ( "golang.org/x/sync/errgroup" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/internal/lsp/telemetry/trace" ) func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis.Analyzer) ([]*Action, error) { + ctx, ts := trace.StartSpan(ctx, "source.analyze") + defer ts.End() if ctx.Err() != nil { return nil, ctx.Err() } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 805c441a..8bf7e21b 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -13,6 +13,7 @@ import ( "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/internal/lsp/snippet" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -263,6 +264,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") + defer ts.End() file := f.GetAST(ctx) if file == nil { return nil, nil, fmt.Errorf("no AST for %s", f.URI()) diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 07200a3c..398f3665 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -16,11 +16,14 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) // 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") + defer ts.End() file := f.GetAST(ctx) if file == nil { return nil, fmt.Errorf("no AST for %s", f.URI()) @@ -50,6 +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") + defer ts.End() data, _, err := f.Handle(ctx).Read(ctx) if err != nil { return nil, err @@ -128,6 +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") + defer ts.End() data, _, err := file.Handle(ctx).Read(ctx) if err != nil { file.View().Session().Logger().Errorf(ctx, "Cannot compute text edits: %v", err) diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index c67377ff..eb3a25c2 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -11,10 +11,13 @@ import ( "go/token" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) func Highlight(ctx context.Context, f GoFile, pos token.Pos) ([]span.Span, error) { + ctx, ts := trace.StartSpan(ctx, "source.Highlight") + defer ts.End() file := f.GetAST(ctx) if file == nil { return nil, fmt.Errorf("no AST for %s", f.URI()) diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index b66299ed..db412ef5 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -12,6 +12,8 @@ import ( "go/format" "go/types" "strings" + + "golang.org/x/tools/internal/lsp/telemetry/trace" ) type documentation struct { @@ -31,6 +33,8 @@ const ( ) func (i *IdentifierInfo) Hover(ctx context.Context, markdownSupported bool, hoverKind HoverKind) (string, error) { + ctx, ts := trace.StartSpan(ctx, "source.Hover") + defer ts.End() h, err := i.decl.hover(ctx) if err != nil { return "", err @@ -68,6 +72,8 @@ func formatDocumentation(hoverKind HoverKind, c *ast.CommentGroup) string { } func (d declaration) hover(ctx context.Context) (*documentation, error) { + ctx, ts := trace.StartSpan(ctx, "source.hover") + defer ts.End() obj := d.obj switch node := d.node.(type) { case *ast.GenDecl: diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 981019f6..061c229b 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -13,6 +13,7 @@ import ( "strconv" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -62,6 +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") + defer ts.End() file := f.GetAST(ctx) if file == nil { return nil, fmt.Errorf("no AST for %s", f.URI()) diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index cab6a21c..0b199e68 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -10,6 +10,7 @@ import ( "go/ast" "go/types" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -25,6 +26,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") + defer ts.End() var references []*ReferenceInfo // If the object declaration is nil, assume it is an import spec and do not look for references. diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 41668a80..885b9903 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -15,6 +15,7 @@ import ( "regexp" "golang.org/x/tools/go/types/typeutil" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" "golang.org/x/tools/refactor/satisfy" ) @@ -36,6 +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") + defer ts.End() if i.Name == newName { return nil, fmt.Errorf("old and new names are the same: %s", newName) } diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index ed232621..7a657b6b 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -12,6 +12,7 @@ import ( "go/types" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/lsp/telemetry/trace" ) type SignatureInformation struct { @@ -25,6 +26,8 @@ type ParameterInformation struct { } func SignatureHelp(ctx context.Context, f GoFile, pos token.Pos) (*SignatureInformation, error) { + ctx, ts := trace.StartSpan(ctx, "source.SignatureHelp") + defer ts.End() file := f.GetAST(ctx) if file == nil { return nil, fmt.Errorf("no AST for %s", f.URI()) diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 879be50d..28ff24e0 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -12,6 +12,7 @@ import ( "go/token" "go/types" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -41,6 +42,8 @@ type Symbol struct { } func DocumentSymbols(ctx context.Context, f GoFile) ([]Symbol, error) { + ctx, ts := trace.StartSpan(ctx, "source.DocumentSymbols") + defer ts.End() fset := f.FileSet() file := f.GetAST(ctx) if file == nil { diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go index 0a908a8c..c24fceb9 100644 --- a/internal/lsp/symbols.go +++ b/internal/lsp/symbols.go @@ -9,10 +9,13 @@ import ( "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSymbolParams) ([]protocol.DocumentSymbol, error) { + ctx, ts := trace.StartSpan(ctx, "lsp.Server.documentSymbol") + defer ts.End() uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 7fe8d81e..66f9a624 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -12,6 +12,7 @@ import ( "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/telemetry/trace" "golang.org/x/tools/internal/span" ) @@ -64,6 +65,9 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo // Run diagnostics on the newly-changed file. go func() { ctx := view.BackgroundContext() + //TODO: connect the remote span? + ctx, ts := trace.StartSpan(ctx, "lsp:background-worker") + defer ts.End() s.Diagnostics(ctx, view, uri) }() return nil