Merge branch 'main' into update-gcp-readme

.chloggen/make-supervisor-a-compatible-windows-service.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: opampsupervisor
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Make supervisor runnable as a Windows Service.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [34774]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

.github/pull_request_template.md

@@ -1,9 +1,15 @@
-**Description:** <Describe what has changed.>
 <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
 Ex. Adding a feature - Explain what this achieves.-->
+#### Description
 
-**Link to tracking Issue:** <Issue number if applicable>
+<!-- Issue number (e.g. #1234) or full URL to issue, if applicable. -->
+#### Link to tracking issue
+Fixes
 
-**Testing:** <Describe what testing was performed and which tests were added.>
+<!--Describe what testing was performed and which tests were added.-->
+#### Testing
 
-**Documentation:** <Describe the documentation added.>
+<!--Describe the documentation added.-->
+#### Documentation
+
+<!--Please delete paragraphs that you did not use before submitting.-->

.github/workflows/e2e-tests-windows.yml

@@ -82,3 +82,48 @@ jobs:
         run: |
           cd cmd/opampsupervisor
           go test -v --tags=e2e
+
+  windows-supervisor-service-test:
+    runs-on: windows-latest
+    if: ${{ github.actor != 'dependabot[bot]' && (contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') }}
+    needs: [collector-build]
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v4
+      - name: Setup Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ~1.22.7
+          cache: false
+      - name: Cache Go
+        uses: actions/cache@v4
+        env:
+          cache-name: cache-go-modules
+        with:
+          path: |
+            ~\go\pkg\mod
+            ~\AppData\Local\go-build
+          key: go-build-cache-${{ runner.os }}-${{ matrix.group }}-go-${{ hashFiles('**/go.sum') }}
+      - name: Ensure required ports in the dynamic range are available
+        run: |
+          & ${{ github.workspace }}\.github\workflows\scripts\win-required-ports.ps1
+      - name: Download Collector Binary
+        uses: actions/download-artifact@v4
+        with:
+          name: collector-binary
+          path: bin/
+      - name: Build supervisor
+        run: cd cmd/opampsupervisor; go build
+      - name: Install supervisor as a service
+        run: |
+          New-Service -Name "opampsupervisor" -StartupType "Manual" -BinaryPathName "${PWD}\cmd\opampsupervisor --config ${PWD}\cmd\opampsupervisor\supervisor\testdata\supervisor_windows_service_test_config.yaml\"
+          eventcreate.exe /t information /id 1 /l application /d "Creating event provider for 'opampsupervisor'" /so opampsupervisor
+      - name: Test supervisor service
+        working-directory: ${{ github.workspace }}/cmd/opampsupervisor
+        run: |
+          go test -timeout 90s -run ^TestSupervisorAsService$ -v -tags=win32service
+      - name: Remove opampsupervisor service
+        if: always()
+        run: |
+          Remove-Service opampsupervisor
+          Remove-Item HKLM:\SYSTEM\CurrentControlSet\Services\EventLog\Application\opampsupervisor

cmd/opampsupervisor/e2e_test.go

@@ -1414,7 +1414,6 @@ func TestSupervisorInfoLoggingLevel(t *testing.T) {
 	// Read from log file checking for Info level logs
 	logFile, err := os.Open(supervisorLogFilePath)
 	require.NoError(t, err)
-	defer logFile.Close()
 
 	scanner := bufio.NewScanner(logFile)
 	check := false
@@ -1434,6 +1433,7 @@ func TestSupervisorInfoLoggingLevel(t *testing.T) {
 	}
 	// verify at least 1 log was read
 	require.True(t, check)
+	require.NoError(t, logFile.Close())
 }
 
 func findRandomPort() (int, error) {

cmd/opampsupervisor/main.go

@@ -5,6 +5,7 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"log"
 	"os"
 	"os/signal"
@@ -15,33 +16,39 @@ import (
 )
 
 func main() {
+	if err := run(); err != nil {
+		log.Fatal(err)
+	}
+}
+
+func runInteractive() error {
 	configFlag := flag.String("config", "", "Path to a supervisor configuration file")
 	flag.Parse()
 
 	cfg, err := config.Load(*configFlag)
 	if err != nil {
-		log.Fatal("failed to load config: %w", err)
+		return fmt.Errorf("failed to load config: %w", err)
 	}
 
 	logger, err := telemetry.NewLogger(cfg.Telemetry.Logs)
 	if err != nil {
-		log.Fatal("failed to create logger: %w", err)
+		return fmt.Errorf("failed to create logger: %w", err)
 	}
 
 	supervisor, err := supervisor.NewSupervisor(logger, cfg)
 	if err != nil {
-		logger.Error(err.Error())
-		os.Exit(-1)
-		return
+		return fmt.Errorf("failed to create supervisor: %w", err)
 	}
 
 	err = supervisor.Start()
 	if err != nil {
-		log.Fatal("failed to start supervisor: %w", err)
+		return fmt.Errorf("failed to start supervisor: %w", err)
 	}
 
 	interrupt := make(chan os.Signal, 1)
 	signal.Notify(interrupt, os.Interrupt)
 	<-interrupt
 	supervisor.Shutdown()
+
+	return nil
 }

cmd/opampsupervisor/main_others.go

@@ -0,0 +1,10 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build !windows
+
+package main
+
+func run() error {
+	return runInteractive()
+}

cmd/opampsupervisor/main_windows.go

@@ -0,0 +1,71 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build windows
+
+package main
+
+import (
+	"errors"
+	"fmt"
+
+	"golang.org/x/sys/windows"
+	"golang.org/x/sys/windows/svc"
+
+	"github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor"
+)
+
+var (
+	kernel32API = windows.NewLazySystemDLL("kernel32.dll")
+
+	allocConsoleProc = kernel32API.NewProc("AllocConsole")
+	freeConsoleProc  = kernel32API.NewProc("FreeConsole")
+)
+
+func run() error {
+	// always allocate a console in case we're running as service
+	if err := allocConsole(); err != nil {
+		if !errors.Is(err, windows.ERROR_ACCESS_DENIED) {
+			// Per https://learn.microsoft.com/en-us/windows/console/allocconsole#remarks
+			// AllocConsole fails with this error when there's already a console attached, such as not being ran as service
+			// ignore this error and only return other errors
+			return fmt.Errorf("alloc console: %w", err)
+		}
+
+	}
+	defer func() {
+		_ = freeConsole()
+	}()
+
+	// No need to supply service name when startup is invoked through
+	// the Service Control Manager directly.
+	if err := svc.Run("", supervisor.NewSvcHandler()); err != nil {
+		if errors.Is(err, windows.ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
+			// Per https://learn.microsoft.com/en-us/windows/win32/api/winsvc/nf-winsvc-startservicectrldispatchera#return-value
+			// this means that the process is not running as a service, so run interactively.
+
+			return runInteractive()
+		}
+		return fmt.Errorf("failed to start supervisor: %w", err)
+	}
+	return nil
+}
+
+// windows services don't get created with a console
+// need to allocate a console in order to send CTRL_BREAK_EVENT to agent sub process
+func allocConsole() error {
+	ret, _, err := allocConsoleProc.Call()
+	if ret == 0 {
+		return err
+	}
+	return nil
+}
+
+// free console once we're done with it
+func freeConsole() error {
+	ret, _, err := freeConsoleProc.Call()
+	if ret == 0 {
+		return err
+	}
+	return nil
+}

cmd/opampsupervisor/supervisor/commander/commander_windows.go

@@ -6,6 +6,7 @@
 package commander
 
 import (
+	"fmt"
 	"os"
 	"syscall"
 
@@ -24,7 +25,7 @@ func sendShutdownSignal(process *os.Process) error {
 	// See: https://learn.microsoft.com/en-us/windows/console/generateconsolectrlevent
 	r, _, e := ctrlEventProc.Call(syscall.CTRL_BREAK_EVENT, uintptr(process.Pid))
 	if r == 0 {
-		return e
+		return fmt.Errorf("sendShutdownSignal to PID '%d': %w", process.Pid, e)
 	}
 
 	return nil