Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Normalize "DD_TRACE_AGENT_URL" to work with both unix-scheme prefix and not. #453

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
24 changes: 24 additions & 0 deletions bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
})
}
}
9 changes: 6 additions & 3 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 21 additions & 4 deletions legacy_bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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://")
}
33 changes: 33 additions & 0 deletions legacy_bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Loading