From de37b504ef5bcd5f27c679d6d67821b272424077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendik=20Nesb=C3=B8?= Date: Wed, 18 Dec 2024 11:09:36 +0100 Subject: [PATCH] feat: Normalize "DD_TRACE_AGENT_URL" to work with both unix-scheme prefix 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 | 26 ++++++++++++++++++++++++++ bootstrap_test.go | 24 ++++++++++++++++++++++++ config/config.go | 9 ++++++--- docs/index.md | 2 +- legacy_bootstrap.go | 25 +++++++++++++++++++++---- legacy_bootstrap_test.go | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 8 deletions(-) diff --git a/bootstrap.go b/bootstrap.go index 9676e16..3250f87 100644 --- a/bootstrap.go +++ b/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 diff --git a/bootstrap_test.go b/bootstrap_test.go index f379cd3..d04abf7 100644 --- a/bootstrap_test.go +++ b/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) + }) + } +} diff --git a/config/config.go b/config/config.go index ecfa859..6c3c740 100644 --- a/config/config.go +++ b/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 } diff --git a/docs/index.md b/docs/index.md index 4d68065..cbf11bc 100644 --- a/docs/index.md +++ b/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: diff --git a/legacy_bootstrap.go b/legacy_bootstrap.go index 03aa1d2..66fbe14 100644 --- a/legacy_bootstrap.go +++ b/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://") +} diff --git a/legacy_bootstrap_test.go b/legacy_bootstrap_test.go index ab7f26c..ebf8b53 100644 --- a/legacy_bootstrap_test.go +++ b/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) + }) + } +}