From fa055457156641b311b331af56f3d2b3b4d103af Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 10 May 2021 10:27:35 -0400 Subject: [PATCH] internal/lsp/debug: add a facility to track known bugs Sometimes users report issues related to edge cases in Gopls that aren't reproducible. In some of these cases, we end up guarding against conditions that shouldn't be possible, which is an unfortunately fragile solution. Add a new debug.Bug function to both annotate such branches as known bugs, and help find them when they reoccur. For now this just records them in the debug server, but in the future we could send the user a message to the effect of "hey, a known bug has occurred" for debug builds of gopls. Also included are some minor cosmetic fixes. Change-Id: I95df0caf2c81f430661cabd573ce8e338fa69934 Reviewed-on: https://go-review.googlesource.com/c/tools/+/318369 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Rebecca Stambler --- .../internal/regtest/misc/debugserver_test.go | 1 + .../event/export/prometheus/prometheus.go | 4 +- internal/lsp/debug/serve.go | 48 ++++++++++++++++++- internal/lsp/debug/tag/tag.go | 4 ++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/gopls/internal/regtest/misc/debugserver_test.go b/gopls/internal/regtest/misc/debugserver_test.go index 88cfded908..c0df87070c 100644 --- a/gopls/internal/regtest/misc/debugserver_test.go +++ b/gopls/internal/regtest/misc/debugserver_test.go @@ -1,6 +1,7 @@ // Copyright 2021 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 misc import ( diff --git a/internal/event/export/prometheus/prometheus.go b/internal/event/export/prometheus/prometheus.go index 847babcb89..0281f60a35 100644 --- a/internal/event/export/prometheus/prometheus.go +++ b/internal/event/export/prometheus/prometheus.go @@ -27,13 +27,13 @@ type Exporter struct { metrics []metric.Data } -func (e *Exporter) ProcessEvent(ctx context.Context, ev core.Event, ln label.Map) context.Context { +func (e *Exporter) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map) context.Context { if !event.IsMetric(ev) { return ctx } e.mu.Lock() defer e.mu.Unlock() - metrics := metric.Entries.Get(ln).([]metric.Data) + metrics := metric.Entries.Get(lm).([]metric.Data) for _, data := range metrics { name := data.Handle() // We keep the metrics in name sorted order so the page is stable and easy diff --git a/internal/lsp/debug/serve.go b/internal/lsp/debug/serve.go index 58e59b5f2a..d7dd5445d2 100644 --- a/internal/lsp/debug/serve.go +++ b/internal/lsp/debug/serve.go @@ -20,6 +20,7 @@ import ( "path/filepath" "runtime" rpprof "runtime/pprof" + "sort" "strconv" "strings" "sync" @@ -76,6 +77,46 @@ type State struct { mu sync.Mutex clients []*Client servers []*Server + + // bugs maps bug description -> formatted event + bugs map[string]string +} + +func Bug(ctx context.Context, desc string) { + labels := [3]label.Label{ + tag.Bug.Of(desc), + } + _, file, line, ok := runtime.Caller(1) + if ok { + labels[1] = tag.Callsite.Of(fmt.Sprintf("%s:%d", file, line)) + } + core.Export(ctx, core.MakeEvent(labels, nil)) +} + +type bug struct { + Description, Event string +} + +func (st *State) Bugs() []bug { + st.mu.Lock() + defer st.mu.Unlock() + var bugs []bug + for k, v := range st.bugs { + bugs = append(bugs, bug{k, v}) + } + sort.Slice(bugs, func(i, j int) bool { + return bugs[i].Description < bugs[j].Description + }) + return bugs +} + +func (st *State) recordBug(description, event string) { + st.mu.Lock() + defer st.mu.Unlock() + if st.bugs == nil { + st.bugs = make(map[string]string) + } + st.bugs[description] = event } // Caches returns the set of Cache objects currently being served. @@ -338,7 +379,7 @@ func (i *Instance) AddService(s protocol.Server, session *cache.Session) { stdlog.Printf("unable to find a Client to add the protocol.Server to") } -func getMemory(r *http.Request) interface{} { +func getMemory(_ *http.Request) interface{} { var m runtime.MemStats runtime.ReadMemStats(&m) return m @@ -636,6 +677,9 @@ func makeInstanceExporter(i *Instance) event.Exporter { } } } + if b := tag.Bug.Get(ev); b != "" { + i.State.recordBug(b, fmt.Sprintf("%v", ev)) + } return ctx } // StdTrace must be above export.Spans below (by convention, export @@ -766,6 +810,8 @@ var MainTmpl = template.Must(template.Must(BaseTemplate.Clone()).Parse(`
    {{range .State.Clients}}
  • {{template "clientlink" .Session.ID}}
    • {{end}}

Servers

    {{range .State.Servers}}
  • {{template "serverlink" .ID}}
    • {{end}}
+

Known bugs encountered

+
{{range .State.Bugs}}
{{.Description}}
{{.Event}}
{{end}}
{{end}} `)) diff --git a/internal/lsp/debug/tag/tag.go b/internal/lsp/debug/tag/tag.go index ff2f2ecd38..1d00038f0a 100644 --- a/internal/lsp/debug/tag/tag.go +++ b/internal/lsp/debug/tag/tag.go @@ -43,6 +43,10 @@ var ( ClientID = keys.NewString("client_id", "") Level = keys.NewInt("level", "The logging level") + + // Bug tracks occurrences of known bugs in the server. + Bug = keys.NewString("bug", "A bug has occurred") + Callsite = keys.NewString("callsite", "gopls function call site") ) var (