From 5965b314fc04a7704f800be677f6955333ec4ea1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?=
 <felix.geisendoerfer@datadoghq.com>
Date: Tue, 4 May 2021 13:32:00 +0200
Subject: [PATCH] internal/log: Improve API for testing (#916)

internal/log: Improve API for testing

I just noticed that d9472a0d caused the profiler package to always emit
some log output, regardless of `go test -v` being used or not. I'm very
sorry about this. IMO tests should never print to stdout unless
requested.

This patch is my attempt to fix this by making it easy for individual
tests to capture the log output (see UseLogger, RecordLogger). I've also
added DiscardLogger since it can be very convenient for tests that want
to ignore log output without capturing it. Arguably RecordLogger could
be used for this as well, but that would do a poor job of capturing the
intend of such code.
---
 internal/log/log.go       | 37 +++++++++++++++++++++++++++++++++++--
 profiler/profiler_test.go | 12 ++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/internal/log/log.go b/internal/log/log.go
index 4934617f5e..a887721c2d 100644
--- a/internal/log/log.go
+++ b/internal/log/log.go
@@ -36,12 +36,17 @@ var (
 	logger ddtrace.Logger = &defaultLogger{l: log.New(os.Stderr, "", log.LstdFlags)}
 )
 
-// UseLogger sets l as the active logger.
-func UseLogger(l ddtrace.Logger) {
+// UseLogger sets l as the active logger and returns a function to restore the
+// previous logger. The return value is mostly useful when testing.
+func UseLogger(l ddtrace.Logger) (undo func()) {
 	Flush()
 	mu.Lock()
 	defer mu.Unlock()
+	old := logger
 	logger = l
+	return func() {
+		logger = old
+	}
 }
 
 // SetLevel sets the given lvl for logging.
@@ -172,3 +177,31 @@ func printMsg(lvl, format string, a ...interface{}) {
 type defaultLogger struct{ l *log.Logger }
 
 func (p *defaultLogger) Log(msg string) { p.l.Print(msg) }
+
+// DiscardLogger discards every call to Log().
+type DiscardLogger struct{}
+
+// Log implements ddtrace.Logger.
+func (d DiscardLogger) Log(msg string) {}
+
+// RecordLogger records every call to Log() and makes it available via Logs().
+type RecordLogger struct {
+	m    sync.Mutex
+	logs []string
+}
+
+// Log implements ddtrace.Logger.
+func (r *RecordLogger) Log(msg string) {
+	r.m.Lock()
+	defer r.m.Unlock()
+	r.logs = append(r.logs, msg)
+}
+
+// Logs returns the ordered list of logs recorded by the logger.
+func (r *RecordLogger) Logs() []string {
+	r.m.Lock()
+	defer r.m.Unlock()
+	copied := make([]string, len(r.logs))
+	copy(copied, r.logs)
+	return copied
+}
diff --git a/profiler/profiler_test.go b/profiler/profiler_test.go
index b6fc4f756f..4e38cb54ed 100644
--- a/profiler/profiler_test.go
+++ b/profiler/profiler_test.go
@@ -15,6 +15,8 @@ import (
 	"testing"
 	"time"
 
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -59,13 +61,21 @@ func TestStart(t *testing.T) {
 	})
 
 	t.Run("options/GoodAPIKey/Agent", func(t *testing.T) {
+		rl := &log.RecordLogger{}
+		defer log.UseLogger(rl)()
+
 		err := Start(WithAPIKey("12345678901234567890123456789012"))
 		defer Stop()
 		assert.Nil(t, err)
 		assert.Equal(t, activeProfiler.cfg.agentURL, activeProfiler.cfg.targetURL)
+		assert.Equal(t, 1, len(rl.Logs()))
+		assert.Contains(t, rl.Logs()[0], "profiler.WithAPIKey")
 	})
 
 	t.Run("options/GoodAPIKey/Agentless", func(t *testing.T) {
+		rl := &log.RecordLogger{}
+		defer log.UseLogger(rl)()
+
 		err := Start(
 			WithAPIKey("12345678901234567890123456789012"),
 			WithAgentlessUpload(),
@@ -73,6 +83,8 @@ func TestStart(t *testing.T) {
 		defer Stop()
 		assert.Nil(t, err)
 		assert.Equal(t, activeProfiler.cfg.apiURL, activeProfiler.cfg.targetURL)
+		assert.Equal(t, 1, len(rl.Logs()))
+		assert.Contains(t, rl.Logs()[0], "profiler.WithAgentlessUpload")
 	})
 
 	t.Run("options/BadAPIKey", func(t *testing.T) {