From 014bc0ed66d74a3ad155bbcc6824e7eb8f41a5ae Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Thu, 9 Jan 2025 14:48:15 -0800 Subject: [PATCH 1/3] reverting env/service filtering in tracingconfigpoller --- .../trace/core/TracingConfigPoller.java | 29 ---- .../datadog/trace/core/CoreTracerTest.groovy | 129 ------------------ 2 files changed, 158 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java b/dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java index 9a3223b3fa1..bf5421617e7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java @@ -106,24 +106,6 @@ public void accept(ConfigKey configKey, byte[] content, PollingRateHinter hinter Okio.buffer(Okio.source(new ByteArrayInputStream(content)))); if (null != overrides && null != overrides.libConfig) { - ServiceTarget serviceTarget = overrides.serviceTarget; - if (serviceTarget != null) { - String targetService = serviceTarget.service; - String thisService = Config.get().getServiceName(); - if (targetService != null && !targetService.equalsIgnoreCase(thisService)) { - log.debug( - "Skipping config for service {}. Current service is {}", - targetService, - thisService); - throw new IllegalArgumentException("service mismatch"); - } - String targetEnv = serviceTarget.env; - String thisEnv = Config.get().getEnv(); - if (targetEnv != null && !targetEnv.equalsIgnoreCase(thisEnv)) { - log.debug("Skipping config for env {}. Current env is {}", targetEnv, thisEnv); - throw new IllegalArgumentException("env mismatch"); - } - } receivedOverrides = true; applyConfigOverrides(checkConfig(overrides.libConfig)); if (log.isDebugEnabled()) { @@ -266,21 +248,10 @@ private Map parseTagListToMap(List input) { } static final class ConfigOverrides { - @Json(name = "service_target") - public ServiceTarget serviceTarget; - @Json(name = "lib_config") public LibConfig libConfig; } - static final class ServiceTarget { - @Json(name = "service") - public String service; - - @Json(name = "env") - public String env; - } - static final class LibConfig { @Json(name = "tracing_enabled") public Boolean tracingEnabled; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy index f91b8b7b580..ce7f2e90fc5 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy @@ -570,135 +570,6 @@ class CoreTracerTest extends DDCoreSpecification { tracer?.close() } - - def "reject configuration when target service+env mismatch"() { - setup: - injectSysConfig(SERVICE_NAME, service) - injectSysConfig(ENV, env) - - def key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config") - def poller = Mock(ConfigurationPoller) - def sco = new SharedCommunicationObjects( - okHttpClient: Mock(OkHttpClient), - monitoring: Mock(Monitoring), - agentUrl: HttpUrl.get('https://example.com'), - featuresDiscovery: Mock(DDAgentFeaturesDiscovery), - configurationPoller: poller - ) - - def updater - - when: - def tracer = CoreTracer.builder() - .sharedCommunicationObjects(sco) - .pollForTracingConfiguration() - .build() - - then: - 1 * poller.addListener(Product.APM_TRACING, _ as ProductListener) >> { - updater = it[1] // capture config updater for further testing - } - and: - tracer.captureTraceConfig().serviceMapping == [:] - - when: - updater.accept(key, """ - { - "service_target": { - "service": "${targetService}", - "env": "${targetEnv}" - }, - "lib_config": - { - "tracing_service_mapping": - [{ - "from_key": "foobar", - "to_name": "bar" - }] - } - } - """.getBytes(StandardCharsets.UTF_8), null) - updater.commit() - - then: "configuration should not be applied" - tracer.captureTraceConfig().serviceMapping == [:] - and: - thrown(IllegalArgumentException) - - cleanup: - tracer?.close() - - where: - service | env | targetService | targetEnv - "service" | "env" | "service_1" | "env" - "service" | "env" | "service" | "env_1" - "service" | "env" | "service_2" | "env_2" - } - - def "accept configuration when target service+env match case-insensitive"() { - setup: - injectSysConfig(SERVICE_NAME, service) - injectSysConfig(ENV, env) - - def key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config") - def poller = Mock(ConfigurationPoller) - def sco = new SharedCommunicationObjects( - okHttpClient: Mock(OkHttpClient), - monitoring: Mock(Monitoring), - agentUrl: HttpUrl.get('https://example.com'), - featuresDiscovery: Mock(DDAgentFeaturesDiscovery), - configurationPoller: poller - ) - - def updater - - when: - def tracer = CoreTracer.builder() - .sharedCommunicationObjects(sco) - .pollForTracingConfiguration() - .build() - - then: - 1 * poller.addListener(Product.APM_TRACING, _ as ProductListener) >> { - updater = it[1] // capture config updater for further testing - } - and: - tracer.captureTraceConfig().serviceMapping == [:] - - when: - updater.accept(key, """ - { - "service_target": { - "service": "${targetService}", - "env": "${targetEnv}" - }, - "lib_config": - { - "tracing_service_mapping": - [{ - "from_key": "foobar", - "to_name": "bar" - }] - } - } - """.getBytes(StandardCharsets.UTF_8), null) - updater.commit() - - then: "configuration should be applied" - tracer.captureTraceConfig().serviceMapping == ["foobar":"bar"] - - cleanup: - tracer?.close() - - where: - service | env | targetService | targetEnv - "service" | "env" | "service" | "env" - "service" | "env" | "SERVICE" | "env" - "service" | "env" | "service" | "ENV" - "service" | "env" | "SERVICE" | "ENV" - "SERVICE" | "ENV" | "service" | "env" - } - def "flushes on tracer close if configured to do so"() { given: def writer = new WriterWithExplicitFlush() From bf5da72d1d193465aeba324072c6ce3892ff2f19 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 17 Jan 2025 13:20:52 -0500 Subject: [PATCH 2/3] adding a unit test to verify filtering doesn't exist --- .../datadog/trace/core/CoreTracerTest.groovy | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy index ce7f2e90fc5..0d5df6500b3 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy @@ -582,6 +582,68 @@ class CoreTracerTest extends DDCoreSpecification { then: !writer.flushedTraces.empty } + + def "verify no filtering of service/env"() { + setup: + injectSysConfig(SERVICE_NAME, service) + injectSysConfig(ENV, env) + + def key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config") + def poller = Mock(ConfigurationPoller) + def sco = new SharedCommunicationObjects( + okHttpClient: Mock(OkHttpClient), + monitoring: Mock(Monitoring), + agentUrl: HttpUrl.get('https://example.com'), + featuresDiscovery: Mock(DDAgentFeaturesDiscovery), + configurationPoller: poller + ) + + def updater + + when: + def tracer = CoreTracer.builder() + .sharedCommunicationObjects(sco) + .pollForTracingConfiguration() + .build() + + then: + 1 * poller.addListener(Product.APM_TRACING, _ as ProductListener) >> { + updater = it[1] // capture config updater for further testing + } + and: + tracer.captureTraceConfig().serviceMapping == [:] + + when: + updater.accept(key, """ + { + "service_target": { + "service": "${targetService}", + "env": "${targetEnv}" + }, + "lib_config": + { + "tracing_service_mapping": + [{ + "from_key": "foobar", + "to_name": "bar" + }] + } + } + """.getBytes(StandardCharsets.UTF_8), null) + updater.commit() + + then: "configuration should be applied" + tracer.captureTraceConfig().serviceMapping == ["foobar":"bar"] + + cleanup: + tracer?.close() + + where: + service | env | targetService | targetEnv + "service" | "env" | "service_1" | "env" + "service" | "env" | "service" | "env_1" + "service" | "env" | "service_2" | "env_2" + } } class WriterWithExplicitFlush implements datadog.trace.common.writer.Writer { From b07e2ead0dcface3dade5c140bf381de0277cd2e Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 17 Jan 2025 13:22:14 -0500 Subject: [PATCH 3/3] updating test name --- .../src/test/groovy/datadog/trace/core/CoreTracerTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy index 0d5df6500b3..5d7dc2b915f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreTracerTest.groovy @@ -583,7 +583,7 @@ class CoreTracerTest extends DDCoreSpecification { !writer.flushedTraces.empty } - def "verify no filtering of service/env"() { + def "verify no filtering of service/env when mismatched with DD_SERVICE/DD_ENV"() { setup: injectSysConfig(SERVICE_NAME, service) injectSysConfig(ENV, env)