Skip to content

Commit

Permalink
improve testing logger (#5346)
Browse files Browse the repository at this point in the history
* add loggertesting package

move logger.NewTesting to loggertesting.New
add loggertesting.PrintObservedLogs to pretty print logs from a observer.ObservedLogs
  • Loading branch information
AndersonQ authored Sep 6, 2024
1 parent 46532d2 commit 0126540
Show file tree
Hide file tree
Showing 28 changed files with 179 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,15 @@ import (

"github.com/elastic/elastic-agent-client/v7/pkg/client"
"github.com/elastic/elastic-agent-client/v7/pkg/proto"
"github.com/elastic/elastic-agent/pkg/control/v2/cproto"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/internal/pkg/core/monitoring/config"
"github.com/elastic/elastic-agent/internal/pkg/diagnostics"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi"
"github.com/elastic/elastic-agent/pkg/component"
"github.com/elastic/elastic-agent/pkg/component/runtime"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/control/v2/cproto"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
mockhandlers "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/agent/application/actions/handlers"
mockackers "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/fleetapi/acker"
)
Expand Down Expand Up @@ -81,7 +80,7 @@ func TestDiagnosticHandlerHappyPathWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{hook1})
Expand Down Expand Up @@ -162,7 +161,7 @@ func TestDiagnosticHandlerUploaderErrorWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
Expand Down Expand Up @@ -203,7 +202,7 @@ func TestDiagnosticHandlerZipArchiveErrorWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
Expand Down Expand Up @@ -239,7 +238,7 @@ func TestDiagnosticHandlerAckErrorWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
Expand Down Expand Up @@ -278,7 +277,7 @@ func TestDiagnosticHandlerCommitErrorWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
Expand Down Expand Up @@ -318,7 +317,7 @@ func TestDiagnosticHandlerContexteExpiredErrorWithLogs(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, observedLogs := logger.NewTesting("diagnostic-handler-test")
testLogger, observedLogs := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
Expand Down Expand Up @@ -362,7 +361,7 @@ func TestDiagnosticHandlerWithCPUProfile(t *testing.T) {

mockDiagProvider := mockhandlers.NewDiagnosticsProvider(t)
mockUploader := mockhandlers.NewUploader(t)
testLogger, _ := logger.NewTesting("diagnostic-handler-test")
testLogger, _ := loggertest.New("diagnostic-handler-test")
handler := NewDiagnostics(testLogger, tempAgentRoot, mockDiagProvider, defaultRateLimit, mockUploader)

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{hook1})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/fleetapi/client"
"github.com/elastic/elastic-agent/internal/pkg/remote"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
mockhandlers "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/agent/application/actions/handlers"
)

