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>

internal/lsp/cache/analysis.go

@@ -343,7 +343,7 @@ func runAnalysis(ctx context.Context, fset *token.FileSet, analyzer *analysis.An
 	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)

internal/lsp/cache/load.go

@@ -90,13 +90,13 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 		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.

internal/lsp/cache/view.go

@@ -321,7 +321,7 @@ func (v *view) refreshProcessEnv() {
 	// 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)

internal/lsp/completion.go

@@ -9,6 +9,7 @@ import (
 	"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 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	}
 
 	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{

internal/lsp/debug/serve.go

@@ -27,6 +27,7 @@ import (
 	"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 @@ func (i *Instance) Serve(ctx context.Context) error {
 	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 }))

internal/lsp/debug/tag/tag.go

@@ -26,6 +26,14 @@ var (
 	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 (

internal/lsp/signature_help.go

@@ -7,6 +7,7 @@ package lsp
 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 @@ func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHe
 	}
 	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{

internal/lsp/source/completion_format.go

@@ -293,7 +293,7 @@ func formatFieldList(ctx context.Context, v View, list *ast.FieldList) ([]string
 		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())

internal/lsp/source/options.go

@@ -37,10 +37,10 @@ import (
 	"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 @@ func (o *Options) set(name string, value interface{}) OptionResult {
 		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":

internal/telemetry/event/key.go

@@ -17,12 +17,6 @@ type Key struct {
 	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 {