internal/telemetry: delete the event.TagOf method

It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.

Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index a517223..cd1a718 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -343,7 +343,7 @@
 	for _, diag := range diagnostics {
 		srcErr, err := sourceError(ctx, fset, pkg, diag)
 		if err != nil {
-			event.Error(ctx, "unable to compute analysis error position", err, event.TagOf("category", diag.Category), tag.Package.Of(pkg.ID()))
+			event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(pkg.ID()))
 			continue
 		}
 		data.diagnostics = append(data.diagnostics, srcErr)
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index d7b5b58..a778c88 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -90,13 +90,13 @@
 		return ctx.Err()
 	}
 
-	event.Print(ctx, "go/packages.Load", event.TagOf("snapshot", s.ID()), event.TagOf("query", query), event.TagOf("packages", len(pkgs)))
+	event.Print(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
 	if len(pkgs) == 0 {
 		return err
 	}
 	for _, pkg := range pkgs {
 		if !containsDir || s.view.Options().VerboseOutput {
-			event.Print(ctx, "go/packages.Load", event.TagOf("snapshot", s.ID()), event.TagOf("package", pkg.PkgPath), event.TagOf("files", pkg.CompiledGoFiles))
+			event.Print(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.PackagePath.Of(pkg.PkgPath), tag.Files.Of(pkg.CompiledGoFiles))
 		}
 		// Ignore packages with no sources, since we will never be able to
 		// correctly invalidate that metadata.
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 933ec40..2feb4c0 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -321,7 +321,7 @@
 	// We don't have a context handy to use for logging, so use the stdlib for now.
 	event.Print(v.baseCtx, "background imports cache refresh starting")
 	err := imports.PrimeCache(context.Background(), env)
-	event.Print(v.baseCtx, fmt.Sprintf("background refresh finished after %v", time.Since(start)), event.TagOf("Error", err))
+	event.Print(v.baseCtx, fmt.Sprintf("background refresh finished after %v", time.Since(start)), event.Err.Of(err))
 
 	v.importsMu.Lock()
 	v.cacheRefreshDuration = time.Since(start)
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index e9331dd..0be6e36 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -9,6 +9,7 @@
 	"fmt"
 	"strings"
 
+	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/telemetry/event"
@@ -29,7 +30,7 @@
 	}
 
 	if err != nil {
-		event.Print(ctx, "no completions found", event.TagOf("At", params.Position), event.TagOf("Failure", err))
+		event.Print(ctx, "no completions found", tag.Position.Of(params.Position), event.Err.Of("Failure"))
 	}
 	if candidates == nil {
 		return &protocol.CompletionList{
diff --git a/internal/lsp/debug/serve.go b/internal/lsp/debug/serve.go
index 7f8e87a..315eb4f 100644
--- a/internal/lsp/debug/serve.go
+++ b/internal/lsp/debug/serve.go
@@ -27,6 +27,7 @@
 	"sync"
 	"time"
 
+	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/telemetry/event"
@@ -462,7 +463,7 @@
 	if strings.HasSuffix(i.DebugAddress, ":0") {
 		log.Printf("debug server listening on port %d", port)
 	}
-	event.Print(ctx, "Debug serving", event.TagOf("Port", port))
+	event.Print(ctx, "Debug serving", tag.Port.Of(port))
 	go func() {
 		mux := http.NewServeMux()
 		mux.HandleFunc("/", render(mainTmpl, func(*http.Request) interface{} { return i }))
diff --git a/internal/lsp/debug/tag/tag.go b/internal/lsp/debug/tag/tag.go
index d4e9219..389fd90 100644
--- a/internal/lsp/debug/tag/tag.go
+++ b/internal/lsp/debug/tag/tag.go
@@ -26,6 +26,14 @@
 	Query         = &event.Key{Name: "query"}
 	Snapshot      = &event.Key{Name: "snapshot"}
 	Operation     = &event.Key{Name: "operation"}
+
+	Position     = &event.Key{Name: "position"}
+	Category     = &event.Key{Name: "category"}
+	PackageCount = &event.Key{Name: "packages"}
+	Files        = &event.Key{Name: "files"}
+	Port         = &event.Key{Name: "port"}
+	Type         = &event.Key{Name: "type"}
+	HoverKind    = &event.Key{Name: "hoverkind"}
 )
 
 var (
diff --git a/internal/lsp/signature_help.go b/internal/lsp/signature_help.go
index e4bb094..8bf43f6 100644
--- a/internal/lsp/signature_help.go
+++ b/internal/lsp/signature_help.go
@@ -7,6 +7,7 @@
 import (
 	"context"
 
+	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/telemetry/event"
@@ -19,7 +20,7 @@
 	}
 	info, activeParameter, err := source.SignatureHelp(ctx, snapshot, fh, params.Position)
 	if err != nil {
-		event.Print(ctx, "no signature help", event.TagOf("At", params.Position), event.TagOf("Failure", err))
+		event.Print(ctx, "no signature help", tag.Position.Of(params.Position), event.Err.Of(err))
 		return nil, nil
 	}
 	return &protocol.SignatureHelp{
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 5144ba5..00b5ac3 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -293,7 +293,7 @@
 		cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4}
 		b := &bytes.Buffer{}
 		if err := cfg.Fprint(b, v.Session().Cache().FileSet(), p.Type); err != nil {
-			event.Error(ctx, "unable to print type", nil, event.TagOf("Type", p.Type))
+			event.Error(ctx, "unable to print type", nil, tag.Type.Of(p.Type))
 			continue
 		}
 		typ := replacer.Replace(b.String())
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 5c1eb17..72c3729 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -37,10 +37,10 @@
 	"golang.org/x/tools/go/analysis/passes/unreachable"
 	"golang.org/x/tools/go/analysis/passes/unsafeptr"
 	"golang.org/x/tools/go/analysis/passes/unusedresult"
+	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/diff/myers"
 	"golang.org/x/tools/internal/lsp/protocol"
-	"golang.org/x/tools/internal/telemetry/event"
 	errors "golang.org/x/xerrors"
 )
 
@@ -354,7 +354,7 @@
 		case "Structured":
 			o.HoverKind = Structured
 		default:
-			result.errorf("Unsupported hover kind", event.TagOf("HoverKind", hoverKind))
+			result.errorf("Unsupported hover kind", tag.HoverKind.Of(hoverKind))
 		}
 
 	case "linkTarget":
diff --git a/internal/telemetry/event/key.go b/internal/telemetry/event/key.go
index d24fff6..bf9f50d 100644
--- a/internal/telemetry/event/key.go
+++ b/internal/telemetry/event/key.go
@@ -17,12 +17,6 @@
 	Description string
 }
 
-// TagOf returns a Tag for a key and value.
-// This is a trivial helper that makes common logging easier to read.
-func TagOf(name string, value interface{}) Tag {
-	return Tag{Key: &Key{Name: name}, Value: value}
-}
-
 // Of creates a new Tag with this key and the supplied value.
 // You can use this when building a tag list.
 func (k *Key) Of(v interface{}) Tag {