Expand Down Expand Up @@ -165,7 +166,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T
t.Run("policy with proxy config", func(t *testing.T) {
t.Run("rollback client changes when cannot create client",
func(t *testing.T) {
log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -221,7 +222,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T

t.Run("rollback client changes when cannot reach fleet-server",
func(t *testing.T) {
log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -277,7 +278,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T
})

t.Run("a new Hosts and no proxy changes the remote config", func(t *testing.T) {
log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -321,7 +322,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T
})

t.Run("a proxy changes the fleet client", func(t *testing.T) {
log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -371,7 +372,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T
func(t *testing.T) {
wantProxy := mockProxy.URL

log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -433,7 +434,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T
}))
defer alwaysErroringServer.Close()

log, _ := logger.NewTesting("TestPolicyChangeHandler")
log, _ := loggertest.New("TestPolicyChangeHandler")
var setterCalledCount int
setter := testSetter{SetClientFn: func(c client.Sender) {
setterCalledCount++
Expand Down Expand Up @@ -886,7 +887,7 @@ func TestPolicyChangeHandler_handlePolicyChange_FleetClientSettings(t *testing.T

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
log, logs := logger.NewTesting(tc.name)
log, logs := loggertest.New(tc.name)
defer func() {
if t.Failed() {
t.Log("test failed, see handler logs below:")
Expand Down Expand Up @@ -1081,7 +1082,7 @@ func TestPolicyChangeHandler_handlePolicyChange_LogLevelSet(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

log, _ := logger.NewTesting(tt.name)
log, _ := loggertest.New(tt.name)
mockLogLevelSetter := mockhandlers.NewLogLevelSetter(t)

if tt.setupExpectations != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
mockhandlers "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/agent/application/actions/handlers"
mockinfo "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/agent/application/info"
mockfleetacker "github.com/elastic/elastic-agent/testing/mocks/internal_/pkg/fleetapi/acker"
Expand Down Expand Up @@ -88,7 +89,7 @@ func TestSettings_SetLogLevel(t *testing.T) {
tt.setupMocks(t, mockLogLevelSetter, mockAgentInfo)
}

log, _ := logger.NewTesting(tt.name)
log, _ := loggertest.New(tt.name)

ctx := context.Background()

Expand Down Expand Up @@ -191,7 +192,7 @@ func TestSettings_handleLogLevel(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
log, _ := logger.NewTesting(tt.name)
log, _ := loggertest.New(tt.name)
mockAgentInfo := mockinfo.NewAgent(t)
mockLogLevelSetter := mockhandlers.NewLogLevelSetter(t)
mockAcker := mockfleetacker.NewAcker(t)
Expand Down
5 changes: 2 additions & 3 deletions internal/pkg/agent/application/apm_config_modifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ import (
"gopkg.in/yaml.v2"

"github.com/elastic/elastic-agent-client/v7/pkg/proto"

"github.com/elastic/elastic-agent/internal/pkg/config"
"github.com/elastic/elastic-agent/pkg/component"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
)

type injectedConfigAssertion func(*testing.T, []component.Component)
Expand Down Expand Up @@ -449,7 +448,7 @@ func TestPatchAPMConfig(t *testing.T) {
require.NoError(t, err)
agtConf, err := config.NewConfigFrom(tt.args.agentFileCfg)
require.NoError(t, err)
log, _ := logger.NewTesting(tt.name)
log, _ := loggertest.New(tt.name)
patcher := PatchAPMConfig(log, agtConf)

mcc := &mockConfigChange{c: fleetConf}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/agent/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/agent/application/info"
"github.com/elastic/elastic-agent/internal/pkg/config"
"github.com/elastic/elastic-agent/internal/pkg/testutils"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
"github.com/elastic/elastic-agent/pkg/limits"
)

Expand Down Expand Up @@ -48,7 +48,7 @@ func TestMergeFleetConfig(t *testing.T) {
}

func TestLimitsLog(t *testing.T) {
log, obs := logger.NewTesting("TestLimitsLog")
log, obs := loggertest.New("TestLimitsLog")
ctx, cn := context.WithCancel(context.Background())
defer cn()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
"github.com/elastic/elastic-agent/pkg/component"
"github.com/elastic/elastic-agent/pkg/component/runtime"
agentclient "github.com/elastic/elastic-agent/pkg/control/v2/client"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
"github.com/elastic/elastic-agent/pkg/utils/broadcaster"
)

Expand Down Expand Up @@ -338,7 +338,7 @@ func TestCoordinatorReportsInvalidPolicy(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()

log, obs := logger.NewTesting("")
log, obs := loggertest.New("")
defer func() {
if t.Failed() {
t.Log("test failed, coordinator logs below:")
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/agent/application/dispatcher/dispatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/fleetapi"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi/acker"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi/acker/noop"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
)

type mockHandler struct {
Expand Down Expand Up @@ -756,7 +756,7 @@ func TestReportNextScheduledUpgrade(t *testing.T) {
detailsSetter := func(upgradeDetails *details.Details) {
actualDetails = upgradeDetails
}
log, obs := logger.NewTesting("report_next_upgrade_details")
log, obs := loggertest.New("report_next_upgrade_details")

d.reportNextScheduledUpgrade(test.actions, detailsSetter, log)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"context"
"encoding/json"
"fmt"

"io"
"net/http"
"net/url"
Expand All @@ -32,6 +31,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/scheduler"
agentclient "github.com/elastic/elastic-agent/pkg/control/v2/client"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
)

type clientCallbackFunc func(headers http.Header, body io.Reader) (*http.Response, error)
Expand Down Expand Up @@ -317,7 +317,7 @@ func TestFleetGateway(t *testing.T) {
scheduler := scheduler.NewStepper()
client := newTestingClient()

log, _ := logger.NewTesting("fleet_gateway")
log, _ := loggertest.New("fleet_gateway")

stateStore := newStateStore(t, log)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

aConfig "github.com/elastic/elastic-agent/internal/pkg/config"
monitoringCfg "github.com/elastic/elastic-agent/internal/pkg/core/monitoring/config"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
)

func TestReload(t *testing.T) {
Expand Down Expand Up @@ -78,7 +78,7 @@ agent.monitoring.enabled: false
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
fsc := &fakeServerController{}
log, _ := logger.NewTesting(tc.name)
log, _ := loggertest.New(tc.name)
cfg := &monitoringCfg.MonitoringConfig{
Enabled: tc.currEnabled,
MonitorMetrics: tc.currMetrics,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
agentlibsconfig "github.com/elastic/elastic-agent-libs/config"
"github.com/elastic/elastic-agent-libs/transport/httpcommon"
"github.com/elastic/elastic-agent/internal/pkg/config"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
)

func TestReload(t *testing.T) {
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestReload(t *testing.T) {
},
}

l, _ := logger.NewTesting("t")
l, _ := loggertest.New("t")
for _, tc := range testCases {
cfg := tc.initialConfig
reloader := NewReloader(cfg, l)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download"
"github.com/elastic/elastic-agent/internal/pkg/release"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
agtversion "github.com/elastic/elastic-agent/pkg/version"
"github.com/elastic/elastic-agent/testing/pgptest"
)
Expand Down Expand Up @@ -201,7 +202,7 @@ func TestVerify(t *testing.T) {

for _, tc := range tt {
t.Run(tc.Name, func(t *testing.T) {
log, obs := logger.NewTesting("TestVerify")
log, obs := loggertest.New("TestVerify")
targetDir := t.TempDir()

timeout := 30 * time.Second
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
agtversion "github.com/elastic/elastic-agent/pkg/version"

"github.com/docker/go-units"
Expand Down Expand Up @@ -109,7 +110,7 @@ func TestDownloadBodyError(t *testing.T) {
Architecture: "64",
}

log, obs := logger.NewTesting("downloader")
log, obs := loggertest.New("downloader")
upgradeDetails := details.NewDetails("8.12.0", details.StateRequested, "")
testClient := NewDownloaderWithClient(log, config, *client, upgradeDetails)
artifactPath, err := testClient.Download(context.Background(), beatSpec, version)
Expand Down Expand Up @@ -166,7 +167,7 @@ func TestDownloadLogProgressWithLength(t *testing.T) {
},
}

log, obs := logger.NewTesting("downloader")
log, obs := loggertest.New("downloader")
upgradeDetails := details.NewDetails("8.12.0", details.StateRequested, "")
testClient := NewDownloaderWithClient(log, config, *client, upgradeDetails)
artifactPath, err := testClient.Download(context.Background(), beatSpec, version)
Expand Down Expand Up @@ -249,7 +250,7 @@ func TestDownloadLogProgressWithoutLength(t *testing.T) {
},
}

log, obs := logger.NewTesting("downloader")
log, obs := loggertest.New("downloader")
upgradeDetails := details.NewDetails("8.12.0", details.StateRequested, "")
testClient := NewDownloaderWithClient(log, config, *client, upgradeDetails)
artifactPath, err := testClient.Download(context.Background(), beatSpec, version)
Expand Down Expand Up @@ -510,7 +511,7 @@ func TestDownloadVersion(t *testing.T) {
defer server.Close()

elasticClient := server.Client()
log, _ := logger.NewTesting("downloader")
log, _ := loggertest.New("downloader")
upgradeDetails := details.NewDetails(tt.args.version.String(), details.StateRequested, "")
config := tt.fields.config
config.SourceURI = server.URL
Expand Down
Loading

0 comments on commit 0126540

Please sign in to comment.