fix: Windows updater log fix (#595)

* Added os specific log path

Signed-off-by: Corbin Phelps <[email protected]>

* make tests run on windows

* make fmt, fix function redefinitions

* reduce diff

* add license

Co-authored-by: Corbin Phelps <[email protected]>

updater/internal/logging/logging.go → updater/internal/logging/logging_others.go

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//go:build !windows
+
 package logging
 
 import (

updater/internal/logging/logging_test.go

@@ -18,24 +18,23 @@ 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()
+		// We don't use t.TempDir here, because we can't clean up the out directory on windows.
+		// We also don't clean up the out directory; It's in the temporary directory and may be cleaned up manually at any time.
+		tmpDir, err := os.MkdirTemp("", "test-logger-existing-file")
+		require.NoError(t, err)
+		// Remove previous log directory if it exists
+		require.NoError(t, os.RemoveAll(filepath.Join(tmpDir, "log")))
 		require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "log"), 0775))
 
-		updaterLogPath := filepath.Join(tmpDir, "log", "updater.log")
+		updaterLogPath, err := filepath.Abs(filepath.Join(tmpDir, "log", "updater.log"))
+		require.NoError(t, err)
 
 		initialBytes := []byte("Some existing bytes")
 		require.NoError(t, os.WriteFile(updaterLogPath, initialBytes, 0660))
@@ -57,16 +56,14 @@ func TestNewLogger(t *testing.T) {
 	})
 
 	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()
+		tmpDir, err := os.MkdirTemp("", "test-logger-no-existing-file")
+		require.NoError(t, err)
+		// Remove previous log directory if it exists
+		require.NoError(t, os.RemoveAll(filepath.Join(tmpDir, "log")))
 		require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "log"), 0775))
 
-		updaterLogPath := filepath.Join(tmpDir, "log", "updater.log")
+		updaterLogPath, err := filepath.Abs(filepath.Join(tmpDir, "log", "updater.log"))
+		require.NoError(t, err)
 
 		logger, err := NewLogger(tmpDir)
 		require.NoError(t, err)

updater/internal/logging/logging_windows.go

@@ -0,0 +1,68 @@
+// 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"
+	"net/url"
+	"os"
+	"sync"
+
+	"github.com/observiq/observiq-otel-collector/updater/internal/path"
+	"go.uber.org/zap"
+)
+
+var registerSinkOnce = &sync.Once{}
+
+// NewLogger returns a new logger, that logs to the log directory relative to installDir.
+// It deletes the previous log file, as well.
+// NewLogger must only be called once, at the start of the program.
+func NewLogger(installDir string) (*zap.Logger, error) {
+	// On windows, absolute paths do not work for zap's default sink, so we must register it.
+	// see: https://github.com/uber-go/zap/issues/621
+	var err error
+	registerSinkOnce.Do(func() {
+		err = zap.RegisterSink("winfile", newWinFileSink)
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to registed windows file sink: %w", err)
+	}
+
+	logFile := path.LogFile(installDir)
+
+	err = os.RemoveAll(logFile)
+	if err != nil {
+		return nil, fmt.Errorf("failed to remove previous log file: %w", err)
+	}
+
+	conf := zap.NewProductionConfig()
+	conf.OutputPaths = []string{
+		"winfile:///" + logFile,
+	}
+
+	prodLogger, err := conf.Build()
+	if err != nil {
+		return nil, fmt.Errorf("failed to build logger: %w", err)
+	}
+
+	return prodLogger, nil
+}
+
+// Windows requires a special sink, so that we may properly parse the file path
+// See: https://github.com/uber-go/zap/issues/621
+func newWinFileSink(u *url.URL) (zap.Sink, error) {
+	// Remove leading slash left by url.Parse()
+	return os.OpenFile(u.Path[1:], os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0600)
+}