From bae47717b4afdd6ca23d889ed5be82a9b23b1f47 Mon Sep 17 00:00:00 2001 From: Eliott Bouhana Date: Fri, 31 Jan 2025 15:16:51 +0100 Subject: [PATCH] add more changes to make the telemetry client work Signed-off-by: Eliott Bouhana --- .../internal/telemetrytest/telemetry_test.go | 14 +-- ddtrace/opentelemetry/telemetry_test.go | 3 +- ddtrace/opentelemetry/tracer_test.go | 5 +- ddtrace/tracer/otel_dd_mappings_test.go | 12 ++- ddtrace/tracer/remote_config_test.go | 28 +++--- ddtrace/tracer/spancontext_test.go | 2 +- ddtrace/tracer/telemetry_test.go | 90 ++++++++----------- internal/appsec/features.go | 8 +- 8 files changed, 80 insertions(+), 82 deletions(-) diff --git a/contrib/internal/telemetrytest/telemetry_test.go b/contrib/internal/telemetrytest/telemetry_test.go index 237b582a76..1c457a22b6 100644 --- a/contrib/internal/telemetrytest/telemetry_test.go +++ b/contrib/internal/telemetrytest/telemetry_test.go @@ -16,7 +16,6 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry/telemetrytest" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,12 +24,13 @@ import ( func TestIntegrationInfo(t *testing.T) { // mux.NewRouter() uses the net/http and gorilla/mux integration client := new(telemetrytest.MockClient) - telemetry.SwapClient(client) - mux.NewRouter() - integrations := client.Integrations - require.Len(t, integrations, 2) - assert.Equal(t, integrations[0].Name, "net/http") - assert.Equal(t, integrations[1].Name, "gorilla/mux") + client.On("AppStart").Return() + client.On("MarkIntegrationAsLoaded", telemetry.Integration{Name: "net/http", Version: "", Error: ""}).Return() + client.On("MarkIntegrationAsLoaded", telemetry.Integration{Name: "gorilla/mux", Version: "", Error: ""}).Return() + telemetry.StartApp(client) + _ = mux.NewRouter() + + client.AssertExpectations(t) } type contribPkg struct { diff --git a/ddtrace/opentelemetry/telemetry_test.go b/ddtrace/opentelemetry/telemetry_test.go index 4d61b40743..d3697a285e 100644 --- a/ddtrace/opentelemetry/telemetry_test.go +++ b/ddtrace/opentelemetry/telemetry_test.go @@ -73,7 +73,7 @@ func TestTelemetry(t *testing.T) { for k, v := range test.env { t.Setenv(k, v) } - telemetryClient := new(telemetrytest.MockClient) + telemetryClient := new(telemetrytest.RecordClient) original := telemetry.GlobalClient() telemetry.SwapClient(telemetryClient) defer telemetry.SwapClient(original) @@ -82,7 +82,6 @@ func TestTelemetry(t *testing.T) { p.Tracer("") defer p.Shutdown() - assert.True(t, telemetryClient.Started) assert.Contains(t, telemetryClient.Configuration, telemetry.Configuration{Name: "trace_propagation_style_inject", Value: test.expectedInject}) assert.Contains(t, telemetryClient.Configuration, telemetry.Configuration{Name: "trace_propagation_style_extract", Value: test.expectedExtract}) }) diff --git a/ddtrace/opentelemetry/tracer_test.go b/ddtrace/opentelemetry/tracer_test.go index c6139ba5e7..c7736c4c9d 100644 --- a/ddtrace/opentelemetry/tracer_test.go +++ b/ddtrace/opentelemetry/tracer_test.go @@ -195,7 +195,7 @@ func TestShutdownOnce(t *testing.T) { } func TestSpanTelemetry(t *testing.T) { - telemetryClient := new(telemetrytest.MockClient) + telemetryClient := new(telemetrytest.RecordClient) original := telemetry.GlobalClient() telemetry.SwapClient(telemetryClient) defer telemetry.SwapClient(original) @@ -203,8 +203,7 @@ func TestSpanTelemetry(t *testing.T) { otel.SetTracerProvider(tp) tr := otel.Tracer("") _, _ = tr.Start(context.Background(), "otel.span") - telemetryClient.AssertCalled(t, "Count", telemetry.NamespaceTracers, "spans_created", telemetryTags) - telemetryClient.AssertNumberOfCalls(t, "Count", 1) + assert.NotZero(t, telemetryClient.Count(telemetry.NamespaceTracers, "spans_created", telemetryTags).Get()) } func TestConcurrentSetAttributes(_ *testing.T) { diff --git a/ddtrace/tracer/otel_dd_mappings_test.go b/ddtrace/tracer/otel_dd_mappings_test.go index 65fe5c87ec..3621fba44b 100644 --- a/ddtrace/tracer/otel_dd_mappings_test.go +++ b/ddtrace/tracer/otel_dd_mappings_test.go @@ -30,6 +30,9 @@ func TestAssessSource(t *testing.T) { }) t.Run("both", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) + metricHandle := new(telemetrytest.MockMetricHandle) + telemetryClient.On("Count", telemetry.NamespaceTracers, "otel.env.hiding", []string{"config_datadog:dd_service", "config_opentelemetry:otel_service_name"}).Return(metricHandle) + metricHandle.On("Submit", 1.0) original := telemetry.GlobalClient() telemetry.SwapClient(telemetryClient) defer telemetry.SwapClient(original) @@ -38,16 +41,21 @@ func TestAssessSource(t *testing.T) { t.Setenv("OTEL_SERVICE_NAME", "123") v := getDDorOtelConfig("service") assert.Equal(t, "abc", v) - telemetryClient.AssertCalled(t, "Count", telemetry.NamespaceTracers, "otel.env.hiding", 1.0, []string{"config_datadog:dd_service", "config_opentelemetry:otel_service_name"}, true) + telemetryClient.AssertExpectations(t) + metricHandle.AssertExpectations(t) }) t.Run("invalid-ot", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) + metricHandle := new(telemetrytest.MockMetricHandle) + telemetryClient.On("Count", telemetry.NamespaceTracers, "otel.env.invalid", []string{"config_datadog:dd_service", "config_opentelemetry:otel_service_name"}).Return(metricHandle) + metricHandle.On("Submit", 1.0) original := telemetry.GlobalClient() telemetry.SwapClient(telemetryClient) defer telemetry.SwapClient(original) t.Setenv("OTEL_LOG_LEVEL", "nonesense") v := getDDorOtelConfig("debugMode") assert.Equal(t, "", v) - telemetryClient.AssertCalled(t, "Count", telemetry.NamespaceTracers, "otel.env.invalid", 1.0, []string{"config_datadog:dd_trace_debug", "config_opentelemetry:otel_log_level"}, true) + telemetryClient.AssertExpectations(t) + metricHandle.AssertExpectations(t) }) } diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 76437404f2..965b9cca52 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -25,7 +25,7 @@ import ( func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("RC sampling rate = 0.5 is applied and can be reverted", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() @@ -96,7 +96,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("DD_TRACE_SAMPLE_RATE=0.1 and RC sampling rate = 0.2", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_SAMPLE_RATE", "0.1") tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) @@ -145,7 +145,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("DD_TRACE_SAMPLING_RULES rate=0.1 and RC trace sampling rules rate = 1.0", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_SAMPLING_RULES", `[{ "service": "my-service", @@ -205,7 +205,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("DD_TRACE_SAMPLING_RULES=0.1 and RC rule rate=1.0 and revert", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_SAMPLING_RULES", `[{ "service": "my-service", @@ -288,7 +288,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("RC rule with tags", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() @@ -334,7 +334,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("RC header tags = X-Test-Header:my-tag-name is applied and can be reverted", func(t *testing.T) { defer globalconfig.ClearHeaderTags() telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() @@ -380,7 +380,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("RC tracing_enabled = false is applied", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tr, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() @@ -437,7 +437,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { func(t *testing.T) { defer globalconfig.ClearHeaderTags() telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_HEADER_TAGS", "X-Test-Header:my-tag-name-from-env") tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) @@ -490,7 +490,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { func(t *testing.T) { defer globalconfig.ClearHeaderTags() telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer( t, @@ -544,7 +544,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("Invalid payload", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() @@ -566,7 +566,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("Service mismatch", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() @@ -586,7 +586,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("Env mismatch", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() @@ -606,7 +606,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("DD_TAGS=key0:val0,key1:val1, WithGlobalTag=key2:val2 and RC tags = key3:val3,key4:val4", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TAGS", "key0:val0,key1:val1") tracer, _, _, stop := startTestTracer( @@ -677,7 +677,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t.Run("Deleted config", func(t *testing.T) { defer globalconfig.ClearHeaderTags() telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_SAMPLE_RATE", "0.1") t.Setenv("DD_TRACE_HEADER_TAGS", "X-Test-Header:my-tag-from-env") diff --git a/ddtrace/tracer/spancontext_test.go b/ddtrace/tracer/spancontext_test.go index e6e8d7a51d..a93f5fc83f 100644 --- a/ddtrace/tracer/spancontext_test.go +++ b/ddtrace/tracer/spancontext_test.go @@ -174,7 +174,7 @@ func TestPartialFlush(t *testing.T) { t.Run("WithFlush", func(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) telemetryClient.ProductStarted(telemetry.NamespaceTracers) - defer mockGlobalClient(telemetryClient)() + defer testTelemetryClient(telemetryClient)() tracer, transport, flush, stop := startTestTracer(t) defer stop() diff --git a/ddtrace/tracer/telemetry_test.go b/ddtrace/tracer/telemetry_test.go index a021185fbc..994c27f8ac 100644 --- a/ddtrace/tracer/telemetry_test.go +++ b/ddtrace/tracer/telemetry_test.go @@ -7,7 +7,6 @@ package tracer import ( "fmt" - "reflect" "testing" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" @@ -18,7 +17,7 @@ import ( "github.com/stretchr/testify/assert" ) -func mockGlobalClient(client telemetry.Client) func() { +func testTelemetryClient(client telemetry.Client) func() { orig := telemetry.GlobalClient() telemetry.SwapClient(client) return func() { @@ -26,20 +25,10 @@ func mockGlobalClient(client telemetry.Client) func() { } } -func checkConfig(t *testing.T, cfgs []telemetry.Configuration, key string, value any) { - for _, c := range cfgs { - if c.Name == key && reflect.DeepEqual(c.Value, value) { - return - } - } - - t.Fatalf("could not find configuration key %s with value %v", key, value) -} - func TestTelemetryEnabled(t *testing.T) { t.Run("tracer start", func(t *testing.T) { - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() Start( WithDebugMode(true), @@ -61,24 +50,24 @@ func TestTelemetryEnabled(t *testing.T) { defer Stop() assert.True(t, telemetryClient.Started) - telemetryClient.AssertNumberOfCalls(t, "ApplyOps", 1) - checkConfig(t, telemetryClient.Configuration, "trace_debug_enabled", true) - checkConfig(t, telemetryClient.Configuration, "service", "test-serv") - checkConfig(t, telemetryClient.Configuration, "env", "test-env") - checkConfig(t, telemetryClient.Configuration, "runtime_metrics_enabled", true) - checkConfig(t, telemetryClient.Configuration, "stats_computation_enabled", false) - checkConfig(t, telemetryClient.Configuration, "trace_enabled", true) - checkConfig(t, telemetryClient.Configuration, "trace_span_attribute_schema", 0) - checkConfig(t, telemetryClient.Configuration, "trace_peer_service_defaults_enabled", true) - checkConfig(t, telemetryClient.Configuration, "trace_peer_service_mapping", "key:val") - checkConfig(t, telemetryClient.Configuration, "debug_stack_enabled", false) - checkConfig(t, telemetryClient.Configuration, "orchestrion_enabled", false) - checkConfig(t, telemetryClient.Configuration, "trace_sample_rate", nil) // default value is NaN which is sanitized to nil - checkConfig(t, telemetryClient.Configuration, "trace_header_tags", "key:val,key2:val2") - checkConfig(t, telemetryClient.Configuration, "trace_sample_rules", + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_debug_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "service", "test-serv") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "env", "test-env") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "runtime_metrics_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "stats_computation_enabled", false) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_span_attribute_schema", 0) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_peer_service_defaults_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_peer_service_mapping", "key:val") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "debug_stack_enabled", false) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "orchestrion_enabled", false) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_sample_rate", nil) // default value is NaN which is sanitized to nil + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_header_tags", "key:val,key2:val2") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_sample_rules", `[{"service":"test-serv","name":"op-name","resource":"resource-*","sample_rate":0.1,"tags":{"tag-a":"tv-a??"}}]`) - checkConfig(t, telemetryClient.Configuration, "span_sample_rules", "[]") - assert.NotZero(t, telemetryClient.Distribution(telemetry.NamespaceTracers, "init_time", nil).Get()) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "span_sample_rules", "[]") + + assert.NotZero(t, telemetryClient.Distribution(telemetry.NamespaceGeneral, "init_time", nil).Get()) }) t.Run("telemetry customer or dynamic rules", func(t *testing.T) { @@ -90,8 +79,8 @@ func TestTelemetryEnabled(t *testing.T) { } rule.Provenance = prov - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() Start(WithService("test-serv"), WithSamplingRules([]SamplingRule{rule}), ) @@ -99,7 +88,7 @@ func TestTelemetryEnabled(t *testing.T) { defer Stop() assert.True(t, telemetryClient.Started) - checkConfig(t, telemetryClient.Configuration, "trace_sample_rules", + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_sample_rules", fmt.Sprintf(`[{"service":"test-serv","name":"op-name","resource":"resource-*","sample_rate":0.1,"tags":{"tag-a":"tv-a??"},"provenance":"%s"}]`, prov.String())) } }) @@ -115,8 +104,8 @@ func TestTelemetryEnabled(t *testing.T) { rules[i].Provenance = Local } - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() Start(WithService("test-serv"), WithSamplingRules(rules), ) @@ -124,15 +113,15 @@ func TestTelemetryEnabled(t *testing.T) { defer Stop() assert.True(t, telemetryClient.Started) - checkConfig(t, telemetryClient.Configuration, "trace_sample_rules", + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_sample_rules", `[{"service":"test-serv","name":"op-name","resource":"resource-*","sample_rate":0.1,"tags":{"tag-a":"tv-a??"}}]`) - checkConfig(t, telemetryClient.Configuration, "span_sample_rules", + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "span_sample_rules", `[{"service":"test-serv","name":"op-name","sample_rate":0.1}]`) }) t.Run("tracer start with empty rules", func(t *testing.T) { - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() t.Setenv("DD_TRACE_SAMPLING_RULES", "") t.Setenv("DD_SPAN_SAMPLING_RULES", "") @@ -146,13 +135,13 @@ func TestTelemetryEnabled(t *testing.T) { c.Value = telemetry.SanitizeConfigValue(c.Value) cfgs = append(cfgs) } - checkConfig(t, cfgs, "trace_sample_rules", "[]") - checkConfig(t, cfgs, "span_sample_rules", "[]") + telemetrytest.CheckConfig(t, cfgs, "trace_sample_rules", "[]") + telemetrytest.CheckConfig(t, cfgs, "span_sample_rules", "[]") }) t.Run("profiler start, tracer start", func(t *testing.T) { - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() profiler.Start() defer profiler.Stop() Start( @@ -160,18 +149,17 @@ func TestTelemetryEnabled(t *testing.T) { ) defer globalconfig.SetServiceName("") defer Stop() - checkConfig(t, telemetryClient.Configuration, "service", "test-serv") - telemetryClient.AssertNumberOfCalls(t, "ApplyOps", 2) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "service", "test-serv") }) t.Run("orchestrion telemetry", func(t *testing.T) { - telemetryClient := new(telemetrytest.MockClient) - defer mockGlobalClient(telemetryClient)() + telemetryClient := new(telemetrytest.RecordClient) + defer testTelemetryClient(telemetryClient)() Start(WithOrchestrion(map[string]string{"k1": "v1", "k2": "v2"})) defer Stop() - checkConfig(t, telemetryClient.Configuration, "orchestrion_enabled", true) - checkConfig(t, telemetryClient.Configuration, "orchestrion_k1", "v1") - checkConfig(t, telemetryClient.Configuration, "orchestrion_k2", "v2") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "orchestrion_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "orchestrion_k1", "v1") + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "orchestrion_k2", "v2") }) } diff --git a/internal/appsec/features.go b/internal/appsec/features.go index ca286de742..e9ef073378 100644 --- a/internal/appsec/features.go +++ b/internal/appsec/features.go @@ -66,8 +66,12 @@ func (a *appsec) SwapRootOperation() error { oldFeatures := a.features a.features = newFeatures - log.Debug("appsec: stopping the following features: %v", oldFeatures) - log.Debug("appsec: starting the following features: %v", newFeatures) + if len(oldFeatures) > 0 { + log.Debug("appsec: stopping the following features: %v", oldFeatures) + } + if len(newFeatures) > 0 { + log.Debug("appsec: starting the following features: %v", newFeatures) + } dyngo.SwapRootOperation(newRoot)