internal/lsp: use a background context for the background worker

A detatched context ends up attributing all background work to the initialize
function.

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

internal/jsonrpc2/jsonrpc2.go

@@ -305,7 +305,7 @@ type combined struct {
 // caused the termination.
 // It must be called exactly once for each Conn.
 // It returns only when the reader is closed or there is an error in the stream.
-func (c *Conn) Run(ctx context.Context) error {
+func (c *Conn) Run(runCtx context.Context) error {
 	// we need to make the next request "lock" in an unlocked state to allow
 	// the first incoming request to proceed. All later requests are unlocked
 	// by the preceding request going to parallel mode.
@@ -313,7 +313,7 @@ func (c *Conn) Run(ctx context.Context) error {
 	close(nextRequest)
 	for {
 		// get the data for a message
-		data, n, err := c.stream.Read(ctx)
+		data, n, err := c.stream.Read(runCtx)
 		if err != nil {
 			// the stream failed, we cannot continue
 			return err
@@ -324,7 +324,7 @@ func (c *Conn) Run(ctx context.Context) error {
 			// a badly formed message arrived, log it and continue
 			// we trust the stream to have isolated the error to just this message
 			for _, h := range c.handlers {
-				h.Error(ctx, fmt.Errorf("unmarshal failed: %v", err))
+				h.Error(runCtx, fmt.Errorf("unmarshal failed: %v", err))
 			}
 			continue
 		}
@@ -332,7 +332,7 @@ func (c *Conn) Run(ctx context.Context) error {
 		switch {
 		case msg.Method != "":
 			// if method is set it must be a request
-			ctx, cancelReq := context.WithCancel(ctx)
+			reqCtx, cancelReq := context.WithCancel(runCtx)
 			thisRequest := nextRequest
 			nextRequest = make(chan struct{})
 			req := &Request{
@@ -347,8 +347,8 @@ func (c *Conn) Run(ctx context.Context) error {
 				},
 			}
 			for _, h := range c.handlers {
-				ctx = h.Request(ctx, Receive, &req.WireRequest)
-				ctx = h.Read(ctx, n)
+				reqCtx = h.Request(reqCtx, Receive, &req.WireRequest)
+				reqCtx = h.Read(reqCtx, n)
 			}
 			c.setHandling(req, true)
 			go func() {
@@ -357,17 +357,17 @@ func (c *Conn) Run(ctx context.Context) error {
 				defer func() {
 					c.setHandling(req, false)
 					if !req.IsNotify() && req.state < requestReplied {
-						req.Reply(ctx, nil, NewErrorf(CodeInternalError, "method %q did not reply", req.Method))
+						req.Reply(reqCtx, nil, NewErrorf(CodeInternalError, "method %q did not reply", req.Method))
 					}
 					req.Parallel()
 					for _, h := range c.handlers {
-						h.Done(ctx, err)
+						h.Done(reqCtx, err)
 					}
 					cancelReq()
 				}()
 				delivered := false
 				for _, h := range c.handlers {
-					if h.Deliver(ctx, req, delivered) {
+					if h.Deliver(reqCtx, req, delivered) {
 						delivered = true
 					}
 				}
@@ -390,7 +390,7 @@ func (c *Conn) Run(ctx context.Context) error {
 			close(rchan)
 		default:
 			for _, h := range c.handlers {
-				h.Error(ctx, fmt.Errorf("message not a call, notify or response, ignoring"))
+				h.Error(runCtx, fmt.Errorf("message not a call, notify or response, ignoring"))
 			}
 		}
 	}

internal/lsp/cache/session.go

@@ -16,6 +16,7 @@ import (
 
 	"golang.org/x/tools/internal/lsp/debug"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/lsp/telemetry/trace"
 	"golang.org/x/tools/internal/lsp/xlog"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/xcontext"
@@ -67,12 +68,14 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI) sou
 	index := atomic.AddInt64(&viewIndex, 1)
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
-	ctx = xcontext.Detach(ctx)
-	backgroundCtx, cancel := context.WithCancel(ctx)
+	// We want a true background context and not a detatched context here
+	// the spans need to be unrelated and no tag values should pollute it.
+	baseCtx := trace.Detach(xcontext.Detach(ctx))
+	backgroundCtx, cancel := context.WithCancel(baseCtx)
 	v := &view{
 		session:       s,
 		id:            strconv.FormatInt(index, 10),
-		baseCtx:       ctx,
+		baseCtx:       baseCtx,
 		backgroundCtx: backgroundCtx,
 		cancel:        cancel,
 		name:          name,

internal/lsp/telemetry/trace/trace.go

@@ -14,3 +14,7 @@ import (
 func StartSpan(ctx context.Context, name string, tags ...tag.Tag) (context.Context, func()) {
 	return tag.With(ctx, tags...), func() {}
 }
+
+// Detach returns a context without an associated span.
+// This allows the creation of spans that are not children of the current span.
+func Detach(ctx context.Context) context.Context { return ctx }