diff --git a/ddtrace/tracer/civisibility_transport_test.go b/ddtrace/tracer/civisibility_transport_test.go index 89e8f83d80..a5979b4ee6 100644 --- a/ddtrace/tracer/civisibility_transport_test.go +++ b/ddtrace/tracer/civisibility_transport_test.go @@ -15,7 +15,7 @@ import ( "strings" "testing" - "github.com/DataDog/dd-trace-go/v2/internal" + internalconfig "github.com/DataDog/dd-trace-go/v2/internal/config" "github.com/stretchr/testify/assert" "github.com/tinylib/msgp/msgp" @@ -82,11 +82,6 @@ func runTransportTest(t *testing.T, agentless, shouldSetAPIKey bool) { defer srv.Close() parsedURL, _ := url.Parse(srv.URL) - c := config{ - ciVisibilityEnabled: true, - httpClient: internal.DefaultHTTPClient(defaultHTTPTimeout, false), - agentURL: parsedURL, - } // Set CI Visibility environment variables for the test if agentless { @@ -97,8 +92,14 @@ func runTransportTest(t *testing.T, agentless, shouldSetAPIKey bool) { } } + // Use newTestConfig to get proper HTTP client setup (keep-alives disabled) + c, err := newTestConfig() + assert.NoError(err) + c.agentURL = parsedURL + c.internalConfig.SetCiVisibilityEnabled(true, internalconfig.OriginCode) + for _, tc := range testCases { - transport := newCiVisibilityTransport(&c) + transport := newCiVisibilityTransport(c) p := newCiVisibilityPayload() for _, t := range tc.payload { @@ -127,7 +128,8 @@ func TestCIVisibilityTransportSecureLogging(t *testing.T) { os.Unsetenv(constants.CIVisibilityAgentlessURLEnvironmentVariable) }() - cfg := &config{} + cfg, err := newTestConfig() + assert.NoError(t, err) transport := newCiVisibilityTransport(cfg) assert.NotNil(t, transport) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 0ce39ee3a7..e4f5f6ed36 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -316,9 +316,6 @@ type config struct { // globalSampleRate holds sample rate read from environment variables. globalSampleRate float64 - // ciVisibilityEnabled controls if the tracer is loaded with CI Visibility mode. default false - ciVisibilityEnabled bool - // ciVisibilityAgentless controls if the tracer is loaded with CI Visibility agentless mode. default false ciVisibilityAgentless bool @@ -616,8 +613,7 @@ func newConfig(opts ...StartOption) (*config, error) { log.SetLevel(log.LevelDebug) } // Check if CI Visibility mode is enabled - if internal.BoolEnv(constants.CIVisibilityEnabledEnvironmentVariable, false) { - c.ciVisibilityEnabled = true // Enable CI Visibility mode + if c.internalConfig.CiVisibilityEnabled() { c.httpClientTimeout = time.Second * 45 // Increase timeout up to 45 seconds (same as other tracers in CIVis mode) c.logStartup = false // If we are in CI Visibility mode we don't want to log the startup to stdout to avoid polluting the output ciTransport := newCiVisibilityTransport(c) // Create a default CI Visibility Transport diff --git a/ddtrace/tracer/stats.go b/ddtrace/tracer/stats.go index f6fb104b55..accfc962aa 100644 --- a/ddtrace/tracer/stats.go +++ b/ddtrace/tracer/stats.go @@ -77,7 +77,7 @@ func newConcentrator(c *config, bucketSize int64, statsdClient internal.StatsdCl log.Debug("No DD Env found, normally the agent should have one") } gitCommitSha := "" - if c.ciVisibilityEnabled { + if c.internalConfig.CiVisibilityEnabled() { // We only have this data if we're in CI Visibility gitCommitSha = utils.GetCITags()[constants.GitCommitSHA] } diff --git a/ddtrace/tracer/stats_test.go b/ddtrace/tracer/stats_test.go index c760d3836f..151c00c0d8 100644 --- a/ddtrace/tracer/stats_test.go +++ b/ddtrace/tracer/stats_test.go @@ -18,6 +18,7 @@ import ( "github.com/DataDog/dd-trace-go/v2/ddtrace/ext" "github.com/DataDog/dd-trace-go/v2/internal/civisibility/constants" "github.com/DataDog/dd-trace-go/v2/internal/civisibility/utils" + internalconfig "github.com/DataDog/dd-trace-go/v2/internal/config" "github.com/DataDog/dd-trace-go/v2/internal/processtags" "github.com/DataDog/dd-trace-go/v2/internal/statsdtest" ) @@ -45,7 +46,9 @@ func TestConcentrator(t *testing.T) { } t.Run("start-stop", func(t *testing.T) { assert := assert.New(t) - c := newConcentrator(&config{}, bucketSize, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(err) + c := newConcentrator(cfg, bucketSize, &statsd.NoOpClientDirect{}) assert.EqualValues(atomic.LoadUint32(&c.stopped), 1) c.Start() assert.EqualValues(atomic.LoadUint32(&c.stopped), 0) @@ -64,7 +67,11 @@ func TestConcentrator(t *testing.T) { t.Run("flusher", func(t *testing.T) { t.Run("old", func(t *testing.T) { transport := newDummyTransport() - c := newConcentrator(&config{transport: transport, env: "someEnv"}, 500_000, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + cfg.env = "someEnv" + c := newConcentrator(cfg, 500_000, &statsd.NoOpClientDirect{}) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -82,7 +89,11 @@ func TestConcentrator(t *testing.T) { t.Run("recent+stats", func(t *testing.T) { transport := newDummyTransport() testStats := &statsdtest.TestStatsdClient{} - c := newConcentrator(&config{transport: transport, env: "someEnv"}, (10 * time.Second).Nanoseconds(), testStats) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + cfg.env = "someEnv" + c := newConcentrator(cfg, (10 * time.Second).Nanoseconds(), testStats) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -109,7 +120,12 @@ func TestConcentrator(t *testing.T) { t.Run("ciGitSha", func(t *testing.T) { utils.AddCITags(constants.GitCommitSHA, "DEADBEEF") transport := newDummyTransport() - c := newConcentrator(&config{transport: transport, env: "someEnv", ciVisibilityEnabled: true}, (10 * time.Second).Nanoseconds(), &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + cfg.env = "someEnv" + cfg.internalConfig.SetCiVisibilityEnabled(true, internalconfig.OriginCode) + c := newConcentrator(cfg, (10 * time.Second).Nanoseconds(), &statsd.NoOpClientDirect{}) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -123,7 +139,10 @@ func TestConcentrator(t *testing.T) { // stats should be sent if the concentrator is stopped t.Run("stop", func(t *testing.T) { transport := newDummyTransport() - c := newConcentrator(&config{transport: transport}, 500_000, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + c := newConcentrator(cfg, 500_000, &statsd.NoOpClientDirect{}) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -137,7 +156,10 @@ func TestConcentrator(t *testing.T) { processtags.Reload() transport := newDummyTransport() - c := newConcentrator(&config{transport: transport}, 500_000, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + c := newConcentrator(cfg, 500_000, &statsd.NoOpClientDirect{}) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -154,7 +176,10 @@ func TestConcentrator(t *testing.T) { processtags.Reload() transport := newDummyTransport() - c := newConcentrator(&config{transport: transport}, 500_000, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + c := newConcentrator(cfg, 500_000, &statsd.NoOpClientDirect{}) assert.Len(t, transport.Stats(), 0) ss1, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -184,7 +209,12 @@ func TestShouldObfuscate(t *testing.T) { {name: "agent version newer", tracerVersion: 2, agentVersion: 3, expectedShouldObfuscate: false}, } { t.Run(params.name, func(t *testing.T) { - c := newConcentrator(&config{transport: tsp, env: "someEnv", agent: agentFeatures{obfuscationVersion: params.agentVersion}}, bucketSize, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = tsp + cfg.env = "someEnv" + cfg.agent.obfuscationVersion = params.agentVersion + c := newConcentrator(cfg, bucketSize, &statsd.NoOpClientDirect{}) defer func(oldVersion int) { tracerObfuscationVersion = oldVersion }(tracerObfuscationVersion) tracerObfuscationVersion = params.tracerVersion assert.Equal(t, params.expectedShouldObfuscate, c.shouldObfuscate()) @@ -203,7 +233,12 @@ func TestObfuscation(t *testing.T) { resource: "GET somekey", } tsp := newDummyTransport() - c := newConcentrator(&config{transport: tsp, env: "someEnv", agent: agentFeatures{obfuscationVersion: 2}}, bucketSize, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = tsp + cfg.env = "someEnv" + cfg.agent.obfuscationVersion = 2 + c := newConcentrator(cfg, bucketSize, &statsd.NoOpClientDirect{}) defer func(oldVersion int) { tracerObfuscationVersion = oldVersion }(tracerObfuscationVersion) tracerObfuscationVersion = 2 @@ -237,7 +272,11 @@ func TestStatsByKind(t *testing.T) { s1.SetTag("span.kind", "client") s2.SetTag("span.kind", "invalid") - c := newConcentrator(&config{transport: newDummyTransport(), env: "someEnv"}, 100, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = newDummyTransport() + cfg.env = "someEnv" + c := newConcentrator(cfg, 100, &statsd.NoOpClientDirect{}) _, ok := c.newTracerStatSpan(&s1, nil) assert.True(t, ok) @@ -249,29 +288,29 @@ func TestConcentratorDefaultEnv(t *testing.T) { assert := assert.New(t) t.Run("uses-agent-default-env-when-no-tracer-env", func(t *testing.T) { - cfg := &config{ - transport: newDummyTransport(), - agent: agentFeatures{defaultEnv: "agent-prod"}, - } + cfg, err := newTestConfig() + assert.NoError(err) + cfg.transport = newDummyTransport() + cfg.agent.defaultEnv = "agent-prod" c := newConcentrator(cfg, 100, &statsd.NoOpClientDirect{}) assert.Equal("agent-prod", c.aggregationKey.Env) }) t.Run("prefers-tracer-env-over-agent-default", func(t *testing.T) { - cfg := &config{ - transport: newDummyTransport(), - env: "tracer-staging", - agent: agentFeatures{defaultEnv: "agent-prod"}, - } + cfg, err := newTestConfig() + assert.NoError(err) + cfg.transport = newDummyTransport() + cfg.env = "tracer-staging" + cfg.agent.defaultEnv = "agent-prod" c := newConcentrator(cfg, 100, &statsd.NoOpClientDirect{}) assert.Equal("tracer-staging", c.aggregationKey.Env) }) t.Run("falls-back-to-unknown-env-when-both-empty", func(t *testing.T) { - cfg := &config{ - transport: newDummyTransport(), - agent: agentFeatures{}, - } + cfg, err := newTestConfig() + assert.NoError(err) + cfg.transport = newDummyTransport() + cfg.agent = agentFeatures{} c := newConcentrator(cfg, 100, &statsd.NoOpClientDirect{}) assert.Equal("unknown-env", c.aggregationKey.Env) }) @@ -293,7 +332,11 @@ func TestStatsIncludeHTTPMethodAndEndpoint(t *testing.T) { }, } transport := newDummyTransport() - c := newConcentrator(&config{transport: transport, env: "someEnv"}, bucketSize, &statsd.NoOpClientDirect{}) + cfg, err := newTestConfig() + assert.NoError(t, err) + cfg.transport = transport + cfg.env = "someEnv" + c := newConcentrator(cfg, bucketSize, &statsd.NoOpClientDirect{}) ss, ok := c.newTracerStatSpan(&s, nil) require.True(t, ok) c.Start() diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 33b0541830..605d1db82d 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -219,7 +219,7 @@ func Start(opts ...StartOption) error { t.Stop() return nil } - if t.config.ciVisibilityEnabled && t.config.ciVisibilityNoopTracer { + if t.config.internalConfig.CiVisibilityEnabled() && t.config.ciVisibilityNoopTracer { setGlobalTracer(wrapWithCiVisibilityNoopTracer(t)) } else { setGlobalTracer(t) @@ -400,7 +400,7 @@ func newUnstartedTracer(opts ...StartOption) (t *tracer, err error) { } }() var writer traceWriter - if c.ciVisibilityEnabled { + if c.internalConfig.CiVisibilityEnabled() { writer = newCiVisibilityTraceWriter(c) } else if c.logToStdout { writer = newLogTraceWriter(c, statsd) diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index 4ee0d9e464..ff02e6257a 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -2464,8 +2464,12 @@ func TestFlush(t *testing.T) { tr.statsd.Close() tr.statsd = ts + cfg, err := newTestConfig() + assert.NoError(t, err) transport := newDummyTransport() - c := newConcentrator(&config{transport: transport, env: "someEnv"}, defaultStatsBucketSize, &statsd.NoOpClientDirect{}) + cfg.transport = transport + cfg.env = "someEnv" + c := newConcentrator(cfg, defaultStatsBucketSize, &statsd.NoOpClientDirect{}) tr.stats.Stop() tr.stats = c c.Start() diff --git a/internal/config/config.go b/internal/config/config.go index 143e1adf78..7cde338974 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/DataDog/dd-trace-go/v2/internal/civisibility/constants" "github.com/DataDog/dd-trace-go/v2/internal/telemetry" ) @@ -59,10 +60,11 @@ type Config struct { dataStreamsMonitoringEnabled bool dynamicInstrumentationEnabled bool globalSampleRate float64 - ciVisibilityEnabled bool - ciVisibilityAgentless bool - logDirectory string - traceRateLimitPerSecond float64 + // ciVisibilityEnabled controls if the tracer is loaded with CI Visibility mode. default false + ciVisibilityEnabled bool + ciVisibilityAgentless bool + logDirectory string + traceRateLimitPerSecond float64 } // loadConfig initializes and returns a new config by reading from all configured sources. @@ -94,8 +96,8 @@ func loadConfig() *Config { cfg.dataStreamsMonitoringEnabled = provider.getBool("DD_DATA_STREAMS_ENABLED", false) cfg.dynamicInstrumentationEnabled = provider.getBool("DD_DYNAMIC_INSTRUMENTATION_ENABLED", false) cfg.globalSampleRate = provider.getFloat("DD_TRACE_SAMPLE_RATE", 0.0) - cfg.ciVisibilityEnabled = provider.getBool("DD_CIVISIBILITY_ENABLED", false) - cfg.ciVisibilityAgentless = provider.getBool("DD_CIVISIBILITY_AGENTLESS_ENABLED", false) + cfg.ciVisibilityEnabled = provider.getBool(constants.CIVisibilityEnabledEnvironmentVariable, false) + cfg.ciVisibilityAgentless = provider.getBool(constants.CIVisibilityAgentlessEnabledEnvironmentVariable, false) cfg.logDirectory = provider.getString("DD_TRACE_LOG_DIRECTORY", "") cfg.traceRateLimitPerSecond = provider.getFloat("DD_TRACE_RATE_LIMIT", 0.0) @@ -134,3 +136,16 @@ func (c *Config) SetDebug(enabled bool, origin telemetry.Origin) { c.debug = enabled telemetry.RegisterAppConfig("DD_TRACE_DEBUG", enabled, origin) } + +func (c *Config) CiVisibilityEnabled() bool { + c.mu.RLock() + defer c.mu.RUnlock() + return c.ciVisibilityEnabled +} + +func (c *Config) SetCiVisibilityEnabled(enabled bool, origin telemetry.Origin) { + c.mu.Lock() + defer c.mu.Unlock() + c.ciVisibilityEnabled = enabled + telemetry.RegisterAppConfig(constants.CIVisibilityEnabledEnvironmentVariable, enabled, origin) +}