diff --git a/updater/cmd/updater/main.go b/updater/cmd/updater/main.go index be21574d9..9d5204101 100644 --- a/updater/cmd/updater/main.go +++ b/updater/cmd/updater/main.go @@ -24,6 +24,8 @@ import ( "github.com/observiq/observiq-otel-collector/packagestate" "github.com/observiq/observiq-otel-collector/updater/internal/install" + "github.com/observiq/observiq-otel-collector/updater/internal/logging" + "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/observiq/observiq-otel-collector/updater/internal/rollback" "github.com/observiq/observiq-otel-collector/updater/internal/state" "github.com/observiq/observiq-otel-collector/updater/internal/version" @@ -32,9 +34,8 @@ import ( "go.uber.org/zap" ) -// Unimplemented func main() { - var showVersion = pflag.BoolP("version", "v", false, "Prints the version of the collector and exits, if specified.") + var showVersion = pflag.BoolP("version", "v", false, "Prints the version of the updater and exits, if specified.") var tmpDir = pflag.String("tmpdir", "", "Temporary directory for artifacts. Parent of the 'rollback' directory.") pflag.Parse() @@ -51,58 +52,71 @@ func main() { os.Exit(1) } + // We can't create the zap logger yet, because we don't know the install dir, which is needed + // to create the logger. So we pass a Nop logger here. + installDir, err := path.InstallDir(zap.NewNop()) + if err != nil { + log.Fatalf("Failed to determine install directory: %s", err) + } + + logger, err := logging.NewLogger(installDir) + if err != nil { + log.Fatalf("Failed to create logger: %s", err) + } + // Create a monitor and load the package status file - // TODO replace nop logger with real one - monitor, err := state.NewCollectorMonitor(zap.NewNop()) + monitor, err := state.NewCollectorMonitor(logger) if err != nil { - log.Fatalln("Failed to create monitor:", err) + logger.Fatal("Failed to create monitor", zap.Error(err)) } - installer, err := install.NewInstaller(*tmpDir) + installer, err := install.NewInstaller(logger, *tmpDir) if err != nil { - log.Fatalf("Failed to create installer: %s", err) + logger.Fatal("Failed to create installer", zap.Error(err)) } - rb, err := rollback.NewRollbacker(*tmpDir) + rb, err := rollback.NewRollbacker(logger, *tmpDir) if err != nil { - log.Fatalf("Failed to create rollbacker: %s", err) + logger.Fatal("Failed to create rollbacker", zap.Error(err)) } if err := rb.Backup(); err != nil { - log.Fatalf("Failed to backup: %s", err) + logger.Fatal("Failed to backup", zap.Error(err)) } if err := installer.Install(rb); err != nil { - log.Default().Printf("Failed to install: %s", err) + logger.Error("Failed to install", zap.Error(err)) // Set the state to failed before rollback so collector knows it failed if setErr := monitor.SetState(packagestate.DefaultFileName, protobufs.PackageStatus_InstallFailed, err); setErr != nil { - log.Println("Failed to set state on install failure:", setErr) + logger.Error("Failed to set state on install failure", zap.Error(setErr)) } rb.Rollback() - log.Default().Fatalf("Rollback complete") + logger.Fatal("Rollback complete") } // Create a context with timeout to wait for a success or failed status checkCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + logger.Debug("Installation successful, begin monitor for success") + // Monitor the install state if err := monitor.MonitorForSuccess(checkCtx, packagestate.DefaultFileName); err != nil { - log.Println("Failed to install:", err) + logger.Error("Failed to install", zap.Error(err)) // If this is not an error due to the collector setting a failed status we need to set a failed status if !errors.Is(err, state.ErrFailedStatus) { // Set the state to failed before rollback so collector knows it failed if setErr := monitor.SetState(packagestate.DefaultFileName, protobufs.PackageStatus_InstallFailed, err); setErr != nil { - log.Println("Failed to set state on install failure:", setErr) + logger.Error("Failed to set state on install failure", zap.Error(setErr)) } } rb.Rollback() - log.Fatalln("Rollback complete") + logger.Fatal("Rollback complete") } // Successful update - log.Println("Update Complete") + logger.Info("Update Complete") } diff --git a/updater/go.mod b/updater/go.mod index 32c25c7ab..c4232e5d6 100644 --- a/updater/go.mod +++ b/updater/go.mod @@ -13,6 +13,7 @@ require ( ) require ( + github.com/benbjohnson/clock v1.1.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.4.0 // indirect diff --git a/updater/go.sum b/updater/go.sum index fdb019d02..d0465dead 100644 --- a/updater/go.sum +++ b/updater/go.sum @@ -1,11 +1,15 @@ github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= +github.com/cenkalti/backoff/v4 v4.1.2/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= @@ -13,10 +17,13 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/oklog/ulid/v2 v2.0.2/go.mod h1:mtBL0Qe/0HAx6/a4Z30qxVIAL1eQDweXq5lxOEiwQ68= github.com/open-telemetry/opamp-go v0.2.0 h1:dV7wTkG5XNiorU62N1CJPr3f5dM0PGEtUUBtvK+LEG0= github.com/open-telemetry/opamp-go v0.2.0/go.mod h1:IMdeuHGVc5CjKSu5/oNV0o+UmiXuahoHvoZ4GOmAI9M= +github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= @@ -35,6 +42,7 @@ go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/goleak v1.1.12 h1:gZAh5/EyT/HQwlpkCy6wTpqfH9H8Lz8zbm3dZh+OyzA= +go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= @@ -68,6 +76,7 @@ golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= +google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -75,6 +84,7 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/updater/internal/action/file_action.go b/updater/internal/action/file_action.go index d62f3ab81..abf367ab9 100644 --- a/updater/internal/action/file_action.go +++ b/updater/internal/action/file_action.go @@ -22,6 +22,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/file" "github.com/observiq/observiq-otel-collector/updater/internal/path" + "go.uber.org/zap" ) // CopyFileAction is an action that records a file being copied from FromPath to ToPath @@ -34,16 +35,17 @@ type CopyFileAction struct { // FileCreated is a bool that records whether this action had to create a new file or not FileCreated bool backupDir string - latestDir string + logger *zap.Logger } var _ RollbackableAction = (*CopyFileAction)(nil) +var _ fmt.Stringer = (*CopyFileAction)(nil) // NewCopyFileAction creates a new CopyFileAction that indicates a file was copied from // fromPathRel into toPath. tmpDir is specified for rollback purposes. // NOTE: This action MUST be created BEFORE the action actually takes place; This allows // for previous existence of the file to be recorded. -func NewCopyFileAction(fromPathRel, toPath, tmpDir string) (*CopyFileAction, error) { +func NewCopyFileAction(logger *zap.Logger, fromPathRel, toPath, tmpDir string) (*CopyFileAction, error) { fileExists := true _, err := os.Stat(toPath) switch { @@ -59,7 +61,7 @@ func NewCopyFileAction(fromPathRel, toPath, tmpDir string) (*CopyFileAction, err // The file will be created if it doesn't already exist FileCreated: !fileExists, backupDir: path.BackupDirFromTempDir(tmpDir), - latestDir: path.LatestDirFromTempDir(tmpDir), + logger: logger.Named("copy-file-action"), }, nil } @@ -74,9 +76,13 @@ func (c CopyFileAction) Rollback() error { // join the relative path to the backup directory to get the location of the backup path backupFilePath := filepath.Join(c.backupDir, c.FromPathRel) - if err := file.CopyFile(backupFilePath, c.ToPath, true); err != nil { + if err := file.CopyFile(c.logger.Named("copy-file"), backupFilePath, c.ToPath, true); err != nil { return fmt.Errorf("failed to copy file: %w", err) } return nil } + +func (c CopyFileAction) String() string { + return fmt.Sprintf("CopyFileAction{FromPathRel: '%s', ToPath: '%s', FileCreated: '%t'}", c.FromPathRel, c.ToPath, c.FileCreated) +} diff --git a/updater/internal/action/file_action_test.go b/updater/internal/action/file_action_test.go index c61a74238..080b875d5 100644 --- a/updater/internal/action/file_action_test.go +++ b/updater/internal/action/file_action_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestNewCopyFileAction(t *testing.T) { @@ -29,7 +30,7 @@ func TestNewCopyFileAction(t *testing.T) { outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testTempDir, "latest", "test.txt") - a, err := NewCopyFileAction(inFile, outFile, testTempDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testTempDir) require.NoError(t, err) require.Equal(t, &CopyFileAction{ @@ -37,7 +38,7 @@ func TestNewCopyFileAction(t *testing.T) { ToPath: outFile, FileCreated: true, backupDir: filepath.Join(testTempDir, "rollback"), - latestDir: filepath.Join(testTempDir, "latest"), + logger: a.logger, }, a) }) @@ -51,7 +52,7 @@ func TestNewCopyFileAction(t *testing.T) { require.NoError(t, err) require.NoError(t, f.Close()) - a, err := NewCopyFileAction(inFile, outFile, testTempDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testTempDir) require.NoError(t, err) require.Equal(t, &CopyFileAction{ @@ -59,7 +60,7 @@ func TestNewCopyFileAction(t *testing.T) { ToPath: outFile, FileCreated: false, backupDir: filepath.Join(testTempDir, "rollback"), - latestDir: filepath.Join(testTempDir, "latest"), + logger: a.logger, }, a) }) } @@ -71,7 +72,7 @@ func TestCopyFileActionRollback(t *testing.T) { outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testTempDir, "latest", "test.txt") - a, err := NewCopyFileAction(inFile, outFile, testTempDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testTempDir) require.NoError(t, err) inBytes, err := os.ReadFile(inFile) @@ -100,7 +101,7 @@ func TestCopyFileActionRollback(t *testing.T) { err = os.WriteFile(outFile, originalBytes, 0600) require.NoError(t, err) - a, err := NewCopyFileAction(inFileRel, outFile, testTempDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFileRel, outFile, testTempDir) require.NoError(t, err) // Overwrite original file with latest file @@ -135,7 +136,7 @@ func TestCopyFileActionRollback(t *testing.T) { err = os.WriteFile(outFile, originalBytes, 0600) require.NoError(t, err) - a, err := NewCopyFileAction(inFile, outFile, testTempDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testTempDir) require.NoError(t, err) // Overwrite original file with latest file diff --git a/updater/internal/action/service_action.go b/updater/internal/action/service_action.go index 000dab426..c45f10cac 100644 --- a/updater/internal/action/service_action.go +++ b/updater/internal/action/service_action.go @@ -17,6 +17,7 @@ package action import ( "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/observiq/observiq-otel-collector/updater/internal/service" + "go.uber.org/zap" ) // ServiceStopAction is an action that records that a service was stopped. @@ -65,9 +66,11 @@ type ServiceUpdateAction struct { var _ RollbackableAction = (*ServiceUpdateAction)(nil) // NewServiceUpdateAction creates a new ServiceUpdateAction -func NewServiceUpdateAction(tmpDir string) *ServiceUpdateAction { +func NewServiceUpdateAction(logger *zap.Logger, tmpDir string) *ServiceUpdateAction { + namedLogger := logger.Named("service-update-action") return &ServiceUpdateAction{ backupSvc: service.NewService( + namedLogger, "", // latestDir doesn't matter here service.WithServiceFile(path.BackupServiceFile(path.ServiceFileDir(path.BackupDirFromTempDir(tmpDir)))), ), diff --git a/updater/internal/action/service_action_test.go b/updater/internal/action/service_action_test.go index 42127b427..a67c6d5e5 100644 --- a/updater/internal/action/service_action_test.go +++ b/updater/internal/action/service_action_test.go @@ -19,6 +19,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/service/mocks" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestServiceStartAction(t *testing.T) { @@ -43,7 +44,7 @@ func TestServiceStopAction(t *testing.T) { func TestServiceUpdateAction(t *testing.T) { svc := mocks.NewService(t) - sua := NewServiceUpdateAction("./testdata") + sua := NewServiceUpdateAction(zaptest.NewLogger(t), "./testdata") sua.backupSvc = svc svc.On("Update").Once().Return(nil) diff --git a/updater/internal/file/file.go b/updater/internal/file/file.go index 468be246f..395f43b38 100644 --- a/updater/internal/file/file.go +++ b/updater/internal/file/file.go @@ -17,14 +17,15 @@ package file import ( "fmt" "io" - "log" "os" "path/filepath" + + "go.uber.org/zap" ) // CopyFile copies the file from pathIn to pathOut. // If the file does not exist, it is created. If the file does exist, it is truncated before writing. -func CopyFile(pathIn, pathOut string, overwrite bool) error { +func CopyFile(logger *zap.Logger, pathIn, pathOut string, overwrite bool) error { pathInClean := filepath.Clean(pathIn) // Open the input file for reading. @@ -35,7 +36,7 @@ func CopyFile(pathIn, pathOut string, overwrite bool) error { defer func() { err := inFile.Close() if err != nil { - log.Default().Printf("CopyFile: Failed to close input file: %s", err) + logger.Info("Failed to close input file", zap.Error(err)) } }() @@ -58,7 +59,7 @@ func CopyFile(pathIn, pathOut string, overwrite bool) error { defer func() { err := outFile.Close() if err != nil { - log.Default().Printf("CopyFile: Failed to close output file: %s", err) + logger.Info("Failed to close output file", zap.Error(err)) } }() diff --git a/updater/internal/file/file_test.go b/updater/internal/file/file_test.go index 5db7bea0f..5dc4c4cd3 100644 --- a/updater/internal/file/file_test.go +++ b/updater/internal/file/file_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestCopyFile(t *testing.T) { @@ -31,7 +32,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "test.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(inFile, outFile, true) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true) require.NoError(t, err) require.FileExists(t, outFile) @@ -63,7 +64,7 @@ func TestCopyFile(t *testing.T) { err = os.WriteFile(outFile, []byte("This is a file that already exists"), 0640) require.NoError(t, err) - err = CopyFile(inFile, outFile, true) + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true) require.NoError(t, err) require.FileExists(t, outFile) @@ -85,7 +86,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "does-not-exist.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(inFile, outFile, true) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true) require.ErrorContains(t, err, "failed to open input file") require.NoFileExists(t, outFile) }) @@ -99,7 +100,7 @@ func TestCopyFile(t *testing.T) { err := os.WriteFile(outFile, []byte("This is a file that already exists"), 0600) require.NoError(t, err) - err = CopyFile(inFile, outFile, true) + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true) require.ErrorContains(t, err, "failed to open input file") require.FileExists(t, outFile) @@ -117,7 +118,7 @@ func TestCopyFile(t *testing.T) { err := os.WriteFile(outFile, []byte("This is a file that already exists"), 0640) require.NoError(t, err) - err = CopyFile(inFile, outFile, false) + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, false) require.ErrorContains(t, err, "failed to open output file") require.FileExists(t, outFile) @@ -139,7 +140,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "test.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(inFile, outFile, false) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, false) require.NoError(t, err) require.FileExists(t, outFile) diff --git a/updater/internal/install/install.go b/updater/internal/install/install.go index 18de2900c..a6f21271c 100644 --- a/updater/internal/install/install.go +++ b/updater/internal/install/install.go @@ -25,6 +25,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/observiq/observiq-otel-collector/updater/internal/rollback" "github.com/observiq/observiq-otel-collector/updater/internal/service" + "go.uber.org/zap" ) // Installer allows you to install files from latestDir into installDir, @@ -34,21 +35,25 @@ type Installer struct { installDir string tmpDir string svc service.Service + logger *zap.Logger } // NewInstaller returns a new instance of an Installer. -func NewInstaller(tmpDir string) (*Installer, error) { +func NewInstaller(logger *zap.Logger, tmpDir string) (*Installer, error) { + namedLogger := logger.Named("installer") + latestDir := path.LatestDirFromTempDir(tmpDir) - installDirPath, err := path.InstallDir() + installDirPath, err := path.InstallDir(namedLogger.Named("install-dir")) if err != nil { return nil, fmt.Errorf("failed to determine install dir: %w", err) } return &Installer{ latestDir: latestDir, - svc: service.NewService(latestDir), + svc: service.NewService(namedLogger, latestDir), installDir: installDirPath, tmpDir: tmpDir, + logger: namedLogger, }, nil } @@ -60,31 +65,35 @@ func (i Installer) Install(rb rollback.ActionAppender) error { return fmt.Errorf("failed to stop service: %w", err) } rb.AppendAction(action.NewServiceStopAction(i.svc)) + i.logger.Debug("Service stopped") // install files that go to installDirPath to their correct location, // excluding any config files (logging.yaml, config.yaml, manager.yaml) - if err := copyFiles(i.latestDir, i.installDir, i.tmpDir, rb); err != nil { + if err := copyFiles(i.logger, i.latestDir, i.installDir, i.tmpDir, rb); err != nil { return fmt.Errorf("failed to install new files: %w", err) } + i.logger.Debug("Install artifacts copied") // Update old service config to new service config if err := i.svc.Update(); err != nil { return fmt.Errorf("failed to update service: %w", err) } - rb.AppendAction(action.NewServiceUpdateAction(i.tmpDir)) + rb.AppendAction(action.NewServiceUpdateAction(i.logger, i.tmpDir)) + i.logger.Debug("Updated service configuration") // Start service if err := i.svc.Start(); err != nil { return fmt.Errorf("failed to start service: %w", err) } rb.AppendAction(action.NewServiceStartAction(i.svc)) + i.logger.Debug("Service started") return nil } // copyFiles moves the file tree rooted at latestDirPath to installDirPath, // skipping configuration files. Appends CopyFileAction-s to the Rollbacker as it copies file. -func copyFiles(inputPath, outputPath, tmpDir string, rb rollback.ActionAppender) error { +func copyFiles(logger *zap.Logger, inputPath, outputPath, tmpDir string, rb rollback.ActionAppender) error { err := filepath.WalkDir(inputPath, func(inPath string, d fs.DirEntry, err error) error { switch { case err != nil: @@ -116,7 +125,7 @@ func copyFiles(inputPath, outputPath, tmpDir string, rb rollback.ActionAppender) // We create the action record here, because we want to record whether the file exists or not before // we open the file (which will end up creating the file). - cfa, err := action.NewCopyFileAction(relPath, outPath, tmpDir) + cfa, err := action.NewCopyFileAction(logger, relPath, outPath, tmpDir) if err != nil { return fmt.Errorf("failed to create copy file action: %w", err) } @@ -126,7 +135,7 @@ func copyFiles(inputPath, outputPath, tmpDir string, rb rollback.ActionAppender) // and we will want to roll that back if that is the case. rb.AppendAction(cfa) - if err := file.CopyFile(inPath, outPath, true); err != nil { + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, true); err != nil { return fmt.Errorf("failed to copy file: %w", err) } diff --git a/updater/internal/install/install_test.go b/updater/internal/install/install_test.go index bf9b0e844..1a3d2ec9d 100644 --- a/updater/internal/install/install_test.go +++ b/updater/internal/install/install_test.go @@ -26,6 +26,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/service/mocks" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestInstallArtifacts(t *testing.T) { @@ -38,6 +39,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } outDirConfig := filepath.Join(outDir, "config.yaml") @@ -76,6 +78,7 @@ func TestInstallArtifacts(t *testing.T) { contentsEqual(t, filepath.Join(outDir, "test-folder", "another-test.txt"), "This is a nested text file\n") copyTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), installer.tmpDir, @@ -84,6 +87,7 @@ func TestInstallArtifacts(t *testing.T) { copyTestTxtAction.FileCreated = true copyNestedTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), installer.tmpDir, @@ -95,7 +99,7 @@ func TestInstallArtifacts(t *testing.T) { action.NewServiceStopAction(svc), copyNestedTestTxtAction, copyTestTxtAction, - action.NewServiceUpdateAction(installer.tmpDir), + action.NewServiceUpdateAction(installer.logger, installer.tmpDir), action.NewServiceStartAction(svc), }, actions) }) @@ -108,6 +112,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } svc.On("Stop").Once().Return(errors.New("stop failed")) @@ -124,6 +129,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } svc.On("Stop").Once().Return(nil) @@ -138,6 +144,7 @@ func TestInstallArtifacts(t *testing.T) { err := installer.Install(rb) require.ErrorContains(t, err, "failed to update service") copyTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), installer.tmpDir, @@ -146,6 +153,7 @@ func TestInstallArtifacts(t *testing.T) { copyTestTxtAction.FileCreated = true copyNestedTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), installer.tmpDir, @@ -168,6 +176,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } svc.On("Stop").Once().Return(nil) @@ -184,6 +193,7 @@ func TestInstallArtifacts(t *testing.T) { require.ErrorContains(t, err, "failed to start service") copyTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), installer.tmpDir, @@ -192,6 +202,7 @@ func TestInstallArtifacts(t *testing.T) { copyTestTxtAction.FileCreated = true copyNestedTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), installer.tmpDir, @@ -203,7 +214,7 @@ func TestInstallArtifacts(t *testing.T) { action.NewServiceStopAction(svc), copyNestedTestTxtAction, copyTestTxtAction, - action.NewServiceUpdateAction(installer.tmpDir), + action.NewServiceUpdateAction(installer.logger, installer.tmpDir), }, actions) }) @@ -215,6 +226,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "non-existent-dir"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } svc.On("Stop").Once().Return(nil) @@ -241,6 +253,7 @@ func TestInstallArtifacts(t *testing.T) { latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, svc: svc, + logger: zaptest.NewLogger(t), } outDirConfig := filepath.Join(outDir, "config.yaml") @@ -270,6 +283,7 @@ func TestInstallArtifacts(t *testing.T) { t.Logf("Error: %s", err) copyTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), installer.tmpDir, @@ -278,6 +292,7 @@ func TestInstallArtifacts(t *testing.T) { copyTestTxtAction.FileCreated = false copyNestedTestTxtAction, err := action.NewCopyFileAction( + installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), installer.tmpDir, diff --git a/updater/internal/logging/logging.go b/updater/internal/logging/logging.go new file mode 100644 index 000000000..1dbc74a9b --- /dev/null +++ b/updater/internal/logging/logging.go @@ -0,0 +1,46 @@ +// Copyright observIQ, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging + +import ( + "fmt" + "os" + + "github.com/observiq/observiq-otel-collector/updater/internal/path" + "go.uber.org/zap" +) + +// NewLogger returns a new logger, that logs to the log directory relative to installDir. +// It deletes the previous log file, as well. +func NewLogger(installDir string) (*zap.Logger, error) { + logFile := path.LogFile(installDir) + + conf := zap.NewProductionConfig() + conf.OutputPaths = []string{ + logFile, + } + + err := os.RemoveAll(logFile) + if err != nil { + return nil, fmt.Errorf("failed to remove previous log file: %w", err) + } + + prodLogger, err := conf.Build() + if err != nil { + return nil, fmt.Errorf("failed to build logger: %w", err) + } + + return prodLogger, nil +} diff --git a/updater/internal/logging/logging_test.go b/updater/internal/logging/logging_test.go new file mode 100644 index 000000000..8911da880 --- /dev/null +++ b/updater/internal/logging/logging_test.go @@ -0,0 +1,79 @@ +// Copyright observIQ, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewLogger(t *testing.T) { + t.Run("Existing file is removed", func(t *testing.T) { + if runtime.GOOS == "windows" { + // Skip on windows, because the log file will still be open + // when the test attempts to remove the temp dir, which ends up making + // the test fail. + t.SkipNow() + } + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "log"), 0775)) + + updaterLogPath := filepath.Join(tmpDir, "log", "updater.log") + + initialBytes := []byte("Some existing bytes") + require.NoError(t, os.WriteFile(updaterLogPath, initialBytes, 0660)) + + logger, err := NewLogger(tmpDir) + require.NoError(t, err) + + currentBytes, err := os.ReadFile(updaterLogPath) + require.NoError(t, err) + + if bytes.HasPrefix(currentBytes, initialBytes) { + t.Fatalf("The log file was not deleted (current bytes: '%s')", currentBytes) + } + + logger.Info("This is a log message") + require.NoError(t, logger.Sync()) + + require.FileExists(t, updaterLogPath) + }) + + t.Run("Logger creates file if existing file does not exist", func(t *testing.T) { + if runtime.GOOS == "windows" { + // Skip on windows, because the log file will still be open + // when the test attempts to remove the temp dir, which ends up making + // the test fail. + t.SkipNow() + } + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "log"), 0775)) + + updaterLogPath := filepath.Join(tmpDir, "log", "updater.log") + + logger, err := NewLogger(tmpDir) + require.NoError(t, err) + + logger.Info("This is a log message") + require.NoError(t, logger.Sync()) + + require.FileExists(t, updaterLogPath) + }) +} diff --git a/updater/internal/path/path.go b/updater/internal/path/path.go index fdd8be108..510a065ea 100644 --- a/updater/internal/path/path.go +++ b/updater/internal/path/path.go @@ -21,6 +21,8 @@ const ( rollbackDirFragment = "rollback" serviceFileDirFragment = "install" serviceFileBackupFilename = "backup.service" + logDirFragment = "log" + logFile = "updater.log" ) // LatestDirFromTempDir gets the path to the "latest" dir, where the new artifacts are, @@ -44,3 +46,8 @@ func ServiceFileDir(installBaseDir string) string { func BackupServiceFile(serviceFileDir string) string { return filepath.Join(serviceFileDir, serviceFileBackupFilename) } + +// LogFile returns the full path to the log file for the updater +func LogFile(installDir string) string { + return filepath.Join(installDir, logDirFragment, logFile) +} diff --git a/updater/internal/path/path_darwin.go b/updater/internal/path/path_darwin.go index 566791e7b..75d5631e6 100644 --- a/updater/internal/path/path_darwin.go +++ b/updater/internal/path/path_darwin.go @@ -14,10 +14,12 @@ package path +import "go.uber.org/zap" + // DarwinInstallDir is the path to the install directory on Darwin. const DarwinInstallDir = "/opt/observiq-otel-collector" // InstallDir returns the filepath to the install directory -func InstallDir() (string, error) { +func InstallDir(_ *zap.Logger) (string, error) { return DarwinInstallDir, nil } diff --git a/updater/internal/path/path_linux.go b/updater/internal/path/path_linux.go index 8f07d49df..41377c621 100644 --- a/updater/internal/path/path_linux.go +++ b/updater/internal/path/path_linux.go @@ -14,10 +14,12 @@ package path +import "go.uber.org/zap" + // LinuxInstallDir is the install directory of the collector on linux. const LinuxInstallDir = "/opt/observiq-otel-collector" // InstallDir returns the filepath to the install directory -func InstallDir() (string, error) { +func InstallDir(_ *zap.Logger) (string, error) { return LinuxInstallDir, nil } diff --git a/updater/internal/path/path_test.go b/updater/internal/path/path_test.go index 2a9b9032f..e6c0fe08b 100644 --- a/updater/internal/path/path_test.go +++ b/updater/internal/path/path_test.go @@ -38,3 +38,8 @@ func TestBackupServiceFile(t *testing.T) { serviceFileDir := filepath.Join("tmp", "rollback", "install") require.Equal(t, filepath.Join(serviceFileDir, "backup.service"), BackupServiceFile(serviceFileDir)) } + +func TestLogFile(t *testing.T) { + installDir := filepath.Join("install") + require.Equal(t, filepath.Join(installDir, "log", "updater.log"), LogFile(installDir)) +} diff --git a/updater/internal/path/path_windows.go b/updater/internal/path/path_windows.go index 44fca1e81..55009405f 100644 --- a/updater/internal/path/path_windows.go +++ b/updater/internal/path/path_windows.go @@ -16,15 +16,15 @@ package path import ( "fmt" - "log" + "go.uber.org/zap" "golang.org/x/sys/windows/registry" ) const defaultProductName = "observIQ Distro for OpenTelemetry Collector" // InstallDirFromRegistry gets the installation dir of the given product from the Windows Registry -func InstallDirFromRegistry(productName string) (string, error) { +func InstallDirFromRegistry(logger *zap.Logger, productName string) (string, error) { // this key is created when installing using the MSI installer keyPath := fmt.Sprintf(`Software\Microsoft\Windows\CurrentVersion\Uninstall\%s`, productName) key, err := registry.OpenKey(registry.LOCAL_MACHINE, keyPath, registry.READ) @@ -34,7 +34,7 @@ func InstallDirFromRegistry(productName string) (string, error) { defer func() { err := key.Close() if err != nil { - log.Default().Printf("InstallDirFromRegistry: failed to close registry key") + logger.Error("InstallDirFromRegistry: failed to close registry key", zap.Error(err)) } }() @@ -48,6 +48,6 @@ func InstallDirFromRegistry(productName string) (string, error) { } // InstallDir returns the filepath to the install directory -func InstallDir() (string, error) { - return InstallDirFromRegistry(defaultProductName) +func InstallDir(logger *zap.Logger) (string, error) { + return InstallDirFromRegistry(logger, defaultProductName) } diff --git a/updater/internal/rollback/rollback.go b/updater/internal/rollback/rollback.go index e94220dd8..e14e71d09 100644 --- a/updater/internal/rollback/rollback.go +++ b/updater/internal/rollback/rollback.go @@ -17,7 +17,6 @@ package rollback import ( "fmt" "io/fs" - "log" "os" "path/filepath" "strings" @@ -26,6 +25,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/file" "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/observiq/observiq-otel-collector/updater/internal/service" + "go.uber.org/zap" ) // ActionAppender is an interface that allows actions to be appended to it. @@ -42,11 +42,14 @@ type Rollbacker struct { installDir string tmpDir string actions []action.RollbackableAction + logger *zap.Logger } // NewRollbacker returns a new Rollbacker -func NewRollbacker(tmpDir string) (*Rollbacker, error) { - installDir, err := path.InstallDir() +func NewRollbacker(logger *zap.Logger, tmpDir string) (*Rollbacker, error) { + namedLogger := logger.Named("rollbacker") + + installDir, err := path.InstallDir(namedLogger.Named("install-dir")) if err != nil { return nil, fmt.Errorf("failed to determine install dir: %w", err) } @@ -55,7 +58,8 @@ func NewRollbacker(tmpDir string) (*Rollbacker, error) { backupDir: path.BackupDirFromTempDir(tmpDir), installDir: installDir, tmpDir: tmpDir, - originalSvc: service.NewService(path.LatestDirFromTempDir(tmpDir)), + logger: namedLogger, + originalSvc: service.NewService(namedLogger, path.LatestDirFromTempDir(tmpDir)), }, nil } @@ -66,13 +70,14 @@ func (r *Rollbacker) AppendAction(action action.RollbackableAction) { // Backup backs up the installDir to the rollbackDir func (r Rollbacker) Backup() error { + r.logger.Debug("Backing up current installation") // Remove any pre-existing backup if err := os.RemoveAll(r.backupDir); err != nil { return fmt.Errorf("failed to remove previous backup: %w", err) } // Copy all the files in the install directory to the backup directory - if err := copyFiles(r.installDir, r.backupDir, r.tmpDir); err != nil { + if err := copyFiles(r.logger, r.installDir, r.backupDir, r.tmpDir); err != nil { return fmt.Errorf("failed to copy files to backup dir: %w", err) } @@ -86,18 +91,20 @@ func (r Rollbacker) Backup() error { // Rollback performs a rollback by undoing all recorded actions. func (r Rollbacker) Rollback() { + r.logger.Debug("Performing rollback") // We need to loop through the actions slice backwards, to roll back the actions in the correct order. // e.g. if StartService was called last, we need to stop the service first, then rollback previous actions. for i := len(r.actions) - 1; i >= 0; i-- { action := r.actions[i] + r.logger.Debug("Rolling back action", zap.Any("action", action)) if err := action.Rollback(); err != nil { - log.Default().Printf("Failed to run rollback option: %s", err) + r.logger.Error("Failed to run rollback action", zap.Error(err)) } } } // copyFiles copies files from inputPath to output path, skipping tmpDir. -func copyFiles(inputPath, outputPath, tmpDir string) error { +func copyFiles(logger *zap.Logger, inputPath, outputPath, tmpDir string) error { absTmpDir, err := filepath.Abs(tmpDir) if err != nil { return fmt.Errorf("failed to get absolute path for temporary directory: %w", err) @@ -140,7 +147,7 @@ func copyFiles(inputPath, outputPath, tmpDir string) error { } // Fail if copying the input file to the output file would fail - if err := file.CopyFile(inPath, outPath, false); err != nil { + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, false); err != nil { return fmt.Errorf("failed to copy file: %w", err) } diff --git a/updater/internal/rollback/rollback_test.go b/updater/internal/rollback/rollback_test.go index 0179b673e..d3a293800 100644 --- a/updater/internal/rollback/rollback_test.go +++ b/updater/internal/rollback/rollback_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestRollbackerBackup(t *testing.T) { @@ -41,6 +42,7 @@ func TestRollbackerBackup(t *testing.T) { backupDir: outDir, installDir: installDir, tmpDir: filepath.Join(installDir, "tmp-dir"), + logger: zaptest.NewLogger(t), } err := rb.Backup() @@ -63,6 +65,7 @@ func TestRollbackerBackup(t *testing.T) { backupDir: outDir, installDir: installDir, tmpDir: filepath.Join(installDir, "tmp-dir"), + logger: zaptest.NewLogger(t), } err := rb.Backup() @@ -85,6 +88,7 @@ func TestRollbackerBackup(t *testing.T) { backupDir: outDir, installDir: installDir, tmpDir: filepath.Join(installDir, "tmp-dir"), + logger: zaptest.NewLogger(t), } err = rb.Backup() @@ -101,7 +105,9 @@ func TestRollbackerRollback(t *testing.T) { t.Run("Runs rollback actions in the correct order", func(t *testing.T) { seq := 0 - rb := &Rollbacker{} + rb := &Rollbacker{ + logger: zaptest.NewLogger(t), + } for i := 0; i < 10; i++ { actionNum := i @@ -124,7 +130,9 @@ func TestRollbackerRollback(t *testing.T) { t.Run("Continues despite rollback errors", func(t *testing.T) { seq := 0 - rb := &Rollbacker{} + rb := &Rollbacker{ + logger: zaptest.NewLogger(t), + } for i := 0; i < 10; i++ { actionNum := i diff --git a/updater/internal/service/service_darwin.go b/updater/internal/service/service_darwin.go index 4883142fc..d05ea6b28 100644 --- a/updater/internal/service/service_darwin.go +++ b/updater/internal/service/service_darwin.go @@ -24,6 +24,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/file" "github.com/observiq/observiq-otel-collector/updater/internal/path" + "go.uber.org/zap" ) const ( @@ -41,11 +42,12 @@ func WithServiceFile(svcFilePath string) Option { } // NewService returns an instance of the Service interface for managing the observiq-otel-collector service on the current OS. -func NewService(latestPath string, opts ...Option) Service { +func NewService(logger *zap.Logger, latestPath string, opts ...Option) Service { darwinSvc := &darwinService{ newServiceFilePath: filepath.Join(path.ServiceFileDir(latestPath), "com.observiq.collector.plist"), installedServiceFilePath: darwinServiceFilePath, installDir: path.DarwinInstallDir, + logger: logger.Named("darwin-service"), } for _, opt := range opts { @@ -62,6 +64,7 @@ type darwinService struct { installedServiceFilePath string // installDir is the root directory of the main installation installDir string + logger *zap.Logger } // Start the service @@ -137,7 +140,7 @@ func (d darwinService) Update() error { } func (d darwinService) Backup(outDir string) error { - if err := file.CopyFile(d.installedServiceFilePath, path.BackupServiceFile(outDir), false); err != nil { + if err := file.CopyFile(d.logger.Named("copy-file"), d.installedServiceFilePath, path.BackupServiceFile(outDir), false); err != nil { return fmt.Errorf("failed to copy service file: %w", err) } diff --git a/updater/internal/service/service_darwin_test.go b/updater/internal/service/service_darwin_test.go index 10a38fb84..06b088d54 100644 --- a/updater/internal/service/service_darwin_test.go +++ b/updater/internal/service/service_darwin_test.go @@ -25,6 +25,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) func TestDarwinServiceInstall(t *testing.T) { @@ -36,6 +37,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.install() @@ -62,6 +64,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.install() @@ -97,6 +100,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "does-not-exist.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.install() @@ -112,6 +116,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.install() @@ -127,6 +132,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.uninstall() @@ -142,6 +148,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.Start() @@ -156,6 +163,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: filepath.Join("testdata", "darwin-service.plist"), installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.Stop() @@ -174,6 +182,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err = d.install() @@ -206,6 +215,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } backupServiceDir := t.TempDir() @@ -223,6 +233,7 @@ func TestDarwinServiceInstall(t *testing.T) { d := &darwinService{ newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := d.install() diff --git a/updater/internal/service/service_linux.go b/updater/internal/service/service_linux.go index 5dd23f652..7b9e8425b 100644 --- a/updater/internal/service/service_linux.go +++ b/updater/internal/service/service_linux.go @@ -26,6 +26,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/file" "github.com/observiq/observiq-otel-collector/updater/internal/path" + "go.uber.org/zap" ) const linuxServiceName = "observiq-otel-collector" @@ -42,11 +43,12 @@ func WithServiceFile(svcFilePath string) Option { } // NewService returns an instance of the Service interface for managing the observiq-otel-collector service on the current OS. -func NewService(latestPath string, opts ...Option) Service { +func NewService(logger *zap.Logger, latestPath string, opts ...Option) Service { linuxSvc := &linuxService{ newServiceFilePath: filepath.Join(path.ServiceFileDir(latestPath), "observiq-otel-collector.service"), serviceName: linuxServiceName, installedServiceFilePath: linuxServiceFilePath, + logger: logger.Named("linux-service"), } for _, opt := range opts { @@ -63,6 +65,7 @@ type linuxService struct { serviceName string // installedServiceFilePath is the file path to the installed unit file installedServiceFilePath string + logger *zap.Logger } // Start the service @@ -160,7 +163,7 @@ func (l linuxService) Update() error { } func (l linuxService) Backup(outDir string) error { - if err := file.CopyFile(l.installedServiceFilePath, path.BackupServiceFile(outDir), false); err != nil { + if err := file.CopyFile(l.logger.Named("copy-file"), l.installedServiceFilePath, path.BackupServiceFile(outDir), false); err != nil { return fmt.Errorf("failed to copy service file: %w", err) } diff --git a/updater/internal/service/service_linux_test.go b/updater/internal/service/service_linux_test.go index 2ac5b69bb..937638f1d 100644 --- a/updater/internal/service/service_linux_test.go +++ b/updater/internal/service/service_linux_test.go @@ -25,6 +25,7 @@ import ( "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" ) // NOTE: These tests must run as root in order to pass @@ -37,6 +38,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.install() @@ -62,6 +64,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.install() @@ -97,6 +100,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "does-not-exist.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.install() @@ -112,6 +116,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.install() @@ -127,6 +132,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.uninstall() @@ -142,6 +148,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.Start() @@ -156,6 +163,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join("testdata", "linux-service.service"), serviceName: "linux-service", installedServiceFilePath: installedServicePath, + logger: zaptest.NewLogger(t), } err := l.Stop() @@ -174,6 +182,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, serviceName: "linux-service", + logger: zaptest.NewLogger(t), } err = d.install() @@ -204,6 +213,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, serviceName: "linux-service", + logger: zaptest.NewLogger(t), } backupServiceDir := t.TempDir() @@ -221,6 +231,7 @@ func TestLinuxServiceInstall(t *testing.T) { newServiceFilePath: newServiceFile, installedServiceFilePath: installedServicePath, serviceName: "linux-service", + logger: zaptest.NewLogger(t), } err := d.install() diff --git a/updater/internal/service/service_windows.go b/updater/internal/service/service_windows.go index 3defcad16..fb6e7f09d 100644 --- a/updater/internal/service/service_windows.go +++ b/updater/internal/service/service_windows.go @@ -25,6 +25,7 @@ import ( "strings" "time" + "go.uber.org/zap" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/mgr" @@ -50,11 +51,12 @@ func WithServiceFile(svcFilePath string) Option { } // NewService returns an instance of the Service interface for managing the observiq-otel-collector service on the current OS. -func NewService(latestPath string, opts ...Option) Service { +func NewService(logger *zap.Logger, latestPath string, opts ...Option) Service { winSvc := &windowsService{ newServiceFilePath: filepath.Join(path.ServiceFileDir(latestPath), "windows_service.json"), serviceName: defaultServiceName, productName: defaultProductName, + logger: logger.Named("windows-service"), } for _, opt := range opts { @@ -71,6 +73,7 @@ type windowsService struct { serviceName string // productName is the name of the installed product productName string + logger *zap.Logger } // Start the service @@ -124,7 +127,7 @@ func (w windowsService) install() error { } // fetch the install directory so that we can determine the binary path that we need to execute - iDir, err := path.InstallDirFromRegistry(w.productName) + iDir, err := path.InstallDirFromRegistry(w.logger.Named("install-dir"), w.productName) if err != nil { return fmt.Errorf("failed to get install dir: %w", err) } @@ -250,7 +253,7 @@ func (w windowsService) Backup(outDir string) error { } defer func() { if err := f.Close(); err != nil { - log.Default().Printf("windowsService.Backup: Failed to close backup service file: %s", err) + w.logger.Error("Failed to close backup service file", zap.Error(err)) } }() @@ -332,7 +335,7 @@ func (w windowsService) currentServiceConfig() (*windowsServiceConfig, error) { return nil, fmt.Errorf("failed to split service BinaryPathName: %w", err) } - iDir, err := path.InstallDirFromRegistry(w.productName) + iDir, err := path.InstallDirFromRegistry(w.logger.Named("install-dir"), w.productName) if err != nil { return nil, fmt.Errorf("failed to get install dir: %w", err) } diff --git a/updater/internal/service/service_windows_test.go b/updater/internal/service/service_windows_test.go index 0ae0eadfa..11fb64b75 100644 --- a/updater/internal/service/service_windows_test.go +++ b/updater/internal/service/service_windows_test.go @@ -26,6 +26,7 @@ import ( "testing" "time" + "go.uber.org/zap/zaptest" "golang.org/x/sys/windows/registry" "github.com/observiq/observiq-otel-collector/updater/internal/path" @@ -56,6 +57,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err = w.install() @@ -105,6 +107,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err = w.install() @@ -153,6 +156,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err = w.install() @@ -198,6 +202,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: filepath.Join(tempDir, "not-a-valid-service.json"), serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err = w.install() @@ -232,6 +237,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err := w.Start() @@ -248,6 +254,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err := w.Stop() @@ -280,6 +287,7 @@ func TestWindowsServiceInstall(t *testing.T) { newServiceFilePath: serviceJSON, serviceName: "windows-service", productName: testProductName, + logger: zaptest.NewLogger(t), } err = w.install() diff --git a/updater/internal/state/monitor.go b/updater/internal/state/monitor.go index 11ef1fd98..d99b9897d 100644 --- a/updater/internal/state/monitor.go +++ b/updater/internal/state/monitor.go @@ -52,8 +52,9 @@ type CollectorMonitor struct { // NewCollectorMonitor create a new Monitor specifically for the collector func NewCollectorMonitor(logger *zap.Logger) (Monitor, error) { + namedLogger := logger.Named("collector-monitor") // Get install directory - installDir, err := path.InstallDir() + installDir, err := path.InstallDir(namedLogger.Named("install-dir")) if err != nil { return nil, fmt.Errorf("failed to determine install directory: %w", err) } @@ -61,7 +62,7 @@ func NewCollectorMonitor(logger *zap.Logger) (Monitor, error) { // Create a collector monitor packageStatusPath := filepath.Join(installDir, packagestate.DefaultFileName) collectorMonitor := &CollectorMonitor{ - stateManager: packagestate.NewFileStateManager(logger, packageStatusPath), + stateManager: packagestate.NewFileStateManager(namedLogger, packageStatusPath), } // Load the current status to ensure the package status file exists