Merge cockroachdb#124027

124027: roachtest: prefix all logs with worker tags r=vidit-bhat a=vidit-bhat Previously, many of the runner logs didn't have the worker tags. This was inadequate because we couldn't break up the logs by `wX` tags to see what each worker was doing. This PR prefixes worker tags to the logs that were missing them. Epic: none Fixes: cockroachdb#114045 Release note: None Co-authored-by: Vidit Bhat <[email protected]>
Dedej-Bergin · May 14, 2024 · 1e9eabb · 1e9eabb
2 parents 3d2e415 + 0a2f34e
commit 1e9eabb
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 8 deletions.
diff --git a/pkg/cmd/roachtest/BUILD.bazel b/pkg/cmd/roachtest/BUILD.bazel
@@ -60,7 +60,6 @@ go_library(
         "//pkg/util/timeutil",
         "//pkg/util/version",
         "@com_github_cockroachdb_errors//:errors",
-        "@com_github_cockroachdb_logtags//:logtags",
         "@com_github_datadog_datadog_go//statsd",
         "@com_github_lib_pq//:pq",
         "@com_github_petermattis_goid//:goid",

diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go
@@ -1927,7 +1927,7 @@ func (c *clusterImpl) Get(
 	return errors.Wrap(roachprod.Get(ctx, l, c.MakeNodes(opts...), src, dest), "cluster.Get")
 }
 
-// Put a string into the specified file on the remote(s).
+// PutString into the specified file on the remote(s).
 func (c *clusterImpl) PutString(
 	ctx context.Context, content, dest string, mode os.FileMode, nodes ...option.Option,
 ) error {

diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go
@@ -51,7 +51,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/version"
 	"github.com/cockroachdb/errors"
-	"github.com/cockroachdb/logtags"
 	"github.com/petermattis/goid"
 )
 
@@ -363,21 +362,29 @@ func (r *testRunner) Run(
 		if err := r.stopper.RunAsyncTask(ctx, "worker", func(ctx context.Context) {
 			defer wg.Done()
 
-			err := r.runWorker(
-				ctx, fmt.Sprintf("w%d", i) /* name */, r.work, qp,
+			name := fmt.Sprintf("w%d", i)
+			formattedPrefix := fmt.Sprintf("[%s] ", name)
+			childLogger, err := l.ChildLogger(name, logger.LogPrefix(formattedPrefix))
+			if err != nil {
+				l.ErrorfCtx(ctx, "unable to create logger %s: %s", name, err)
+				childLogger = l
+			}
+
+			err = r.runWorker(
+				ctx, name, r.work, qp,
 				r.stopper.ShouldQuiesce(),
 				clusterFactory,
 				clustersOpt,
 				lopt,
 				topt,
-				l,
+				childLogger,
 				n*count,
 			)
 
 			if err != nil {
 				// A worker returned an error. Let's shut down.
 				msg := fmt.Sprintf("Worker %d returned with error. Quiescing. Error: %v", i, err)
-				shout(ctx, l, lopt.stdout, msg)
+				shout(ctx, childLogger, lopt.stdout, msg)
 				errs.AddErr(err)
 				// Stop the stopper. This will cause all workers to not pick up more
 				// tests after finishing the currently running one. We add one to the
@@ -557,7 +564,6 @@ func (r *testRunner) runWorker(
 ) error {
 	stdout := lopt.stdout
 
-	ctx = logtags.AddTag(ctx, name, nil /* value */)
 	wStatus := r.addWorker(ctx, name)
 	defer func() {
 		r.removeWorker(ctx, name)