Fix default docker_observer endpoint on Windows (#34358)

pjanotti · web-flow · commit 4d97bc2e2984 · 2024-08-07T10:01:12.000-07:00
**Description:**
The default endpoint for the `docker_observer` is incorrect on Windows:
it should be `npipe:////./pipe/docker_engine` on Windows. Without this
change the observer fails on its default configuration on Windows.

Updated `extension/observer/dockerobserver` and `internal/docker`
(including respective tests) to have a per OS default value for endpoint
that allows the observer to work with default config on Windows.

**Link to tracking Issue:** N/A

**Testing:**
Manual tests using the following config:
```yaml
extensions:
  docker_observer:

receivers:
  receiver_creator:
    watch_observers: [docker_observer]
    receivers:
      nginx:
        rule: type == "container" and name matches "nginx" and port == 80
        config:
          endpoint: '`endpoint`/status'
          collection_interval: 10s

exporters:
  debug:
    verbosity: detailed

service:
  extensions: [docker_observer]
  pipelines:
    logs:
      receivers: [receiver_creator]
      processors: []
      exporters: [debug]
```

**Documentation:**
- [X] Updated readme files
- [X] chloggen (~~pending PR number~~)
diff --git a/.chloggen/docker_observer_default_windows_endpoint.yaml b/.chloggen/docker_observer_default_windows_endpoint.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: 'bug_fix'
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: docker_observer
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Change default endpoint for `docker_observer` on Windows to `npipe:////./pipe/docker_engine`
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [34358]
+
+# (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: []
diff --git a/extension/observer/dockerobserver/README.md b/extension/observer/dockerobserver/README.md
@@ -19,15 +19,15 @@ This observer watches the Docker engine's stream of events to dynamically create
 Requires Docker API Version 1.24+.
 
 The collector will need permissions to access the Docker Engine API, specifically it will need
-read access to the Docker socket (default `unix:///var/run/docker.sock`).
+read access to the Docker socket (default `unix:///var/run/docker.sock` on non-Windows and `npipe:////./pipe/docker_engine` on Windows).
 
 
 ## Example Config
 
 ```yaml
 extensions:
   docker_observer:
-    # url of the docker socket, default to unix:///var/run/docker.sock
+    # url of the docker socket, defaults to unix:///var/run/docker.sock on non-Windows and npipe:////./pipe/docker_engine on Windows
     endpoint: my/path/to/docker.sock
     # list of container image names to exclude
     excluded_images: ['redis', 'another_image_name']
@@ -53,7 +53,7 @@ receivers:
 
 The URL of the docker server.
 
-default: `unix:///var/run/docker.sock`
+default: `unix:///var/run/docker.sock` on non-Windows and `npipe:////./pipe/docker_engine` on Windows
 
 ### `timeout`
 
diff --git a/extension/observer/dockerobserver/config.go b/extension/observer/dockerobserver/config.go
@@ -16,7 +16,8 @@ import (
 // Config defines configuration for docker observer
 type Config struct {
 
-	// The URL of the docker server.  Default is "unix:///var/run/docker.sock"
+	// The URL of the docker server.  Default is "unix:///var/run/docker.sock" on non-Windows
+	// and "npipe:////./pipe/docker_engine" on Windows
 	Endpoint string `mapstructure:"endpoint"`
 
 	// The maximum amount of time to wait for docker API responses.  Default is 5s
diff --git a/extension/observer/dockerobserver/factory.go b/extension/observer/dockerobserver/factory.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"time"
 
+	"github.com/docker/docker/client"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/extension"
 
@@ -25,7 +26,7 @@ func NewFactory() extension.Factory {
 
 func createDefaultConfig() component.Config {
 	return &Config{
-		Endpoint:          "unix:///var/run/docker.sock",
+		Endpoint:          client.DefaultDockerHost,
 		Timeout:           5 * time.Second,
 		CacheSyncInterval: 60 * time.Minute,
 		DockerAPIVersion:  defaultDockerAPIVersion,
diff --git a/extension/observer/dockerobserver/go.mod b/extension/observer/dockerobserver/go.mod
@@ -20,7 +20,7 @@ require (
 require (
 	dario.cat/mergo v1.0.0 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
-	github.com/Microsoft/go-winio v0.6.1 // indirect
+	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/Microsoft/hcsshim v0.11.4 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
@@ -83,12 +83,9 @@ require (
 	go.opentelemetry.io/otel/trace v1.28.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/crypto v0.24.0 // indirect
-	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/net v0.26.0 // indirect
-	golang.org/x/sync v0.7.0 // indirect
 	golang.org/x/sys v0.21.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
-	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157 // indirect
 	google.golang.org/grpc v1.65.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
diff --git a/extension/observer/dockerobserver/go.sum b/extension/observer/dockerobserver/go.sum
diff --git a/internal/docker/config.go b/internal/docker/config.go
@@ -11,10 +11,12 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/client"
 )
 
 type Config struct {
 	// The URL of the docker server. Default is "unix:///var/run/docker.sock"
+	// on non-Windows and "npipe:////./pipe/docker_engine" on Windows
 	Endpoint string `mapstructure:"endpoint"`
 
 	// The maximum amount of time to wait for docker API responses. Default is 5s
@@ -43,7 +45,7 @@ func NewConfig(endpoint string, timeout time.Duration, excludedImages []string,
 // to be used when creating a docker client
 func NewDefaultConfig() *Config {
 	cfg := &Config{
-		Endpoint:         "unix:///var/run/docker.sock",
+		Endpoint:         client.DefaultDockerHost,
 		Timeout:          5 * time.Second,
 		DockerAPIVersion: minimumRequiredDockerAPIVersion,
 	}
diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go
@@ -1,20 +1,15 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-//go:build !windows
-
-// TODO review if tests should succeed on Windows
-
 package docker
 
 import (
 	"context"
 	"fmt"
 	"io"
-	"net"
 	"net/http"
 	"net/http/httptest"
-	"os"
+	"runtime"
 	"strings"
 	"sync"
 	"testing"
@@ -47,34 +42,14 @@ func TestInvalidExclude(t *testing.T) {
 	assert.Equal(t, "could not determine docker client excluded images: invalid glob item: unexpected end of input", err.Error())
 }
 
-func tmpSock(t *testing.T) (net.Listener, string) {
-	f, err := os.CreateTemp(os.TempDir(), "testsock")
-	if err != nil {
-		t.Fatal(err)
-	}
-	addr := f.Name()
-	assert.NoError(t, os.Remove(addr))
-
-	listener, err := net.Listen("unix", addr)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return listener, addr
-}
-
 func TestWatchingTimeouts(t *testing.T) {
-	listener, addr := tmpSock(t)
+	listener, addr := testListener(t)
 	defer func() {
 		assert.NoError(t, listener.Close())
 	}()
 
-	defer func() {
-		assert.NoError(t, os.Remove(addr))
-	}()
-
 	config := &Config{
-		Endpoint: fmt.Sprintf("unix://%s", addr),
+		Endpoint: portableEndpoint(addr),
 		Timeout:  50 * time.Millisecond,
 	}
 
@@ -109,17 +84,14 @@ func TestWatchingTimeouts(t *testing.T) {
 }
 
 func TestFetchingTimeouts(t *testing.T) {
-	listener, addr := tmpSock(t)
+	listener, addr := testListener(t)
 
 	defer func() {
 		assert.NoError(t, listener.Close())
 	}()
-	defer func() {
-		assert.NoError(t, os.Remove(addr))
-	}()
 
 	config := &Config{
-		Endpoint: fmt.Sprintf("unix://%s", addr),
+		Endpoint: portableEndpoint(addr),
 		Timeout:  50 * time.Millisecond,
 	}
 
@@ -165,17 +137,13 @@ func TestFetchingTimeouts(t *testing.T) {
 }
 
 func TestToStatsJSONErrorHandling(t *testing.T) {
-	listener, addr := tmpSock(t)
+	listener, addr := testListener(t)
 	defer func() {
 		assert.NoError(t, listener.Close())
 	}()
 
-	defer func() {
-		assert.NoError(t, os.Remove(addr))
-	}()
-
 	config := &Config{
-		Endpoint: fmt.Sprintf("unix://%s", addr),
+		Endpoint: portableEndpoint(addr),
 		Timeout:  50 * time.Millisecond,
 	}
 
@@ -254,3 +222,11 @@ func TestEventLoopHandlesError(t *testing.T) {
 		return
 	}
 }
+
+func portableEndpoint(addr string) string {
+	endpoint := fmt.Sprintf("unix://%s", addr)
+	if runtime.GOOS == "windows" {
+		endpoint = fmt.Sprintf("npipe://%s", strings.ReplaceAll(addr, "\\", "/"))
+	}
+	return endpoint
+}
diff --git a/internal/docker/docker_test_listener.go b/internal/docker/docker_test_listener.go
@@ -0,0 +1,29 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+//go:build !windows
+
+package docker // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker"
+
+import (
+	"net"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func testListener(t *testing.T) (net.Listener, string) {
+	f, err := os.CreateTemp(os.TempDir(), "testListener")
+	if err != nil {
+		t.Fatal(err)
+	}
+	addr := f.Name()
+	require.NoError(t, os.Remove(addr))
+
+	listener, err := net.Listen("unix", addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return listener, addr
+}
diff --git a/internal/docker/docker_test_listener_windows.go b/internal/docker/docker_test_listener_windows.go
@@ -0,0 +1,22 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+//go:build windows
+
+package docker // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker"
+
+import (
+	"net"
+	"testing"
+
+	"github.com/Microsoft/go-winio"
+	"github.com/stretchr/testify/require"
+)
+
+func testListener(t *testing.T) (net.Listener, string) {
+	addr := "\\\\.\\pipe\\testListener-otel-collector-contrib"
+
+	l, err := winio.ListenPipe(addr, nil)
+	require.NoError(t, err)
+	require.NotNil(t, l)
+	return l, addr
+}
diff --git a/internal/docker/go.mod b/internal/docker/go.mod
@@ -3,6 +3,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker
 go 1.21.0
 
 require (
+	github.com/Microsoft/go-winio v0.6.2
 	github.com/docker/docker v26.1.5+incompatible
 	github.com/gobwas/glob v0.2.3
 	github.com/stretchr/testify v1.9.0
@@ -11,7 +12,6 @@ require (
 )
 
 require (
-	github.com/Microsoft/go-winio v0.4.17 // indirect
 	github.com/containerd/log v0.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/distribution/reference v0.5.0 // indirect
diff --git a/internal/docker/go.sum b/internal/docker/go.sum
diff --git a/receiver/dockerstatsreceiver/go.mod b/receiver/dockerstatsreceiver/go.mod
@@ -26,7 +26,7 @@ require (
 require (
 	dario.cat/mergo v1.0.0 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
-	github.com/Microsoft/go-winio v0.6.1 // indirect
+	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/Microsoft/hcsshim v0.11.4 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.3.0 // indirect
@@ -95,12 +95,9 @@ require (
 	go.opentelemetry.io/otel/sdk/metric v1.28.0 // indirect
 	go.opentelemetry.io/otel/trace v1.28.0 // indirect
 	golang.org/x/crypto v0.24.0 // indirect
-	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/net v0.26.0 // indirect
-	golang.org/x/sync v0.7.0 // indirect
 	golang.org/x/sys v0.21.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
-	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect
 	google.golang.org/grpc v1.65.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
diff --git a/receiver/dockerstatsreceiver/go.sum b/receiver/dockerstatsreceiver/go.sum

-Original file line number
+Diff line change
 go 1.21.0
 require (
 +	github.com/Microsoft/go-winio v0.6.2
 	github.com/docker/docker v26.1.5+incompatible
 	github.com/gobwas/glob v0.2.3
 	github.com/stretchr/testify v1.9.0
+)
 require (
 -	github.com/Microsoft/go-winio v0.4.17 // indirect
 	github.com/containerd/log v0.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/distribution/reference v0.5.0 // indirect