factor parsing into internal pkg, dont error out when os cmd fails

cmd/opampsupervisor/e2e_test.go

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+//go:build e2e
+
 package main
 
 import (

extension/opampextension/internal/util/package_test.go

@@ -0,0 +1,14 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package util
+
+import (
+	"testing"
+
+	"go.uber.org/goleak"
+)
+
+func TestMain(m *testing.M) {
+	goleak.VerifyTestMain(m)
+}

extension/opampextension/internal/util/parse.go

@@ -0,0 +1,48 @@
+package util
+
+import "strings"
+
+// ParseDarwinDescription parses out the OS name and version for darwin machines.
+// 'input' should be the string representation from running the command 'sw_vers'.
+// An example of this command's desired output is below:
+//
+// ProductName:		    macOS
+// ProductVersion:		15.0.1
+// BuildVersion:		24A348
+func ParseDarwinDescription(input string) string {
+	var productName string
+	var productVersion string
+	for _, l := range strings.Split(input, "\n") {
+		line := strings.TrimSpace(l)
+		if raw, ok := strings.CutPrefix(line, "ProductName:"); ok {
+			productName = strings.TrimSpace(raw)
+		} else if raw, ok = strings.CutPrefix(line, "ProductVersion:"); ok {
+			productVersion = strings.TrimSpace(raw)
+		}
+	}
+	if productName != "" && productVersion != "" {
+		return productName + " " + productVersion
+	}
+	return ""
+}
+
+// ParseLinuxDescription parses out the OS name and version for linux machines.
+// 'input' should be the string representation from running the command 'lsb_release -d'.
+// An example of this command's desired output is below:
+//
+// Description:	    Ubuntu 20.04.6 LTS
+func ParseLinuxDescription(input string) string {
+	if raw, ok := strings.CutPrefix(strings.TrimSpace(input), "Description:"); ok {
+		return strings.TrimSpace(raw)
+	}
+	return ""
+}
+
+// ParseLinuxDescription parses out the OS name and version for windows machines.
+// 'input' should be the string representation from running the command 'cmd /c ver'.
+// An example of this command's desired output is below:
+//
+// Microsoft Windows [Version 10.0.20348.2700]
+func ParseWindowsDescription(input string) string {
+	return strings.TrimSpace(input)
+}

extension/opampextension/internal/util/parse_test.go

@@ -0,0 +1,125 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseDarwinDescription(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		input    string
+		expected string
+	}{
+		{
+			desc:     "basic use case",
+			input:    "ProductName: macOS\nProductVersion: 15.0.1\nBuildVersion: 24A348",
+			expected: "macOS 15.0.1",
+		},
+		{
+			desc:     "excessive white space",
+			input:    "			   \n	ProductName:		 macOS\nProductVersion: 15.0.1     \n \n	BuildVersion: 24A348\n",
+			expected: "macOS 15.0.1",
+		},
+		{
+			desc:     "random ordering & excessive white space",
+			input:    "\nProductVersion: 15.0.1     \n \n	BuildVersion: 24A348\n			   \n	ProductName:		 macOS",
+			expected: "macOS 15.0.1",
+		},
+		{
+			desc:     "blank input",
+			input:    "",
+			expected: "",
+		},
+		{
+			desc:     "missing ProductVersion",
+			input:    "ProductName: macOS\nBuildVersion: 24A348",
+			expected: "",
+		},
+		{
+			desc:     "missing ProductName",
+			input:    "ProductVersion: 15.0.1\nBuildVersion: 24A348",
+			expected: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			actual := ParseDarwinDescription(tc.input)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}
+
+func TestParseLinuxDescription(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		input    string
+		expected string
+	}{
+		{
+			desc:     "basic use case",
+			input:    "Description: Ubuntu 20.04.6 LTS",
+			expected: "Ubuntu 20.04.6 LTS",
+		},
+		{
+			desc:     "excessive white space",
+			input:    "         \n Description:            	Ubuntu 20.04.6 LTS			\n		",
+			expected: "Ubuntu 20.04.6 LTS",
+		},
+		{
+			desc:     "blank input",
+			input:    "",
+			expected: "",
+		},
+		{
+			desc:     "missing Description",
+			input:    "Foo: Ubuntu 20.04.6 LTS",
+			expected: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			actual := ParseLinuxDescription(tc.input)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}
+
+func TestParseWindowsDescription(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		input    string
+		expected string
+	}{
+		{
+			desc:     "basic use case",
+			input:    "Microsoft Windows [Version 10.0.20348.2700]",
+			expected: "Microsoft Windows [Version 10.0.20348.2700]",
+		},
+		{
+			desc:     "excessive surrounding white space",
+			input:    "         \n Microsoft Windows [Version 10.0.20348.2700]			\n		",
+			expected: "Microsoft Windows [Version 10.0.20348.2700]",
+		},
+		{
+			desc:     "excessive white space",
+			input:    "         \n Microsoft       Windows [Version 10.0.20348.2700]			\n		",
+			expected: "Microsoft       Windows [Version 10.0.20348.2700]",
+		},
+		{
+			desc:     "blank input",
+			input:    "",
+			expected: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			actual := ParseWindowsDescription(tc.input)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}

extension/opampextension/opamp_agent.go

@@ -30,6 +30,7 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/extension/opampcustommessages"
+	"github.com/open-telemetry/opentelemetry-collector-contrib/extension/opampextension/internal/util"
 )
 
 type opampAgent struct {
@@ -240,10 +241,7 @@ func (o *opampAgent) createAgentDescription() error {
 	if err != nil {
 		return err
 	}
-	description, err := o.getOSDescription()
-	if err != nil {
-		return err
-	}
+	description := getOSDescription(o.logger)
 
 	ident := []*protobufs.KeyValue{
 		stringKeyValue(semconv.AttributeServiceInstanceID, o.instanceID.String()),
@@ -255,9 +253,11 @@ func (o *opampAgent) createAgentDescription() error {
 	// are both automatically determined and defined in the config
 	nonIdentifyingAttributeMap := map[string]string{}
 	nonIdentifyingAttributeMap[semconv.AttributeOSType] = runtime.GOOS
-	nonIdentifyingAttributeMap[semconv.AttributeOSDescription] = description
 	nonIdentifyingAttributeMap[semconv.AttributeHostArch] = runtime.GOARCH
 	nonIdentifyingAttributeMap[semconv.AttributeHostName] = hostname
+	if description != "" {
+		nonIdentifyingAttributeMap[semconv.AttributeOSDescription] = description
+	}
 
 	for k, v := range o.cfg.AgentDescription.NonIdentifyingAttributes {
 		nonIdentifyingAttributeMap[k] = v
@@ -327,43 +327,30 @@ func (o *opampAgent) onMessage(_ context.Context, msg *types.MessageData) {
 	}
 }
 
-func (o *opampAgent) getOSDescription() (string, error) {
+func getOSDescription(logger *zap.Logger) string {
 	switch runtime.GOOS {
-	case "linux":
-		output, err := exec.Command("lsb_release", "-a").Output()
-		if err != nil {
-			return "", fmt.Errorf("get linux details: %w", err)
-		}
-		for _, line := range strings.Split(string(output), "\n") {
-			if raw, ok := strings.CutPrefix(line, "Description:"); ok {
-				return strings.TrimSpace(raw), nil
-			}
-		}
 	case "darwin":
 		output, err := exec.Command("sw_vers").Output()
 		if err != nil {
-			return "", fmt.Errorf("get darwin details: %w", err)
-		}
-		var productName string
-		var productVersion string
-		for _, line := range strings.Split(string(output), "\n") {
-			if raw, ok := strings.CutPrefix(line, "ProductName:"); ok {
-				productName = strings.TrimSpace(raw)
-			} else if raw, ok = strings.CutPrefix(line, "ProductVersion:"); ok {
-				productVersion = strings.TrimSpace(raw)
-			}
+			logger.Error("get darwin OS details using 'sw_vers'", zap.Error(err))
+			return ""
 		}
-		if productName != "" && productVersion != "" {
-			return productName + " " + productVersion, nil
+		return util.ParseDarwinDescription(string(output))
+	case "linux":
+		output, err := exec.Command("lsb_release", "-d").Output()
+		if err != nil {
+			logger.Error("get linux OS details using 'lsb_release -d'", zap.Error(err))
+			return ""
 		}
+		return util.ParseLinuxDescription(string(output))
 	case "windows":
 		output, err := exec.Command("cmd", "/c", "ver").Output()
 		if err != nil {
-			return "", fmt.Errorf("get windows details: %w", err)
-		}
-		if string(output) != "" {
-			return strings.TrimSpace(string(output)), nil
+			logger.Error("get windows OS details using 'cmd /c ver'", zap.Error(err))
+			return ""
 		}
+		return util.ParseWindowsDescription(string(output))
 	}
-	return "", fmt.Errorf("unrecognized os")
+	logger.Error("unrecognized OS to parse details from")
+	return ""
 }

extension/opampextension/opamp_agent_test.go

@@ -6,10 +6,8 @@ package opampextension
 import (
 	"context"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"testing"
 
 	"github.com/google/uuid"
@@ -21,6 +19,7 @@ import (
 	"go.opentelemetry.io/collector/confmap/confmaptest"
 	"go.opentelemetry.io/collector/extension/extensiontest"
 	semconv "go.opentelemetry.io/collector/semconv/v1.27.0"
+	"go.uber.org/zap"
 )
 
 func TestNewOpampAgent(t *testing.T) {
@@ -54,7 +53,7 @@ func TestNewOpampAgentAttributes(t *testing.T) {
 func TestCreateAgentDescription(t *testing.T) {
 	hostname, err := os.Hostname()
 	require.NoError(t, err)
-	osDescription := getOSDescription(t)
+	osDescription := getOSDescription(zap.NewNop())
 
 	serviceName := "otelcol-distrot"
 	serviceVersion := "distro.0"
@@ -246,38 +245,3 @@ func TestParseInstanceIDString(t *testing.T) {
 		})
 	}
 }
-
-func getOSDescription(t *testing.T) string {
-	switch runtime.GOOS {
-	case "linux":
-		output, err := exec.Command("lsb_release", "-a").Output()
-		require.NoError(t, err)
-		for _, line := range strings.Split(string(output), "\n") {
-			if raw, ok := strings.CutPrefix(line, "Description:"); ok {
-				return strings.TrimSpace(raw)
-			}
-		}
-	case "darwin":
-		output, err := exec.Command("sw_vers").Output()
-		require.NoError(t, err)
-		var productName string
-		var productVersion string
-		for _, line := range strings.Split(string(output), "\n") {
-			if raw, ok := strings.CutPrefix(line, "ProductName:"); ok {
-				productName = strings.TrimSpace(raw)
-			} else if raw, ok = strings.CutPrefix(line, "ProductVersion:"); ok {
-				productVersion = strings.TrimSpace(raw)
-			}
-		}
-		if productName != "" && productVersion != "" {
-			return productName + " " + productVersion
-		}
-	case "windows":
-		output, err := exec.Command("cmd", "/c", "ver").Output()
-		require.NoError(t, err)
-		if string(output) != "" {
-			return strings.TrimSpace(string(output))
-		}
-	}
-	return ""
-}