feat: Normalize "DD_TRACE_AGENT_URL" to work with both unix-scheme pr…

…efix and not.

Both for the legacy bootstrapper, and the new bootstrapper, we should allow both `/var/run/datadog/apm.socket` and `unix:///var/run/datadog/apm.socket`. This should help in migrating from the legacy to the new bootstrapper.

bootstrap.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
+	"strings"
 
 	"github.com/coopnorge/go-datadog-lib/v2/internal"
 	datadogLogger "github.com/coopnorge/go-logger/adapter/datadog"
@@ -63,6 +65,10 @@ func Start(ctx context.Context, opts ...Option) (StopFunc, error) {
 		return noop, err
 	}
 
+	if err := normalizeDatadogEnvVars(); err != nil {
+		return noop, fmt.Errorf("failed to normalize Datadog environment variables: %w", err)
+	}
+
 	options, err := resolveOptions(opts)
 	if err != nil {
 		return noop, err
@@ -82,6 +88,26 @@ func Start(ctx context.Context, opts ...Option) (StopFunc, error) {
 	return cancel, err
 }
 
+// normalizeDatadogEnvVars ensures that the environment variables that the Datadog library is on the expected format.
+func normalizeDatadogEnvVars() error {
+	apmEndpoint := os.Getenv(internal.DatadogAPMEndpoint)
+	if normalizedAPMEndpoint, changed := normalizeAPMEndpoint(apmEndpoint); changed {
+		err := os.Setenv(internal.DatadogAPMEndpoint, normalizedAPMEndpoint)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func normalizeAPMEndpoint(apmEndpoint string) (string, bool) {
+	if strings.HasPrefix(apmEndpoint, "/") {
+		// apmEndpoint did not have a scheme set, but it looks like unix scheme, so we explicitly set it.
+		return fmt.Sprintf("unix://%s", apmEndpoint), true
+	}
+	return apmEndpoint, false
+}
+
 // StopFunc is a function signature for functions that stops the Datadog
 // integration.
 type StopFunc func() error

bootstrap_test.go

@@ -2,11 +2,13 @@ package coopdatadog
 
 import (
 	"context"
+	"os"
 	"testing"
 
 	"github.com/coopnorge/go-datadog-lib/v2/internal"
 	"github.com/coopnorge/go-datadog-lib/v2/internal/testhelpers"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBootstrapDatadogDisabled(t *testing.T) {
@@ -46,3 +48,25 @@ func TestBootstrapMissingEnvVar(t *testing.T) {
 	assert.ErrorContains(t, err, "required environmental variable not set: ")
 	assert.NotNil(t, stop)
 }
+
+func TestNormalizeAPMEndpointEnv(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"unix:///var/run/datadog/apm.socket", "unix:///var/run/datadog/apm.socket"}, // Do not change a socket path that is already prefixed
+		{"/var/run/datadog/apm.socket", "unix:///var/run/datadog/apm.socket"},        // Prefix an assumed unix socket path
+		{"http://my-dd-agent:3678", "http://my-dd-agent:3678"},                       // Do not change HTTP-addresses
+		{"http://my-dd-agent", "http://my-dd-agent"},                                 // Do not change HTTP-addresses
+		{"http://10.0.0.6", "http://10.0.0.6"},                                       // Do not change HTTP-addresses
+	}
+	for _, tc := range tests {
+		t.Run(tc.input, func(t *testing.T) {
+			t.Setenv(internal.DatadogAPMEndpoint, tc.input)
+			err := normalizeDatadogEnvVars()
+			require.NoError(t, err)
+			got := os.Getenv(internal.DatadogAPMEndpoint)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}

config/config.go

@@ -42,7 +42,7 @@ type (
 		ServiceVersion string `mapstructure:"dd_version" json:"dd_service_version,omitempty"`
 		// DSD Socket path for DD StatsD, important to have unix prefix for that value, example: unix:///var/run/dd/dsd.socket
 		DSD string `mapstructure:"dd_dogstatsd_url" json:"dd_dsd,omitempty"`
-		// APM Socket path for apm and profiler, unix prefix not needed, example: /var/run/dd/apm.socket
+		// APM Socket path for apm and profiler, unix prefix recommended, but not required, example: unix:///var/run/dd/apm.socket
 		APM string `mapstructure:"dd_trace_agent_url" json:"dd_apm,omitempty"`
 		// EnableExtraProfiling flag enables more optional profilers not recommended for production.
 		EnableExtraProfiling bool `mapstructure:"dd_enable_extra_profiling" json:"dd_enable_extra_profiling,omitempty"`
@@ -96,13 +96,16 @@ func (d DatadogConfig) GetServiceVersion() string {
 }
 
 // GetDsdEndpoint Socket path or URL for DD StatsD
-// for socket important to have unix prefix for that value, example: unix:///var/run/dd/dsd.socket
+// For unix sockets, the unix-scheme prefix is required.
+// Example: unix:///var/run/dd/dsd.socket
 func (d DatadogConfig) GetDsdEndpoint() string {
 	return d.DSD
 }
 
 // GetApmEndpoint Socket path or URL for APM and profiler
-// unix prefix not needed, example: /var/run/dd/apm.socket
+// For unix sockets, the unix-scheme prefix is not needed, but it is recommended to include it.
+// Example: unix:///var/run/dd/apm.socket
+// Example: http://my-agent:1234
 func (d DatadogConfig) GetApmEndpoint() string {
 	return d.APM
 }

docs/index.md

@@ -71,7 +71,7 @@ spec:
             - name: DD_DOGSTATSD_URL
               value: "unix:///var/run/datadog/dsd.socket"
             - name: DD_TRACE_AGENT_URL
-              value: "/var/run/datadog/apm.socket"
+              value: "unix:///var/run/datadog/apm.socket"
             - name: DD_SERVICE
               valueFrom:
                 fieldRef:

legacy_bootstrap.go

@@ -3,6 +3,7 @@ package coopdatadog
 import (
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/coopnorge/go-datadog-lib/v2/config"
 	"github.com/coopnorge/go-datadog-lib/v2/internal"
@@ -121,9 +122,11 @@ func initTracer(cfg config.DatadogParameters, connectionType ConnectionType) {
 	tracerOptions := make([]tracer.StartOption, 0, 5)
 	switch connectionType {
 	case ConnectionTypeSocket:
-		tracerOptions = append(tracerOptions, tracer.WithUDS(cfg.GetApmEndpoint()))
+		socketPath := normalizeLegacySocketPath(cfg.GetApmEndpoint())
+		tracerOptions = append(tracerOptions, tracer.WithUDS(socketPath))
 	case ConnectionTypeHTTP:
-		tracerOptions = append(tracerOptions, tracer.WithAgentAddr(cfg.GetApmEndpoint()))
+		httpAddr := normalizeLegacyHTTPAddr(cfg.GetApmEndpoint())
+		tracerOptions = append(tracerOptions, tracer.WithAgentAddr(httpAddr))
 	case ConnectionTypeAuto:
 		// Let the underlying library determine the URL from environment-variables
 	}
@@ -158,9 +161,11 @@ func initProfiler(cfg config.DatadogParameters, connectionType ConnectionType) e
 	profilerOptions := make([]profiler.Option, 0, 5)
 	switch connectionType {
 	case ConnectionTypeSocket:
-		profilerOptions = append(profilerOptions, profiler.WithUDS(cfg.GetApmEndpoint()))
+		socketPath := normalizeLegacySocketPath(cfg.GetApmEndpoint())
+		profilerOptions = append(profilerOptions, profiler.WithUDS(socketPath))
 	case ConnectionTypeHTTP:
-		profilerOptions = append(profilerOptions, profiler.WithAgentAddr(cfg.GetApmEndpoint()))
+		httpAddr := normalizeLegacyHTTPAddr(cfg.GetApmEndpoint())
+		profilerOptions = append(profilerOptions, profiler.WithAgentAddr(httpAddr))
 	case ConnectionTypeAuto:
 		// Let the underlying library determine the URL from environment-variables
 	}
@@ -177,3 +182,15 @@ func initProfiler(cfg config.DatadogParameters, connectionType ConnectionType) e
 
 	return profiler.Start(profilerOptions...)
 }
+
+// normalizeLegacySocketPath ensures that the socketpath is in the format the tracer.WithUDS and profiler.WithUDS expects.
+func normalizeLegacySocketPath(socketPath string) string {
+	// profiler.WithUDS and tracer.WithUDS expects a path without the scheme
+	return strings.TrimPrefix(socketPath, "unix://")
+}
+
+// normalizeLegacySocketPath ensures that the HTTP address is in the format the tracer.WithAgentAddr and profiler.WithAgentAddr expects.
+func normalizeLegacyHTTPAddr(addr string) string {
+	// profiler.WithAgentAddr and tracer.WithAgentAddr expects a path without the scheme
+	return strings.TrimPrefix(addr, "http://")
+}

legacy_bootstrap_test.go

@@ -63,3 +63,36 @@ func TestValidateConnectionType(t *testing.T) {
 		})
 	}
 }
+
+func TestNormalizeLegacySocketPath(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"unix:///var/run/datadog/apm.socket", "/var/run/datadog/apm.socket"},
+		{"/var/run/datadog/apm.socket", "/var/run/datadog/apm.socket"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.input, func(t *testing.T) {
+			got := normalizeLegacySocketPath(tc.input)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
+
+func TestNormalizeLegacyHTTPAddr(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"http://my-dd-agent:3678", "my-dd-agent:3678"},
+		{"http://my-dd-agent", "my-dd-agent"},
+		{"http://10.0.0.6", "10.0.0.6"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.input, func(t *testing.T) {
+			got := normalizeLegacyHTTPAddr(tc.input)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}