From c450c9f678b0f9ca7fab35ee5bbde2b4a2201a31 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Wed, 15 Sep 2021 10:01:13 -0700 Subject: [PATCH 01/32] Enable DT by default --- .../agent/config/DistributedTracingConfig.java | 2 +- newrelic-agent/src/main/resources/newrelic.yml | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/DistributedTracingConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/DistributedTracingConfig.java index 840966445f..177f12e724 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/DistributedTracingConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/DistributedTracingConfig.java @@ -11,7 +11,7 @@ public class DistributedTracingConfig extends BaseConfig { - private static final boolean DEFAULT_DISTRIBUTED_TRACING = false; + private static final boolean DEFAULT_DISTRIBUTED_TRACING = true; private static final String SYSTEM_PROPERTY_ROOT = "newrelic.config.distributed_tracing."; public static final String ENABLED = "enabled"; diff --git a/newrelic-agent/src/main/resources/newrelic.yml b/newrelic-agent/src/main/resources/newrelic.yml index 3bd0df498f..e659f5f022 100644 --- a/newrelic-agent/src/main/resources/newrelic.yml +++ b/newrelic-agent/src/main/resources/newrelic.yml @@ -205,13 +205,21 @@ common: &default_settings max_samples_stored: 2000 # Distributed tracing lets you see the path that a request takes through your distributed system. - # Enabling distributed tracing changes the behavior of some New Relic features, so carefully consult the transition - # guide before you enable this feature: https://docs.newrelic.com/docs/apm/distributed-tracing/getting-started/transition-guide-distributed-tracing + # This replaces the deprecated legacy Cross Application Tracing. + # https://docs.newrelic.com/docs/apm/transactions/cross-application-traces/introduction-cross-application-traces/ distributed_tracing: - # Set to true to enable distributed tracing. + # Set to false to disable distributed tracing. + # Default is true. + enabled: true + + # Agent versions 5.10.0+ utilize both the newrelic header and W3C Trace Context headers for distributed tracing. + # The newrelic distributed tracing header allows interoperability with older agents that don't support W3C Trace Context headers. + # Agent versions that support W3C Trace Context headers will prioritize them over newrelic headers for distributed tracing. + # If you do not want to utilize the newrelic header, setting this to true will result in the agent excluding the newrelic header + # and only using W3C Trace Context headers for distributed tracing. # Default is false. - enabled: false + exclude_newrelic_header: false # Cross Application Tracing adds request and response headers to # external calls using supported HTTP libraries to provide better From 4b6507d8636cecf9129f7ddc61f3357384411c50 Mon Sep 17 00:00:00 2001 From: xxia Date: Wed, 15 Sep 2021 22:09:20 -0700 Subject: [PATCH 02/32] change default max spans and make it configurable --- .../java/com/newrelic/agent/config/SpanEventsConfig.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java index 7914d1171d..86c3dc5309 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java @@ -11,15 +11,16 @@ public class SpanEventsConfig extends BaseConfig { - public static final int DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST = 1000; + public static final int DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST = 2000; public static final int DEFAULT_TARGET_SAMPLES_STORED = 10; public static final boolean DEFAULT_CROSS_PROCESS_ONLY = false; public static final String COLLECT_SPAN_EVENTS = "collect_span_events"; + public static final String ENABLED = "enabled"; + public static final String MAX_SPAN_EVENTS_PER_HARVEST = "max_samples_stored"; private static final String ROOT = "newrelic.config."; private static final String SPAN_EVENTS = "span_events."; - private static final String SYSTEM_PROPERTY_ROOT = ROOT + SPAN_EVENTS; - public static final String ENABLED = "enabled"; + public static final String SYSTEM_PROPERTY_ROOT = ROOT + SPAN_EVENTS; private static final String TARGET_SAMPLES_STORED = "target_samples_stored"; private static final String CROSS_PROCESS_ONLY = "cross_process_only"; private static final boolean DEFAULT_COLLECT_SPANS = false; @@ -39,7 +40,7 @@ public class SpanEventsConfig extends BaseConfig { public SpanEventsConfig(Map props, boolean dtEnabled) { super(props, SYSTEM_PROPERTY_ROOT); this.dtEnabled = dtEnabled; - this.maxSamplesStored = DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST; + this.maxSamplesStored = getProperty(MAX_SPAN_EVENTS_PER_HARVEST, DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST); this.enabled = initEnabled(maxSamplesStored); this.targetSamplesStored = getProperty(TARGET_SAMPLES_STORED, DEFAULT_TARGET_SAMPLES_STORED); this.crossProcessOnly = getProperty(CROSS_PROCESS_ONLY, DEFAULT_CROSS_PROCESS_ONLY); From dd1ff87408d7bf43c8e7dafd4afb561de33f67a5 Mon Sep 17 00:00:00 2001 From: xxia Date: Wed, 15 Sep 2021 22:09:53 -0700 Subject: [PATCH 03/32] remove unused constants --- .../java/com/newrelic/agent/config/SpanEventsConfig.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java index 86c3dc5309..83a25657b1 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java @@ -25,12 +25,6 @@ public class SpanEventsConfig extends BaseConfig { private static final String CROSS_PROCESS_ONLY = "cross_process_only"; private static final boolean DEFAULT_COLLECT_SPANS = false; - // Span event system properties with root - public static final String SYSTEM_PROPERTY_SPAN_EVENTS_ENABLED = SYSTEM_PROPERTY_ROOT + ENABLED; - public static final String SPAN_EVENTS_ENABLED = SPAN_EVENTS + ENABLED; - // EnvironmentFacade variables - public static final String ENABLED_ENV_KEY = "NEW_RELIC_SPAN_EVENTS_ENABLED"; - private final boolean dtEnabled; private final int maxSamplesStored; private final boolean enabled; @@ -47,7 +41,7 @@ public SpanEventsConfig(Map props, boolean dtEnabled) { } private boolean initEnabled(int maxSamplesStored) { - Boolean configEnabled = getProperty(ENABLED, dtEnabled) && dtEnabled; + boolean configEnabled = getProperty(ENABLED, dtEnabled) && dtEnabled; boolean collectSpanEventsFromCollector = getProperty(COLLECT_SPAN_EVENTS, DEFAULT_COLLECT_SPANS); return maxSamplesStored > 0 && configEnabled && collectSpanEventsFromCollector; } From d960ee46ffcf11d39a8e019e40c1b5c6d411d0e9 Mon Sep 17 00:00:00 2001 From: xxia Date: Wed, 15 Sep 2021 22:10:51 -0700 Subject: [PATCH 04/32] add tests --- .../agent/config/SpanEventsConfigTest.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java new file mode 100644 index 0000000000..e26784c5b7 --- /dev/null +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java @@ -0,0 +1,104 @@ +package com.newrelic.agent.config; + +import com.newrelic.agent.Mocks; +import org.junit.After; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +import static com.newrelic.agent.config.SpanEventsConfig.*; +import static org.junit.Assert.*; + +public class SpanEventsConfigTest { + + @After + public void after() { + SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider()); + } + + @Test + public void isEnabledByDefault() { + //given + Map serverSettings = new HashMap<>(); + ServerProp serverProp = ServerProp.createPropObject(true); + serverSettings.put(COLLECT_SPAN_EVENTS, serverProp); + //when + SpanEventsConfig config = new SpanEventsConfig(serverSettings, true); + //then + assertTrue("SpanEventsConfig isEnabled should be true", config.isEnabled()); + } + + @Test + public void maxSamplesStoredShouldBeDefault() { + //given + Map localSettings = new HashMap<>(); + //when + SpanEventsConfig config = new SpanEventsConfig(localSettings, true); + //then + assertEquals("Max samples stored should be " + DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST, + DEFAULT_MAX_SPAN_EVENTS_PER_HARVEST, config.getMaxSamplesStored()); + } + + @Test + public void maxSamplesStoredOverrideBySystemProperty() { + //given + int customMaxSamples = 1000; + setMaxSamplesViaSystemProp(customMaxSamples); + Map localSettings = new HashMap<>(); + //when + SpanEventsConfig config = new SpanEventsConfig(localSettings, true); + //then + assertEquals("Max samples stored should be " + customMaxSamples, + customMaxSamples, config.getMaxSamplesStored()); + } + + @Test + public void maxSamplesStoredOverrideByEnvironmentProperty() { + //given + int customMaxSamples = 3000; + setMaxSamplesViaEnvProperty(customMaxSamples); + Map localSettings = new HashMap<>(); + //when + SpanEventsConfig config = new SpanEventsConfig(localSettings, true); + //then + assertEquals("Max samples stored should be " + customMaxSamples, + customMaxSamples, config.getMaxSamplesStored()); + } + + @Test + public void maxSamplesStoredEnvironmentPropertyOverrideByServerProp() { + //given + int serverSideMaxSamples = 10000; + Map serverSettings = new HashMap<>(); + ServerProp serverProp = ServerProp.createPropObject(serverSideMaxSamples); + serverSettings.put(COLLECT_SPAN_EVENTS, serverProp); + int customMaxSamples = 1000000; + setMaxSamplesViaEnvProperty(customMaxSamples); + Map localSettings = new HashMap<>(); + //when + SpanEventsConfig config = new SpanEventsConfig(localSettings, true); + //then + assertEquals("Max samples stored should be " + serverSideMaxSamples, + serverSideMaxSamples, config.getMaxSamplesStored()); + } + + private void setMaxSamplesViaSystemProp(int customMaxSamples) { + Map properties = new HashMap<>(); + String key = SYSTEM_PROPERTY_ROOT + MAX_SPAN_EVENTS_PER_HARVEST; + String val = String.valueOf(customMaxSamples); + properties.put(key, val); + Mocks.createSystemPropertyProvider(properties); + } + + private void setMaxSamplesViaEnvProperty(int customMaxSamples) { + Map sysProperties = new HashMap<>(); + Map envProperties = new HashMap<>(); + String key = SYSTEM_PROPERTY_ROOT + MAX_SPAN_EVENTS_PER_HARVEST; + String val = String.valueOf(customMaxSamples); + envProperties.put(key, val); + SystemPropertyProvider testProvider = Mocks.createSystemPropertyProvider(sysProperties,envProperties); + } + + +} From c9c7be2bcccf7a2b67e4d8eeac7c762292d7f2b2 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 12:16:27 -0700 Subject: [PATCH 05/32] remove server side test --- .../agent/config/SpanEventsConfigTest.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java index e26784c5b7..0167b21b2d 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java @@ -67,20 +67,14 @@ public void maxSamplesStoredOverrideByEnvironmentProperty() { } @Test - public void maxSamplesStoredEnvironmentPropertyOverrideByServerProp() { + public void maxSamplesStoredCanBeReset() { //given - int serverSideMaxSamples = 10000; - Map serverSettings = new HashMap<>(); - ServerProp serverProp = ServerProp.createPropObject(serverSideMaxSamples); - serverSettings.put(COLLECT_SPAN_EVENTS, serverProp); - int customMaxSamples = 1000000; - setMaxSamplesViaEnvProperty(customMaxSamples); Map localSettings = new HashMap<>(); //when SpanEventsConfig config = new SpanEventsConfig(localSettings, true); - //then - assertEquals("Max samples stored should be " + serverSideMaxSamples, - serverSideMaxSamples, config.getMaxSamplesStored()); + config.setMaxSamplesStoredByServerProp(10000L); + assertEquals("Max samples stored should be " + 10000L, + 10000L, config.getMaxSamplesStored()); } private void setMaxSamplesViaSystemProp(int customMaxSamples) { From ef93c6f1c50f1dab5fbf163e32b5335034d056b5 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 12:24:36 -0700 Subject: [PATCH 06/32] update span config with server side prop --- .../com/newrelic/agent/config/AgentConfigFactory.java | 3 +++ .../com/newrelic/agent/config/AgentConfigImpl.java | 11 +++++++++++ .../com/newrelic/agent/config/SpanEventsConfig.java | 8 +++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java index 0fd9706695..6027b919ee 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java @@ -21,6 +21,8 @@ import java.util.List; import java.util.Map; +import static com.newrelic.agent.config.SpanEventsConfig.*; + public class AgentConfigFactory { public static final String AGENT_CONFIG = "agent_config"; @@ -182,6 +184,7 @@ public static void mergeServerData(Map settings, Map props) { propsWithSystemProps.putAll(SystemPropertyFactory.getSystemPropertyProvider().getNewRelicEnvVarsWithoutPrefix()); flatten("", propsWithSystemProps, flattenedProps); checkHighSecurityPropsInFlattened(flattenedProps); + checkSpanEventHarvestLimit(); this.flattenedProperties = Collections.unmodifiableMap(flattenedProps); this.waitForTransactionsInMillis = getProperty(WAIT_FOR_TRANSACTIONS, DEFAULT_WAIT_FOR_TRANSACTIONS); this.customInstrumentationEditorAllowed = getProperty(LaspPolicies.LASP_CUSTOM_INSTRUMENTATION_EDITOR, !highSecurity); @@ -524,6 +527,14 @@ private void checkHighSecurityPropsInFlattened(Map flattenedProp } } + private void checkSpanEventHarvestLimit() { + Map spanEventHarvestLimits = getProperty(SERVER_SPAN_HARVEST_CONFIG); + if (spanEventHarvestLimits != null) { + Long harvestLimit = (Long) spanEventHarvestLimits.get(SERVER_SPAN_HARVEST_LIMIT); + spanEventsConfig.setMaxSamplesStoredByServerProp(harvestLimit); + } + } + @SuppressWarnings("unchecked") private void flatten(String prefix, Map source, Map dest) { for (Map.Entry e : source.entrySet()) { diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java index 83a25657b1..0247b046e1 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java @@ -18,6 +18,8 @@ public class SpanEventsConfig extends BaseConfig { public static final String COLLECT_SPAN_EVENTS = "collect_span_events"; public static final String ENABLED = "enabled"; public static final String MAX_SPAN_EVENTS_PER_HARVEST = "max_samples_stored"; + public static final String SERVER_SPAN_HARVEST_CONFIG = "span_event_harvest_config"; + public static final String SERVER_SPAN_HARVEST_LIMIT = "harvest_limit"; private static final String ROOT = "newrelic.config."; private static final String SPAN_EVENTS = "span_events."; public static final String SYSTEM_PROPERTY_ROOT = ROOT + SPAN_EVENTS; @@ -26,7 +28,7 @@ public class SpanEventsConfig extends BaseConfig { private static final boolean DEFAULT_COLLECT_SPANS = false; private final boolean dtEnabled; - private final int maxSamplesStored; + private int maxSamplesStored; private final boolean enabled; private final int targetSamplesStored; private final boolean crossProcessOnly; @@ -54,6 +56,10 @@ public int getMaxSamplesStored() { return maxSamplesStored; } + public void setMaxSamplesStoredByServerProp(Long harvestLimit) { + this.maxSamplesStored = harvestLimit.intValue(); + } + public int getTargetSamplesStored() { return targetSamplesStored; } From 46ae71410d49d54272a76281fcdb592dc3015e6e Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 12:26:34 -0700 Subject: [PATCH 07/32] a bit of clean up --- .../main/java/com/newrelic/agent/config/AgentConfigFactory.java | 2 +- .../main/java/com/newrelic/agent/config/AgentConfigImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java index 6027b919ee..1fe7210885 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigFactory.java @@ -112,7 +112,7 @@ public static void mergeServerData(Map settings, Map attributesInclude = attributesIncludeEnabled - ? settingsConfig.getAttributesConfig().attributesRootInclude() : Collections.emptyList(); + ? settingsConfig.getAttributesConfig().attributesRootInclude() : Collections.emptyList(); String attributesIncludeSecure = Joiner.on(",").join(attributesInclude); Boolean captureMessageParameters = getLaspValue(laspData, LaspPolicies.LASP_MESSAGE_PARAMETERS, true); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java index fc41984851..d7a8d8280f 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java @@ -992,7 +992,7 @@ private Object findPropertyInMap(String[] property, Map map) { } if (result instanceof Map) { Map resultMap = (Map) result; - result = resultMap.containsKey(component) ? resultMap.get(component) : null; + result = resultMap.getOrDefault(component, null); } } From fd0e61943cb3cde8b32d790a447f9aa84153b484 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 14:49:28 -0700 Subject: [PATCH 08/32] refactor type and method name --- .../java/com/newrelic/agent/config/AgentConfigImpl.java | 6 +++--- .../java/com/newrelic/agent/config/SpanEventsConfig.java | 4 ++-- .../com/newrelic/agent/config/SpanEventsConfigTest.java | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java index d7a8d8280f..8c3e385ded 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/AgentConfigImpl.java @@ -352,7 +352,7 @@ private AgentConfigImpl(Map props) { propsWithSystemProps.putAll(SystemPropertyFactory.getSystemPropertyProvider().getNewRelicEnvVarsWithoutPrefix()); flatten("", propsWithSystemProps, flattenedProps); checkHighSecurityPropsInFlattened(flattenedProps); - checkSpanEventHarvestLimit(); + maybeSetServerSpanHarvestLimit(); this.flattenedProperties = Collections.unmodifiableMap(flattenedProps); this.waitForTransactionsInMillis = getProperty(WAIT_FOR_TRANSACTIONS, DEFAULT_WAIT_FOR_TRANSACTIONS); this.customInstrumentationEditorAllowed = getProperty(LaspPolicies.LASP_CUSTOM_INSTRUMENTATION_EDITOR, !highSecurity); @@ -527,11 +527,11 @@ private void checkHighSecurityPropsInFlattened(Map flattenedProp } } - private void checkSpanEventHarvestLimit() { + private void maybeSetServerSpanHarvestLimit() { Map spanEventHarvestLimits = getProperty(SERVER_SPAN_HARVEST_CONFIG); if (spanEventHarvestLimits != null) { Long harvestLimit = (Long) spanEventHarvestLimits.get(SERVER_SPAN_HARVEST_LIMIT); - spanEventsConfig.setMaxSamplesStoredByServerProp(harvestLimit); + spanEventsConfig.setMaxSamplesStoredByServerProp(harvestLimit.intValue()); } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java index 0247b046e1..eebdbcd098 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/SpanEventsConfig.java @@ -56,8 +56,8 @@ public int getMaxSamplesStored() { return maxSamplesStored; } - public void setMaxSamplesStoredByServerProp(Long harvestLimit) { - this.maxSamplesStored = harvestLimit.intValue(); + public void setMaxSamplesStoredByServerProp(int harvestLimit) { + this.maxSamplesStored = harvestLimit; } public int getTargetSamplesStored() { diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java index 0167b21b2d..81613f8452 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/SpanEventsConfigTest.java @@ -72,9 +72,9 @@ public void maxSamplesStoredCanBeReset() { Map localSettings = new HashMap<>(); //when SpanEventsConfig config = new SpanEventsConfig(localSettings, true); - config.setMaxSamplesStoredByServerProp(10000L); - assertEquals("Max samples stored should be " + 10000L, - 10000L, config.getMaxSamplesStored()); + config.setMaxSamplesStoredByServerProp(10000); + assertEquals("Max samples stored should be " + 10000, + 10000, config.getMaxSamplesStored()); } private void setMaxSamplesViaSystemProp(int customMaxSamples) { From 4c497a17947c427b693b3bb01e437bc0e0602252 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 15:09:35 -0700 Subject: [PATCH 09/32] send span limit metric --- .../src/main/java/com/newrelic/agent/Harvestable.java | 9 +++++++++ .../src/main/java/com/newrelic/agent/MetricNames.java | 1 + 2 files changed, 10 insertions(+) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/Harvestable.java b/newrelic-agent/src/main/java/com/newrelic/agent/Harvestable.java index 6dc4302943..ed1156ea6b 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/Harvestable.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/Harvestable.java @@ -9,6 +9,7 @@ import com.newrelic.agent.service.EventService; import com.newrelic.agent.service.ServiceFactory; +import com.newrelic.agent.service.analytics.SpanEventsServiceImpl; import com.newrelic.agent.stats.StatsEngine; import com.newrelic.agent.stats.StatsWork; import com.newrelic.agent.stats.StatsWorks; @@ -71,8 +72,16 @@ public void configure(long reportPeriodInMillis, int maxSamplesStored) { if (maxSamplesStored != service.getMaxSamplesStored()) { service.setMaxSamplesStored(maxSamplesStored); + maybeSendSpanLimitMetric(maxSamplesStored); service.harvestEvents(appName); service.clearReservoir(); } } + + private void maybeSendSpanLimitMetric(int maxSamplesStored) { + if (service instanceof SpanEventsServiceImpl) { + ServiceFactory.getStatsService().doStatsWork( + StatsWorks.getRecordMetricWork(MetricNames.SUPPORTABILITY_SPAN_EVENT_LIMIT, maxSamplesStored)); + } + } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/MetricNames.java b/newrelic-agent/src/main/java/com/newrelic/agent/MetricNames.java index 36f5c76e08..8404ca95b4 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/MetricNames.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/MetricNames.java @@ -434,6 +434,7 @@ public class MetricNames { public static final String SUPPORTABILITY_SPAN_EVENTS = "Supportability/SpanEvents"; // feature is enabled public static final String SUPPORTABILITY_SPAN_EVENT_TOTAL_EVENTS_SENT = "Supportability/SpanEvent/TotalEventsSent"; public static final String SUPPORTABILITY_SPAN_EVENT_TOTAL_EVENTS_SEEN = "Supportability/SpanEvent/TotalEventsSeen"; + public static final String SUPPORTABILITY_SPAN_EVENT_LIMIT = "Supportability/SpanEvent/Limit"; public static final String SUPPORTABILITY_SPAN_EVENT_TOTAL_EVENTS_DISCARDED = "Supportability/SpanEvent/Discarded"; public static final String SUPPORTABILITY_INTERNAL_CUSTOM_EVENTS_TOTAL_EVENTS_SENT = "Supportability/InternalCustomEvents/TotalEventsSent"; From d101ced68397c8009fdbce0d831e5af2cf166a03 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 15:38:40 -0700 Subject: [PATCH 10/32] add and use SpanHarvestConfig to set limits --- .../java/com/newrelic/agent/HarvestServiceImpl.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java index 1169661f84..ae249a1a6c 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java @@ -29,6 +29,9 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; +import static com.newrelic.agent.config.SpanEventsConfig.*; +import static com.newrelic.agent.transport.CollectorMethods.*; + /** * This class is responsible for running the harvest tasks. There is one harvest task per RPM service. A harvest task * reports metric data to the server. @@ -81,6 +84,7 @@ public void startHarvest(IRPMService rpmService) { @VisibleForTesting public void startHarvestables(IRPMService rpmService, AgentConfig config) { Map eventHarvestConfig = config.getProperty(AgentConfigFactory.EVENT_HARVEST_CONFIG); + Map spanHarvestConfig = config.getProperty(SERVER_SPAN_HARVEST_CONFIG); if (eventHarvestConfig == null) { ServiceFactory.getStatsService().doStatsWork(StatsWorks.getIncrementCounterWork( MetricNames.SUPPORTABILITY_CONNECT_MISSING_EVENT_DATA, 1)); @@ -106,6 +110,15 @@ public void startHarvestables(IRPMService rpmService, AgentConfig config) { Agent.LOG.log(Level.FINE, "event_harvest_config from collector was null. Using default value: {0} for: {1}", maxSamplesStored, tracker.harvestable.getEndpointMethodName()); } + + if (spanHarvestConfig != null && tracker.harvestable.getEndpointMethodName().equals(SPAN_EVENT_DATA)) { + Long harvestLimit = (Long) spanHarvestConfig.get(SERVER_SPAN_HARVEST_LIMIT); + reportPeriodInMillis = (Long) spanHarvestConfig.get(REPORT_PERIOD_MS); + if (harvestLimit != null) { + maxSamplesStored = harvestLimit.intValue(); + reportPeriodInMillis = (long) spanHarvestConfig.get(REPORT_PERIOD_MS); + } + } tracker.start(reportPeriodInMillis, maxSamplesStored); } } From 734821e794868639679932ef42fef78986e9f906 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 16:21:01 -0700 Subject: [PATCH 11/32] add test to confirm span config max samples updated by server prop --- .../agent/config/AgentConfigImplTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java index deed07f4ab..47002fedc2 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java @@ -17,6 +17,7 @@ import java.util.*; +import static com.newrelic.agent.config.SpanEventsConfig.SERVER_SPAN_HARVEST_CONFIG; import static org.junit.Assert.*; /* (non-javadoc) @@ -1208,6 +1209,20 @@ public void shouldLogPropertiesFromSystemProperties() { )); } + @Test + public void shouldSetSpanMaxSamplesWithServerProp() { + //given + Map spanHarvestConfig = new HashMap<>(); + Long harvestLimit = 1L; + spanHarvestConfig.put("harvest_limit", harvestLimit); + Map localMap = new HashMap<>(); + localMap.put(SERVER_SPAN_HARVEST_CONFIG, spanHarvestConfig); + //when + AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); + //then + assertEquals("Span max samples should be the harvest limit of: "+harvestLimit.intValue(), harvestLimit.intValue(), config.getSpanEventsConfig().getMaxSamplesStored()); + } + private static EnvironmentFacade createEnvironmentFacade( Map environment, Map systemProps) { EnvironmentFacade environmentFacade = new MapEnvironmentFacade(environment); From 482e86e0ef98e1559a840c57c5652d06d70e0467 Mon Sep 17 00:00:00 2001 From: xxia Date: Thu, 16 Sep 2021 16:28:02 -0700 Subject: [PATCH 12/32] remove not thrown exception in signatures and simplify --- .../agent/config/AgentConfigImplTest.java | 164 +++++++++--------- 1 file changed, 82 insertions(+), 82 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java index 47002fedc2..d8db9a6f24 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigImplTest.java @@ -35,7 +35,7 @@ public void after() { } @Test - public void collectorDefaultHost() throws Exception { + public void collectorDefaultHost() { Map localMap = new HashMap<>(); // regular pre-protocol 15 key @@ -55,7 +55,7 @@ public void collectorDefaultHost() throws Exception { } @Test - public void collectorRegionAwareHost() throws Exception { + public void collectorRegionAwareHost() { Map localMap = new HashMap<>(); // proper 4 character protocol 15 key @@ -92,7 +92,7 @@ public void collectorSetHost() { } @Test - public void defaultMetricIngestUri() throws Exception { + public void defaultMetricIngestUri() { Map localMap = new HashMap<>(); // regular pre-protocol 15 key @@ -112,7 +112,7 @@ public void defaultMetricIngestUri() throws Exception { } @Test - public void regionAwareMetricIngestUri() throws Exception { + public void regionAwareMetricIngestUri() { Map localMap = new HashMap<>(); // proper 4 character protocol 15 key @@ -150,7 +150,7 @@ public void setMetricIngestUri() { } @Test - public void defaultEventIngestUri() throws Exception { + public void defaultEventIngestUri() { Map localMap = new HashMap<>(); // regular pre-protocol 15 key @@ -170,7 +170,7 @@ public void defaultEventIngestUri() throws Exception { } @Test - public void regionAwareEventIngestUri() throws Exception { + public void regionAwareEventIngestUri() { Map localMap = new HashMap<>(); // proper 4 character protocol 15 key @@ -209,16 +209,16 @@ public void setEventIngestUri() { @Test public void extensionReloadAsDottedInYaml() { - AgentConfig hasDefaultReloadModified = AgentConfigImpl.createAgentConfig(Collections.emptyMap()); + AgentConfig hasDefaultReloadModified = AgentConfigImpl.createAgentConfig(Collections.emptyMap()); boolean defaultReloadModified = hasDefaultReloadModified.getExtensionsConfig().shouldReloadModified(); - Map nestedMap = Collections.singletonMap("extensions", Collections.singletonMap("reload_modified", !defaultReloadModified)); + Map nestedMap = Collections.singletonMap("extensions", Collections.singletonMap("reload_modified", !defaultReloadModified)); AgentConfig nestedConfig = AgentConfigImpl.createAgentConfig(nestedMap); assertEquals(!defaultReloadModified, nestedConfig.getExtensionsConfig().shouldReloadModified()); } @Test - public void apiHost() throws Exception { + public void apiHost() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.API_HOST, "bogus"); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -227,7 +227,7 @@ public void apiHost() throws Exception { } @Test - public void apiHostDefault() throws Exception { + public void apiHostDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -235,7 +235,7 @@ public void apiHostDefault() throws Exception { } @Test - public void apiPort() throws Exception { + public void apiPort() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.API_PORT, AgentConfigImpl.DEFAULT_PORT + 1); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -244,7 +244,7 @@ public void apiPort() throws Exception { } @Test - public void apiPortDefault() throws Exception { + public void apiPortDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -252,7 +252,7 @@ public void apiPortDefault() throws Exception { } @Test - public void apiPortDefaultSSL() throws Exception { + public void apiPortDefaultSSL() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -260,7 +260,7 @@ public void apiPortDefaultSSL() throws Exception { } @Test - public void sendEnvironmentInfo() throws Exception { + public void sendEnvironmentInfo() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.SEND_ENVIRONMENT_INFO, !AgentConfigImpl.DEFAULT_SEND_ENVIRONMENT_INFO); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -269,7 +269,7 @@ public void sendEnvironmentInfo() throws Exception { } @Test - public void sendEnvironmentInfoDefault() throws Exception { + public void sendEnvironmentInfoDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -277,7 +277,7 @@ public void sendEnvironmentInfoDefault() throws Exception { } @Test - public void apdexTInMillis() throws Exception { + public void apdexTInMillis() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -285,7 +285,7 @@ public void apdexTInMillis() throws Exception { } @Test - public void apdexTInMillisForTransaction() throws Exception { + public void apdexTInMillisForTransaction() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -312,7 +312,7 @@ public void apdexTInMillisForTransaction() throws Exception { } @Test - public void appNames() throws Exception { + public void appNames() { Map localMap = new HashMap<>(); List appNames = new ArrayList<>(1); appNames.add(" app1"); @@ -320,12 +320,12 @@ public void appNames() throws Exception { AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); assertEquals(1, config.getApplicationNames().size()); - assertTrue(config.getApplicationNames().containsAll(Arrays.asList("app1"))); + assertTrue(config.getApplicationNames().contains("app1")); assertEquals("app1", config.getApplicationName()); } @Test - public void appNamesMultiple() throws Exception { + public void appNamesMultiple() { Map localMap = new HashMap<>(); List appNames = new ArrayList<>(1); appNames.add(" app1"); @@ -339,7 +339,7 @@ public void appNamesMultiple() throws Exception { } @Test - public void appNamesDuplicates() throws Exception { + public void appNamesDuplicates() { Map localMap = new HashMap<>(); List appNames = Arrays.asList(" app1", "app2", "app1"); localMap.put(AgentConfigImpl.APP_NAME, appNames); @@ -351,7 +351,7 @@ public void appNamesDuplicates() throws Exception { } @Test - public void appNamesEmpty() throws Exception { + public void appNamesEmpty() { Map localMap = new HashMap<>(); List appNames = new ArrayList<>(1); localMap.put(AgentConfigImpl.APP_NAME, appNames); @@ -362,26 +362,26 @@ public void appNamesEmpty() throws Exception { } @Test - public void appNamesMissing() throws Exception { - AgentConfig config = AgentConfigImpl.createAgentConfig(Collections.emptyMap()); + public void appNamesMissing() { + AgentConfig config = AgentConfigImpl.createAgentConfig(Collections.emptyMap()); assertEquals(0, config.getApplicationNames().size()); assertNull(config.getApplicationName()); } @Test - public void appNamesString() throws Exception { + public void appNamesString() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.APP_NAME, " app1 "); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); assertEquals(1, config.getApplicationNames().size()); - assertTrue(config.getApplicationNames().containsAll(Arrays.asList("app1"))); + assertTrue(config.getApplicationNames().contains("app1")); assertEquals("app1", config.getApplicationName()); } @Test - public void appNamesStringMultiple() throws Exception { + public void appNamesStringMultiple() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.APP_NAME, " app1; app2;;app3"); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -392,7 +392,7 @@ public void appNamesStringMultiple() throws Exception { } @Test - public void appNamesStringDuplicates() throws Exception { + public void appNamesStringDuplicates() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.APP_NAME, "app1;app2;app2"); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -457,7 +457,7 @@ public void eventsIngestUri() { } @Test - public void waitForRPMConnect() throws Exception { + public void waitForRPMConnect() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.WAIT_FOR_RPM_CONNECT, !AgentConfigImpl.DEFAULT_WAIT_FOR_RPM_CONNECT); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -466,7 +466,7 @@ public void waitForRPMConnect() throws Exception { } @Test - public void waitForRPMConnectSystemProperty() throws Exception { + public void waitForRPMConnectSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.WAIT_FOR_RPM_CONNECT; String val = String.valueOf(!AgentConfigImpl.DEFAULT_WAIT_FOR_RPM_CONNECT); @@ -480,7 +480,7 @@ public void waitForRPMConnectSystemProperty() throws Exception { } @Test - public void waitForRPMConnectServerSystemProperty() throws Exception { + public void waitForRPMConnectServerSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.WAIT_FOR_RPM_CONNECT; String val = String.valueOf(AgentConfigImpl.DEFAULT_WAIT_FOR_RPM_CONNECT); @@ -495,7 +495,7 @@ public void waitForRPMConnectServerSystemProperty() throws Exception { } @Test - public void waitForRPMConnectDefault() throws Exception { + public void waitForRPMConnectDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -503,7 +503,7 @@ public void waitForRPMConnectDefault() throws Exception { } @Test - public void getTransactionSizeLimit() throws Exception { + public void getTransactionSizeLimit() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.TRANSACTION_SIZE_LIMIT, AgentConfigImpl.DEFAULT_TRANSACTION_SIZE_LIMIT + 1); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -513,7 +513,7 @@ public void getTransactionSizeLimit() throws Exception { } @Test - public void getTransactionSizeLimitSystemProperty() throws Exception { + public void getTransactionSizeLimitSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.TRANSACTION_SIZE_LIMIT; String val = String.valueOf(AgentConfigImpl.DEFAULT_TRANSACTION_SIZE_LIMIT + 1); @@ -528,7 +528,7 @@ public void getTransactionSizeLimitSystemProperty() throws Exception { } @Test - public void getTransactionSizeLimitServerSystemProperty() throws Exception { + public void getTransactionSizeLimitServerSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.TRANSACTION_SIZE_LIMIT; String val = String.valueOf(AgentConfigImpl.DEFAULT_TRANSACTION_SIZE_LIMIT); @@ -544,7 +544,7 @@ public void getTransactionSizeLimitServerSystemProperty() throws Exception { } @Test - public void getTransactionSizeLimitDefault() throws Exception { + public void getTransactionSizeLimitDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -553,7 +553,7 @@ public void getTransactionSizeLimitDefault() throws Exception { } @Test - public void isEnableAutoAppNaming() throws Exception { + public void isEnableAutoAppNaming() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.ENABLE_AUTO_APP_NAMING, !AgentConfigImpl.DEFAULT_ENABLE_AUTO_APP_NAMING); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -562,7 +562,7 @@ public void isEnableAutoAppNaming() throws Exception { } @Test - public void isEnableAutoAppNamingSystemProperty() throws Exception { + public void isEnableAutoAppNamingSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.ENABLE_AUTO_APP_NAMING; String val = String.valueOf(!AgentConfigImpl.DEFAULT_ENABLE_AUTO_APP_NAMING); @@ -576,7 +576,7 @@ public void isEnableAutoAppNamingSystemProperty() throws Exception { } @Test - public void isEnableAutoAppNamingServerSystemProperty() throws Exception { + public void isEnableAutoAppNamingServerSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.ENABLE_AUTO_APP_NAMING; String val = String.valueOf(AgentConfigImpl.DEFAULT_ENABLE_AUTO_APP_NAMING); @@ -591,7 +591,7 @@ public void isEnableAutoAppNamingServerSystemProperty() throws Exception { } @Test - public void isEnableAutoAppNamingDefault() throws Exception { + public void isEnableAutoAppNamingDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -599,7 +599,7 @@ public void isEnableAutoAppNamingDefault() throws Exception { } @Test - public void isEnableAutoTransactionNaming() throws Exception { + public void isEnableAutoTransactionNaming() { Map localMap = new HashMap<>(); localMap.put(AgentConfigImpl.ENABLE_AUTO_TRANSACTION_NAMING, !AgentConfigImpl.DEFAULT_ENABLE_AUTO_TRANSACTION_NAMING); @@ -610,7 +610,7 @@ public void isEnableAutoTransactionNaming() throws Exception { } @Test - public void isEnableAutoTransactionNamingSystemProperty() throws Exception { + public void isEnableAutoTransactionNamingSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.ENABLE_AUTO_TRANSACTION_NAMING; String val = String.valueOf(!AgentConfigImpl.DEFAULT_ENABLE_AUTO_TRANSACTION_NAMING); @@ -626,7 +626,7 @@ public void isEnableAutoTransactionNamingSystemProperty() throws Exception { } @Test - public void isEnableAutoTransactionNamingServerSystemProperty() throws Exception { + public void isEnableAutoTransactionNamingServerSystemProperty() { Map properties = new HashMap<>(); String key = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.ENABLE_AUTO_TRANSACTION_NAMING; String val = String.valueOf(AgentConfigImpl.DEFAULT_ENABLE_AUTO_TRANSACTION_NAMING); @@ -642,7 +642,7 @@ public void isEnableAutoTransactionNamingServerSystemProperty() throws Exception } @Test - public void isEnableAutoTransactionNamingDefault() throws Exception { + public void isEnableAutoTransactionNamingDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -651,7 +651,7 @@ public void isEnableAutoTransactionNamingDefault() throws Exception { } @Test - public void getThreadProfilerConfig() throws Exception { + public void getThreadProfilerConfig() { Map localMap = new HashMap<>(); Map profilerMap = new HashMap<>(); profilerMap.put(ThreadProfilerConfigImpl.ENABLED, !ThreadProfilerConfigImpl.DEFAULT_ENABLED); @@ -662,7 +662,7 @@ public void getThreadProfilerConfig() throws Exception { } @Test - public void getThreadProfilerConfigSystemProperty() throws Exception { + public void getThreadProfilerConfigSystemProperty() { Map properties = new HashMap<>(); String key = ThreadProfilerConfigImpl.SYSTEM_PROPERTY_ROOT + ThreadProfilerConfigImpl.ENABLED; String val = String.valueOf(!ThreadProfilerConfigImpl.DEFAULT_ENABLED); @@ -678,7 +678,7 @@ public void getThreadProfilerConfigSystemProperty() throws Exception { } @Test - public void getThreadProfilerConfigServerSystemProperty() throws Exception { + public void getThreadProfilerConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = ThreadProfilerConfigImpl.SYSTEM_PROPERTY_ROOT + ThreadProfilerConfigImpl.ENABLED; String val = String.valueOf(ThreadProfilerConfigImpl.DEFAULT_ENABLED); @@ -695,7 +695,7 @@ public void getThreadProfilerConfigServerSystemProperty() throws Exception { } @Test - public void getThreadProfilerConfigDefault() throws Exception { + public void getThreadProfilerConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -703,7 +703,7 @@ public void getThreadProfilerConfigDefault() throws Exception { } @Test - public void getSqlTraceConfig() throws Exception { + public void getSqlTraceConfig() { Map localMap = new HashMap<>(); Map sqlMap = new HashMap<>(); sqlMap.put(SqlTraceConfigImpl.ENABLED, !SqlTraceConfigImpl.DEFAULT_ENABLED); @@ -714,7 +714,7 @@ public void getSqlTraceConfig() throws Exception { } @Test - public void getSqlTraceConfigSystemProperty() throws Exception { + public void getSqlTraceConfigSystemProperty() { Map properties = new HashMap<>(); String key = SqlTraceConfigImpl.SYSTEM_PROPERTY_ROOT + SqlTraceConfigImpl.ENABLED; String val = String.valueOf(!SqlTraceConfigImpl.DEFAULT_ENABLED); @@ -730,7 +730,7 @@ public void getSqlTraceConfigSystemProperty() throws Exception { } @Test - public void getSqlTraceConfigServerSystemProperty() throws Exception { + public void getSqlTraceConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = SqlTraceConfigImpl.SYSTEM_PROPERTY_ROOT + SqlTraceConfigImpl.ENABLED; String val = String.valueOf(!SqlTraceConfigImpl.DEFAULT_ENABLED); @@ -747,7 +747,7 @@ public void getSqlTraceConfigServerSystemProperty() throws Exception { } @Test - public void getSqlTraceConfigDefault() throws Exception { + public void getSqlTraceConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -755,7 +755,7 @@ public void getSqlTraceConfigDefault() throws Exception { } @Test - public void getSqlTraceConfigDefaultSqlIdSize() throws Exception { + public void getSqlTraceConfigDefaultSqlIdSize() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -763,7 +763,7 @@ public void getSqlTraceConfigDefaultSqlIdSize() throws Exception { } @Test - public void getTransactionTracerConfig() throws Exception { + public void getTransactionTracerConfig() { Map localMap = new HashMap<>(); Map ttMap = new HashMap<>(); ttMap.put(TransactionTracerConfigImpl.ENABLED, !TransactionTracerConfigImpl.DEFAULT_ENABLED); @@ -775,7 +775,7 @@ public void getTransactionTracerConfig() throws Exception { } @Test - public void getCrossProcessConfig() throws Exception { + public void getCrossProcessConfig() { Map localMap = new HashMap<>(); Map catMap = new HashMap<>(); catMap.put(CrossProcessConfigImpl.ENABLED, !CrossProcessConfigImpl.DEFAULT_ENABLED); @@ -787,7 +787,7 @@ public void getCrossProcessConfig() throws Exception { } @Test - public void getCrossProcessConfigDeprecated() throws Exception { + public void getCrossProcessConfigDeprecated() { Map localMap = new HashMap<>(); localMap.put(CrossProcessConfigImpl.CROSS_APPLICATION_TRACING, !CrossProcessConfigImpl.DEFAULT_ENABLED); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -797,7 +797,7 @@ public void getCrossProcessConfigDeprecated() throws Exception { } @Test - public void getCrossProcessConfigBothDeprecatedAndNot() throws Exception { + public void getCrossProcessConfigBothDeprecatedAndNot() { Map localMap = new HashMap<>(); localMap.put(CrossProcessConfigImpl.CROSS_APPLICATION_TRACING, CrossProcessConfigImpl.DEFAULT_ENABLED); Map catMap = new HashMap<>(); @@ -810,7 +810,7 @@ public void getCrossProcessConfigBothDeprecatedAndNot() throws Exception { } @Test - public void getTransactionTracerConfigSystemProperty() throws Exception { + public void getTransactionTracerConfigSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; String val = String.valueOf(!ThreadProfilerConfigImpl.DEFAULT_ENABLED); @@ -827,7 +827,7 @@ public void getTransactionTracerConfigSystemProperty() throws Exception { } @Test - public void getTransactionTracerConfigServerSystemProperty() throws Exception { + public void getTransactionTracerConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; String val = String.valueOf(ThreadProfilerConfigImpl.DEFAULT_ENABLED); @@ -845,15 +845,15 @@ public void getTransactionTracerConfigServerSystemProperty() throws Exception { } @Test - public void getTransactionTracerConfigDefault() throws Exception { + public void getTransactionTracerConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); - assertEquals(false, config.getTransactionTracerConfig().isEnabled()); + assertFalse(config.getTransactionTracerConfig().isEnabled()); } @Test - public void getRequestTransactionTracerConfig() throws Exception { + public void getRequestTransactionTracerConfig() { Map localMap = new HashMap<>(); Map ttMap = new HashMap<>(); Set> categorySet = new HashSet<>(); @@ -871,7 +871,7 @@ public void getRequestTransactionTracerConfig() throws Exception { } @Test - public void getRequestTransactionTracerConfigSystemProperty() throws Exception { + public void getRequestTransactionTracerConfigSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.CATEGORY_REQUEST_SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; @@ -896,7 +896,7 @@ public void getRequestTransactionTracerConfigSystemProperty() throws Exception { } @Test - public void getRequestTransactionTracerConfigServerSystemProperty() throws Exception { + public void getRequestTransactionTracerConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.CATEGORY_REQUEST_SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; @@ -922,15 +922,15 @@ public void getRequestTransactionTracerConfigServerSystemProperty() throws Excep } @Test - public void getRequestTransactionTracerConfigDefault() throws Exception { + public void getRequestTransactionTracerConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); - assertEquals(false, config.getRequestTransactionTracerConfig().isEnabled()); + assertFalse(config.getRequestTransactionTracerConfig().isEnabled()); } @Test - public void getBackgroundTransactionTracerConfig() throws Exception { + public void getBackgroundTransactionTracerConfig() { Map localMap = new HashMap<>(); Map ttMap = new HashMap<>(); Set> categorySet = new HashSet<>(); @@ -948,7 +948,7 @@ public void getBackgroundTransactionTracerConfig() throws Exception { } @Test - public void getBackgroundTransactionTracerConfigSystemProperty() throws Exception { + public void getBackgroundTransactionTracerConfigSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.CATEGORY_BACKGROUND_SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; @@ -972,7 +972,7 @@ public void getBackgroundTransactionTracerConfigSystemProperty() throws Exceptio } @Test - public void getBackgroundTransactionTracerConfigServerSystemProperty() throws Exception { + public void getBackgroundTransactionTracerConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = TransactionTracerConfigImpl.CATEGORY_BACKGROUND_SYSTEM_PROPERTY_ROOT + TransactionTracerConfigImpl.ENABLED; @@ -997,15 +997,15 @@ public void getBackgroundTransactionTracerConfigServerSystemProperty() throws Ex } @Test - public void getBackgroundTransactionTracerConfigDefault() throws Exception { + public void getBackgroundTransactionTracerConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); - assertEquals(false, config.getBackgroundTransactionTracerConfig().isEnabled()); + assertFalse(config.getBackgroundTransactionTracerConfig().isEnabled()); } @Test - public void getErrorCollectorConfig() throws Exception { + public void getErrorCollectorConfig() { Map localMap = new HashMap<>(); Map errorMap = new HashMap<>(); errorMap.put(ErrorCollectorConfigImpl.ENABLED, !ErrorCollectorConfigImpl.DEFAULT_ENABLED); @@ -1017,7 +1017,7 @@ public void getErrorCollectorConfig() throws Exception { } @Test - public void getErrorCollectorConfigSystemProperty() throws Exception { + public void getErrorCollectorConfigSystemProperty() { Map properties = new HashMap<>(); String key = ErrorCollectorConfigImpl.SYSTEM_PROPERTY_ROOT + ErrorCollectorConfigImpl.ENABLED; String val = String.valueOf(!ErrorCollectorConfigImpl.DEFAULT_ENABLED); @@ -1026,11 +1026,11 @@ public void getErrorCollectorConfigSystemProperty() throws Exception { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); - assertEquals(false, config.getErrorCollectorConfig().isEnabled()); + assertFalse(config.getErrorCollectorConfig().isEnabled()); } @Test - public void getErrorCollectorConfigServerSystemProperty() throws Exception { + public void getErrorCollectorConfigServerSystemProperty() { Map properties = new HashMap<>(); String key = ErrorCollectorConfigImpl.SYSTEM_PROPERTY_ROOT + ErrorCollectorConfigImpl.ENABLED; String val = String.valueOf(ErrorCollectorConfigImpl.DEFAULT_ENABLED); @@ -1048,7 +1048,7 @@ public void getErrorCollectorConfigServerSystemProperty() throws Exception { } @Test - public void getErrorCollectorConfigDefault() throws Exception { + public void getErrorCollectorConfigDefault() { Map localMap = new HashMap<>(); AgentConfig config = AgentConfigImpl.createAgentConfig(localMap); @@ -1056,7 +1056,7 @@ public void getErrorCollectorConfigDefault() throws Exception { } @Test - public void getBrowserMonitoringConfig() throws Exception { + public void getBrowserMonitoringConfig() { Map localMap = new HashMap<>(); Map beaconMap = new HashMap<>(); beaconMap.put(BrowserMonitoringConfigImpl.AUTO_INSTRUMENT, !BrowserMonitoringConfigImpl.DEFAULT_AUTO_INSTRUMENT); @@ -1068,7 +1068,7 @@ public void getBrowserMonitoringConfig() throws Exception { } @Test - public void checkSystemPropertyValueOverrideWithTrue() throws Exception { + public void checkSystemPropertyValueOverrideWithTrue() { Map props = new HashMap<>(); String key = "com.newrelic.instrumentation.jcache-datastore-1.0.0"; props.put(key, true); @@ -1083,7 +1083,7 @@ public void checkSystemPropertyValueOverrideWithTrue() throws Exception { } @Test - public void checkSystemPropertyValueOverrideWithFalse() throws Exception { + public void checkSystemPropertyValueOverrideWithFalse() { Map props = new HashMap<>(); String key = "com.newrelic.instrumentation.jcache-datastore-1.0.0"; props.put(key, false); @@ -1170,7 +1170,7 @@ public void shouldLogPropertiesFromEnvironment() { Map env = new HashMap<>(); String key = "newrelic.config.my-prop"; env.put(key, "some value"); - SystemPropertyProvider provider = Mocks.createSystemPropertyProvider(Collections.emptyMap(), env); + SystemPropertyProvider provider = Mocks.createSystemPropertyProvider(Collections.emptyMap(), env); SystemPropertyFactory.setSystemPropertyProvider(provider); TestConfig target = new TestConfig(); @@ -1192,7 +1192,7 @@ public void shouldLogPropertiesFromSystemProperties() { Map env = new HashMap<>(); String key = "newrelic.config.my-prop"; env.put(key, "some value"); - SystemPropertyProvider provider = Mocks.createSystemPropertyProvider(env, Collections.emptyMap()); + SystemPropertyProvider provider = Mocks.createSystemPropertyProvider(env, Collections.emptyMap()); SystemPropertyFactory.setSystemPropertyProvider(provider); TestConfig target = new TestConfig(); @@ -1235,7 +1235,7 @@ private static EnvironmentFacade createEnvironmentFacade( private static class TestConfig extends BaseConfig { TestConfig() { - super(Collections.emptyMap()); + super(Collections.emptyMap()); } void addDeprecatedProp(String property, String newProperty) { From 27f37a41b815214f3bd3cac13bd99a7c3941834d Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Sep 2021 17:50:58 -0700 Subject: [PATCH 13/32] Clean up ExternalAsyncTest --- .../com/newrelic/agent/async/AsyncTest.java | 15 +- .../agent/async/ExternalAsyncTest.java | 358 ++++++++---------- 2 files changed, 164 insertions(+), 209 deletions(-) diff --git a/functional_test/src/test/java/com/newrelic/agent/async/AsyncTest.java b/functional_test/src/test/java/com/newrelic/agent/async/AsyncTest.java index 1dc2f55eba..5fa74de46a 100644 --- a/functional_test/src/test/java/com/newrelic/agent/async/AsyncTest.java +++ b/functional_test/src/test/java/com/newrelic/agent/async/AsyncTest.java @@ -103,7 +103,7 @@ public void verifyCpu(long minCpu) { Assert.assertTrue("The cpu should be greater than 0", val > 0); Assert.assertTrue("The cpu should be greater than the expeted min value " + minCpu, val > minCpu); - long cpuTime = 0l; + long cpuTime = 0L; Collection tracers = new HashSet<>(); tracers.add(data.getRootTracer()); tracers.addAll(data.getTracers()); @@ -241,8 +241,7 @@ private void verifyGivenTransactionSegmentBreadthFirst(TransactionSegment segmen Assert.assertEquals("ROOT", segment.getMetricName()); Assert.assertNotNull(segment.getTraceParameters().get("async_context")); - Queue segments = new LinkedList<>(); - segments.addAll(segment.getChildren()); + Queue segments = new LinkedList<>(segment.getChildren()); int index = 0; while (!segments.isEmpty()) { TransactionSegment seg = segments.poll(); @@ -266,8 +265,7 @@ public void verifyTransactionSegmentNodesWithExecContext(Map met TransactionTrace trace = TransactionTrace.getTransactionTrace(data); TransactionSegment root = trace.getRootSegment(); - Queue segments = new LinkedList<>(); - segments.addAll(root.getChildren()); + Queue segments = new LinkedList<>(root.getChildren()); int index = 0; while (!segments.isEmpty()) { TransactionSegment seg = segments.poll(); @@ -289,8 +287,7 @@ public void verifyTransactionSegmentsChildren(String parent, String... children) TransactionTrace trace = TransactionTrace.getTransactionTrace(data); TransactionSegment root = trace.getRootSegment(); - Queue segments = new LinkedList<>(); - segments.addAll(root.getChildren()); + Queue segments = new LinkedList<>(root.getChildren()); int index = 0; while (!segments.isEmpty()) { TransactionSegment seg = segments.poll(); @@ -374,7 +371,7 @@ private void waitForTransaction() { while ((System.currentTimeMillis() - start) < 5000 && (data == null || stats == null)) { try { Thread.sleep(50); - } catch (InterruptedException e) { + } catch (InterruptedException ignored) { } } } @@ -385,7 +382,7 @@ private void waitForTransactionTimesSet(int timesSet) { while ((System.currentTimeMillis() - start) < 5000 && (data == null || stats == null || this.timesSet != timesSet)) { try { Thread.sleep(50); - } catch (InterruptedException e) { + } catch (InterruptedException ignored) { } } } diff --git a/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java b/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java index 7a35134d81..3b4af34da3 100644 --- a/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java +++ b/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java @@ -10,9 +10,7 @@ import com.newrelic.agent.HeadersUtil; import com.newrelic.agent.Transaction; import com.newrelic.agent.TransactionActivity; -import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.NoOpSegment; -import com.newrelic.agent.bridge.TracedActivity; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.tracers.Tracer; import com.newrelic.agent.tracers.TransactionActivityInitiator; @@ -23,13 +21,14 @@ import com.newrelic.api.agent.InboundHeaders; import com.newrelic.api.agent.NewRelic; import com.newrelic.api.agent.OutboundHeaders; +import com.newrelic.api.agent.Segment; +import com.newrelic.api.agent.Token; import com.newrelic.api.agent.Trace; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import java.io.UnsupportedEncodingException; import java.net.URISyntaxException; import java.util.Collection; import java.util.HashMap; @@ -42,13 +41,37 @@ public class ExternalAsyncTest extends AsyncTest { private static final String ASYNC_TXA_NAME = "activity"; - private static final String HOST = "www.domain.com"; + private static final String HOST = "www.example.com"; private static final String LIBRARY = "library"; - private static final String URI = "http://www.domain.com/some/path"; + private static final String URI = "http://www.example.com/some/path"; private static final String OPERATION_NAME = "operation"; private ExecutorService executorService; + private static Segment startSegment( + OutboundHeaders outbound) { + Segment externalEvent = NewRelic.getAgent().getTransaction().startSegment(ASYNC_TXA_NAME); + Assert.assertNotNull(externalEvent); + externalEvent.addOutboundRequestHeaders(outbound); + return externalEvent; + } + + private static void finishExternalEvent(Segment externalEvent, + Throwable t, InboundHeaders inbound, String host, String library, + String uri, String operationName) { + try { + externalEvent.reportAsExternal(HttpParameters + .library(library) + .uri(new java.net.URI(uri)) + .procedure(operationName) + .inboundHeaders(inbound) + .build()); + } catch (URISyntaxException e) { + Assert.fail(e.getMessage()); + } + externalEvent.end(); + } + @Before public void before() { executorService = Executors.newCachedThreadPool(); @@ -62,128 +85,116 @@ public void after() { @Test public void testSuccess() throws Exception { - long time = doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent = startSegment(new Outbound()); - finishExternalEvent(externalEvent, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - return null; - } + long time = doInTransaction((Callable) () -> { + Segment externalEvent = startSegment(new Outbound()); + finishExternalEvent(externalEvent, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + return null; }); verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent("External/www.domain.com/all", + "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent("External/www.example.com/all", "External/allOther", "External/all"); verifyTransactionSegmentsBreadthFirst( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT); + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT); verifyNoExceptions(); } @Test public void testFailure() throws Exception { - long time = doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent = startSegment(new Outbound()); - Throwable error = new Exception(); - finishExternalEvent(externalEvent, error, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - return null; - } + long time = doInTransaction((Callable) () -> { + Segment externalEvent = startSegment(new Outbound()); + Throwable error = new Exception(); + finishExternalEvent(externalEvent, error, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + return null; }); verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent("External/www.domain.com/all", + "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent("External/www.example.com/all", "External/allOther", "External/all"); verifyTransactionSegmentsBreadthFirst( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT); + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT); verifyNoExceptions(); // ?? } @Test public void testIgnoredTransaction() throws Exception { - doInTransaction(new Callable() { - public Void call() { - NewRelic.getAgent().getTransaction().ignore(); - TracedActivity externalEvent = AgentBridge.getAgent() - .getTransaction().createAndStartTracedActivity(); - Assert.assertTrue(externalEvent.equals(NoOpSegment.INSTANCE)); - return null; - } + doInTransaction((Callable) () -> { + NewRelic.getAgent().getTransaction().ignore(); + Segment externalEvent = NewRelic.getAgent().getTransaction().startSegment("foo"); + Assert.assertEquals(externalEvent, NoOpSegment.INSTANCE); + return null; }); verifyTimesSet(0); } @Test public void testNoTransaction() { - TracedActivity externalEvent = AgentBridge.getAgent().getTransaction() - .createAndStartTracedActivity(); + Segment externalEvent = NewRelic.getAgent().getTransaction().startSegment("foo"); Assert.assertEquals(externalEvent, NoOpSegment.INSTANCE); verifyTimesSet(0); } @Test public void testTwoIntermixed() throws Exception { - long time = doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent1 = startSegment(new Outbound()); - TracedActivity externalEvent2 = startSegment(new Outbound()); - finishExternalEvent(externalEvent1, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - try { - Thread.sleep(1); - } catch (InterruptedException e) { - } - finishExternalEvent(externalEvent2, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - return null; + long time = doInTransaction((Callable) () -> { + Segment externalEvent1 = startSegment(new Outbound()); + Segment externalEvent2 = startSegment(new Outbound()); + finishExternalEvent(externalEvent1, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + try { + Thread.sleep(1); + } catch (InterruptedException ignored) { } + finishExternalEvent(externalEvent2, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + return null; }); verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - 2, "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent(stats, 2, "External/www.domain.com/all", + 2, "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent(stats, 2, "External/www.example.com/all", "External/allOther", "External/all"); verifyTransactionSegmentsBreadthFirst( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT, - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT); + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT, + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT); verifyNoExceptions(); } @Test public void testFinishTwice() throws Exception { - doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent1 = startSegment(new Outbound()); - finishExternalEvent(externalEvent1, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - finishExternalEvent(externalEvent1, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - return null; - } + doInTransaction((Callable) () -> { + Segment externalEvent1 = startSegment(new Outbound()); + finishExternalEvent(externalEvent1, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + finishExternalEvent(externalEvent1, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + return null; }); verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent("External/www.domain.com/all", + "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent("External/www.example.com/all", "External/allOther", "External/all"); verifyScopedMetricsNotPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", @@ -192,7 +203,7 @@ public Void call() { "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT); + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT); verifyNoExceptions(); } @@ -200,23 +211,21 @@ public Void call() { // one meaning this is allowed @Test public void testStartAfterFinish() throws Exception { - doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent1 = startSegment(new Outbound()); - finishExternalEvent(externalEvent1, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - TracedActivity externalEvent2 = startSegment(new Outbound()); - finishExternalEvent(externalEvent2, null, new Inbound(), HOST, - LIBRARY, URI, OPERATION_NAME); - return null; - } + doInTransaction((Callable) () -> { + Segment externalEvent1 = startSegment(new Outbound()); + finishExternalEvent(externalEvent1, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + Segment externalEvent2 = startSegment(new Outbound()); + finishExternalEvent(externalEvent2, null, new Inbound(), HOST, + LIBRARY, URI, OPERATION_NAME); + return null; }); verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - 2, "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent(2, "External/www.domain.com/all", + 2, "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent(2, "External/www.example.com/all", "External/allOther", "External/all"); verifyScopedMetricsNotPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", @@ -225,8 +234,8 @@ public Void call() { "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT, - "External/www.domain.com/library/operation", NO_ASYNC_CONTEXT); + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT, + "External/www.example.com/library/operation", NO_ASYNC_CONTEXT); verifyNoExceptions(); } @@ -234,18 +243,16 @@ public Void call() { public void testCat() throws Exception { final Outbound outbound = new Outbound(); final Inbound inbound = new Inbound(); - doInTransaction(new Callable() { - public Void call() { - TracedActivity externalEvent = startSegment(outbound); - Transaction transaction = Transaction.getTransaction(false); - String encodingKey = transaction.getCrossProcessConfig().getEncodingKey(); - String appData = Obfuscator.obfuscateNameUsingKey( - "[\"crossProcessId\",\"externalTransactionName\"]", - encodingKey); - inbound.headers.put(HeadersUtil.NEWRELIC_APP_DATA_HEADER, appData); - finishExternalEvent(externalEvent, null, inbound, HOST, LIBRARY, URI, OPERATION_NAME); - return null; - } + doInTransaction((Callable) () -> { + Segment externalEvent = startSegment(outbound); + Transaction transaction = Transaction.getTransaction(false); + String encodingKey = transaction.getCrossProcessConfig().getEncodingKey(); + String appData = Obfuscator.obfuscateNameUsingKey( + "[\"crossProcessId\",\"externalTransactionName\"]", + encodingKey); + inbound.headers.put(HeadersUtil.NEWRELIC_APP_DATA_HEADER, appData); + finishExternalEvent(externalEvent, null, inbound, HOST, LIBRARY, URI, OPERATION_NAME); + return null; }); // assert outbound request headers were populated correctly @@ -259,8 +266,8 @@ public Void call() { verifyTimesSet(1); verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "ExternalTransaction/www.domain.com/crossProcessId/externalTransactionName"); - verifyUnscopedMetricsPresent("External/www.domain.com/all", + "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName"); + verifyUnscopedMetricsPresent("External/www.example.com/all", "External/allOther", "External/all"); verifyScopedMetricsNotPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", @@ -269,7 +276,7 @@ public Void call() { "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", Thread.currentThread().getName(), - "ExternalTransaction/www.domain.com/crossProcessId/externalTransactionName", + "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName", NO_ASYNC_CONTEXT); verifyNoExceptions(); } @@ -281,9 +288,8 @@ public void testWithRegisterStart() throws Exception { @Override public Void call() throws Exception { // register a new async activity - final Object asyncKey = new Object(); - Assert.assertTrue(AgentBridge.getAgent().getTransaction() - .registerAsyncActivity(asyncKey)); + Token token = NewRelic.getAgent().getTransaction().getToken(); + Assert.assertTrue(token.isActive()); // submit async activity executorService.submit(new Callable() { @@ -294,22 +300,18 @@ public Void call() throws Exception { .getName(); // start async activity - Assert.assertTrue(AgentBridge.getAgent() - .startAsyncActivity(asyncKey)); + Assert.assertTrue(token.linkAndExpire()); // start external work - final TracedActivity externalEvent = startSegment(new Outbound()); + final Segment externalEvent = startSegment(new Outbound()); // submit external work - executorService.submit(new Callable() { - @Override - public Void call() throws Exception { - // finish external work - finishExternalEvent(externalEvent, null, - new Inbound(), HOST, LIBRARY, URI, - OPERATION_NAME); - return null; - } + executorService.submit((Callable) () -> { + // finish external work + finishExternalEvent(externalEvent, null, + new Inbound(), HOST, LIBRARY, URI, + OPERATION_NAME); + return null; }).get(); return null; } @@ -322,8 +324,8 @@ public Void call() throws Exception { verifyScopedMetricsPresent( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", "Java/" + asyncClassAndThreadName[0] + "/call", - "External/www.domain.com/library/operation"); - verifyUnscopedMetricsPresent("External/www.domain.com/all", + "External/www.example.com/library/operation"); + verifyUnscopedMetricsPresent("External/www.example.com/all", "External/allOther", "External/all"); verifyTransactionSegmentsBreadthFirst( "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", @@ -331,44 +333,35 @@ public Void call() throws Exception { Thread.currentThread().getName(), "Java/" + asyncClassAndThreadName[0] + "/call", asyncClassAndThreadName[1], - "External/www.domain.com/library/operation", "segment-api"); + "External/www.example.com/library/operation", "segment-api"); verifyNoExceptions(); } @Test public void testConcurrentSuccess() throws Exception { - doInTransaction(new Callable() { - @Override - public Void call() throws Exception { - // pretend some external calls are started in a tx - final TracedActivity externalEvent1 = startSegment(new Outbound()); - final TracedActivity externalEvent2 = startSegment(new Outbound()); - - // pretend they're finished asynchronously - executorService.submit(new Callable() { - @Override - public Void call() throws Exception { - Thread.sleep(500); - finishExternalEvent(externalEvent1, null, - new Inbound(), "http://www.one.com", LIBRARY, - "http://www.one.com/path", "one"); - return null; - } - }); - - executorService.submit(new Callable() { - @Override - public Void call() throws Exception { - // finish external work - finishExternalEvent(externalEvent2, null, - new Inbound(), "http://www.two.com", LIBRARY, - "http://www.two.com/path", "two"); - return null; - } - }); + doInTransaction((Callable) () -> { + // pretend some external calls are started in a tx + final Segment externalEvent1 = startSegment(new Outbound()); + final Segment externalEvent2 = startSegment(new Outbound()); + + // pretend they're finished asynchronously + executorService.submit((Callable) () -> { + Thread.sleep(500); + finishExternalEvent(externalEvent1, null, + new Inbound(), "http://www.one.com", LIBRARY, + "http://www.one.com/path", "one"); + return null; + }); + executorService.submit((Callable) () -> { + // finish external work + finishExternalEvent(externalEvent2, null, + new Inbound(), "http://www.two.com", LIBRARY, + "http://www.two.com/path", "two"); return null; - } + }); + + return null; }); executorService.shutdown(); executorService.awaitTermination(1500, TimeUnit.MILLISECONDS); @@ -398,39 +391,30 @@ public Void call() throws Exception { @Test public void testConcurrentFailure() throws Exception { - doInTransaction(new Callable() { - @Override - public Void call() throws Exception { - // pretend some external calls are started in a tx - final TracedActivity externalEvent1 = startSegment(new Outbound()); - final TracedActivity externalEvent2 = startSegment(new Outbound()); - // pretend they're finished asynchronously - executorService.submit(new Callable() { - @Override - public Void call() throws Exception { - Thread.sleep(500); - finishExternalEvent(externalEvent1, null, - new Inbound(), "http://www.one.com", LIBRARY, - "http://www.one.com/path", "one"); - return null; - } - }); - - executorService.submit(new Callable() { - @Override - public Void call() throws Exception { - // finish external work - Throwable throwable = new RuntimeException( - "SomeException"); - finishExternalEvent(externalEvent2, throwable, - new Inbound(), "http://www.two.com", LIBRARY, - "http://www.two.com/path", "two"); - return null; - } - }); - + doInTransaction((Callable) () -> { + // pretend some external calls are started in a tx + final Segment externalEvent1 = startSegment(new Outbound()); + final Segment externalEvent2 = startSegment(new Outbound()); + // pretend they're finished asynchronously + executorService.submit((Callable) () -> { + Thread.sleep(500); + finishExternalEvent(externalEvent1, null, + new Inbound(), "http://www.one.com", LIBRARY, + "http://www.one.com/path", "one"); return null; - } + }); + + executorService.submit((Callable) () -> { + // finish external work + Throwable throwable = new RuntimeException( + "SomeException"); + finishExternalEvent(externalEvent2, throwable, + new Inbound(), "http://www.two.com", LIBRARY, + "http://www.two.com/path", "two"); + return null; + }); + + return null; }); executorService.shutdown(); executorService.awaitTermination(1500, TimeUnit.MILLISECONDS); @@ -458,32 +442,6 @@ public Void call() throws Exception { verifyNoExceptions(); } - private static TracedActivity startSegment( - OutboundHeaders outbound) { - TracedActivity externalEvent = AgentBridge.getAgent().getTransaction() - .createAndStartTracedActivity(); - Assert.assertNotNull(externalEvent); - externalEvent.setAsyncThreadName(ASYNC_TXA_NAME); - externalEvent.getTracedMethod().addOutboundRequestHeaders(outbound); - return externalEvent; - } - - private static void finishExternalEvent(TracedActivity externalEvent, - Throwable t, InboundHeaders inbound, String host, String library, - String uri, String operationName) { - try { - externalEvent.getTracedMethod().reportAsExternal(HttpParameters - .library(library) - .uri(new java.net.URI(uri)) - .procedure(operationName) - .inboundHeaders(inbound) - .build()); - } catch (URISyntaxException e) { - Assert.fail(e.getMessage()); - } - externalEvent.finish(t); - } - @Trace(dispatcher = true) private long doInTransaction(Callable work) throws Exception { long startTime = ServiceFactory.getTransactionTraceService() From 8d135524511804c1da4d8fd32d2b97685122282e Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Sep 2021 19:33:32 -0700 Subject: [PATCH 14/32] Fix ExternalAsyncTest by disabling DT and enabling CAT --- .../agent/async/ExternalAsyncTest.java | 93 +++++++++++-------- .../configs/cross_app_tracing_test.yml | 21 +++++ 2 files changed, 75 insertions(+), 39 deletions(-) create mode 100644 functional_test/src/test/resources/configs/cross_app_tracing_test.yml diff --git a/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java b/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java index 3b4af34da3..b2366c4714 100644 --- a/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java +++ b/functional_test/src/test/java/com/newrelic/agent/async/ExternalAsyncTest.java @@ -28,6 +28,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import test.newrelic.EnvironmentHolderSettingsGenerator; +import test.newrelic.test.agent.EnvironmentHolder; import java.net.URISyntaxException; import java.util.Collection; @@ -45,7 +47,8 @@ public class ExternalAsyncTest extends AsyncTest { private static final String LIBRARY = "library"; private static final String URI = "http://www.example.com/some/path"; private static final String OPERATION_NAME = "operation"; - + private static final String CONFIG_FILE = "configs/cross_app_tracing_test.yml"; + private static final ClassLoader CLASS_LOADER = ExternalAsyncTest.class.getClassLoader(); private ExecutorService executorService; private static Segment startSegment( @@ -72,6 +75,13 @@ private static void finishExternalEvent(Segment externalEvent, externalEvent.end(); } + public EnvironmentHolder setupEnvironmentHolder(String environment) throws Exception { + EnvironmentHolderSettingsGenerator envHolderSettings = new EnvironmentHolderSettingsGenerator(CONFIG_FILE, environment, CLASS_LOADER); + EnvironmentHolder environmentHolder = new EnvironmentHolder(envHolderSettings); + environmentHolder.setupEnvironment(); + return environmentHolder; + } + @Before public void before() { executorService = Executors.newCachedThreadPool(); @@ -207,8 +217,7 @@ public void testFinishTwice() throws Exception { verifyNoExceptions(); } - // with the refactoring of the transaction object, we are no longer one to - // one meaning this is allowed + // with the refactoring of the transaction object, we are no longer one to one meaning this is allowed @Test public void testStartAfterFinish() throws Exception { doInTransaction((Callable) () -> { @@ -241,44 +250,50 @@ public void testStartAfterFinish() throws Exception { @Test public void testCat() throws Exception { - final Outbound outbound = new Outbound(); - final Inbound inbound = new Inbound(); - doInTransaction((Callable) () -> { - Segment externalEvent = startSegment(outbound); - Transaction transaction = Transaction.getTransaction(false); - String encodingKey = transaction.getCrossProcessConfig().getEncodingKey(); - String appData = Obfuscator.obfuscateNameUsingKey( - "[\"crossProcessId\",\"externalTransactionName\"]", - encodingKey); - inbound.headers.put(HeadersUtil.NEWRELIC_APP_DATA_HEADER, appData); - finishExternalEvent(externalEvent, null, inbound, HOST, LIBRARY, URI, OPERATION_NAME); - return null; - }); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - // assert outbound request headers were populated correctly - Assert.assertTrue(outbound.headers - .containsKey(HeadersUtil.NEWRELIC_ID_HEADER)); - Assert.assertTrue(outbound.headers - .containsKey(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)); + try { + final Outbound outbound = new Outbound(); + final Inbound inbound = new Inbound(); + doInTransaction((Callable) () -> { + Segment externalEvent = startSegment(outbound); + Transaction transaction = Transaction.getTransaction(false); + String encodingKey = transaction.getCrossProcessConfig().getEncodingKey(); + String appData = Obfuscator.obfuscateNameUsingKey( + "[\"crossProcessId\",\"externalTransactionName\"]", + encodingKey); + inbound.headers.put(HeadersUtil.NEWRELIC_APP_DATA_HEADER, appData); + finishExternalEvent(externalEvent, null, inbound, HOST, LIBRARY, URI, OPERATION_NAME); + return null; + }); - // assert inbound response headers were processed correctly by checking - // for CAT metric name - verifyTimesSet(1); - verifyScopedMetricsPresent( - "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName"); - verifyUnscopedMetricsPresent("External/www.example.com/all", - "External/allOther", "External/all"); - verifyScopedMetricsNotPresent( - "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "External/www.host2.com/library"); - verifyTransactionSegmentsBreadthFirst( - "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", - Thread.currentThread().getName(), - "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName", - NO_ASYNC_CONTEXT); - verifyNoExceptions(); + // assert outbound request headers were populated correctly + Assert.assertTrue(outbound.headers + .containsKey(HeadersUtil.NEWRELIC_ID_HEADER)); + Assert.assertTrue(outbound.headers + .containsKey(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)); + + // assert inbound response headers were processed correctly by checking for CAT metric name + verifyTimesSet(1); + verifyScopedMetricsPresent( + "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", + "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName"); + verifyUnscopedMetricsPresent("External/www.example.com/all", + "External/allOther", "External/all"); + verifyScopedMetricsNotPresent( + "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", + "External/www.host2.com/library"); + verifyTransactionSegmentsBreadthFirst( + "OtherTransaction/Custom/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", + "Java/com.newrelic.agent.async.ExternalAsyncTest/doInTransaction", + Thread.currentThread().getName(), + "ExternalTransaction/www.example.com/crossProcessId/externalTransactionName", + NO_ASYNC_CONTEXT); + verifyNoExceptions(); + } finally { + holder.close(); + } } @Test diff --git a/functional_test/src/test/resources/configs/cross_app_tracing_test.yml b/functional_test/src/test/resources/configs/cross_app_tracing_test.yml new file mode 100644 index 0000000000..323853a07f --- /dev/null +++ b/functional_test/src/test/resources/configs/cross_app_tracing_test.yml @@ -0,0 +1,21 @@ +cat_enabled_dt_enabled_test: + + cross_application_tracer: + enabled: true + cross_process_id: '12345#xyz' + encoding_key: asdf + trusted_account_ids: '12345' + + distributed_tracing: + enabled: true + +cat_enabled_dt_disabled_test: + + cross_application_tracer: + enabled: true + cross_process_id: '12345#xyz' + encoding_key: asdf + trusted_account_ids: '12345' + + distributed_tracing: + enabled: false From af16ff2f4ec60575243c876c2955de7f7b495636 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Sep 2021 19:44:52 -0700 Subject: [PATCH 15/32] Fix APISupportabilityTest by disabling DT and enabling CAT --- .../APISupportabilityTest.java | 138 ++++++++++++------ 1 file changed, 96 insertions(+), 42 deletions(-) diff --git a/functional_test/src/test/java/com/newrelic/agent/instrumentation/APISupportabilityTest.java b/functional_test/src/test/java/com/newrelic/agent/instrumentation/APISupportabilityTest.java index 2cc9255645..d4426e07dd 100644 --- a/functional_test/src/test/java/com/newrelic/agent/instrumentation/APISupportabilityTest.java +++ b/functional_test/src/test/java/com/newrelic/agent/instrumentation/APISupportabilityTest.java @@ -9,6 +9,7 @@ import com.newrelic.agent.TransactionData; import com.newrelic.agent.TransactionStatsListener; +import com.newrelic.agent.async.ExternalAsyncTest; import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.TracedActivity; import com.newrelic.agent.service.ServiceFactory; @@ -25,6 +26,8 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import test.newrelic.EnvironmentHolderSettingsGenerator; +import test.newrelic.test.agent.EnvironmentHolder; import java.net.URI; import java.net.URISyntaxException; @@ -35,6 +38,8 @@ public class APISupportabilityTest implements TransactionStatsListener { private TransactionData data; private TransactionStats stats; + private static final String CONFIG_FILE = "configs/cross_app_tracing_test.yml"; + private static final ClassLoader CLASS_LOADER = APISupportabilityTest.class.getClassLoader(); @Before public void before() throws Exception { @@ -48,6 +53,13 @@ public void after() { ServiceFactory.getTransactionService().removeTransactionStatsListener(this); } + public EnvironmentHolder setupEnvironmentHolder(String environment) throws Exception { + EnvironmentHolderSettingsGenerator envHolderSettings = new EnvironmentHolderSettingsGenerator(CONFIG_FILE, environment, CLASS_LOADER); + EnvironmentHolder environmentHolder = new EnvironmentHolder(envHolderSettings); + environmentHolder.setupEnvironment(); + return environmentHolder; + } + public static class TokenApiDispatcherTestClass { @Trace(dispatcher = true) public void getTokenLinkAndExpire() { @@ -858,15 +870,22 @@ public void testNewRelicIgnoreNonDispatcherSupportabilityTracking() throws Excep @Test public void testProcessRequestMetadataDispatcherSupportabilityTracking() throws Exception { - String className = APISupportabilityTest.ProcessRequestMetadataDispatcherTestClass.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessRequestMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processRequestMetadataMetric = "Supportability/API/ProcessRequestMetadata/API"; - new ProcessRequestMetadataDispatcherTestClass().getProcessRequestMetadata(); + try { + String className = APISupportabilityTest.ProcessRequestMetadataDispatcherTestClass.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessRequestMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processRequestMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processRequestMetadataMetric)); + final String processRequestMetadataMetric = "Supportability/API/ProcessRequestMetadata/API"; + new ProcessRequestMetadataDispatcherTestClass().getProcessRequestMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processRequestMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processRequestMetadataMetric)); + } finally { + holder.close(); + } } @Test @@ -885,15 +904,22 @@ public void testProcessRequestMetadataNonDispatcherSupportabilityTracking() thro @Test public void testBridgeProcessRequestMetadataDispatcherSupportabilityTracking() throws Exception { - String className = APISupportabilityTest.ProcessRequestMetadataDispatcherBridgeTestClass.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessRequestMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processRequestMetadataMetric = "Supportability/API/ProcessRequestMetadata/API"; - new ProcessRequestMetadataDispatcherBridgeTestClass().getProcessRequestMetadata(); + try { + String className = APISupportabilityTest.ProcessRequestMetadataDispatcherBridgeTestClass.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessRequestMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processRequestMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processRequestMetadataMetric)); + final String processRequestMetadataMetric = "Supportability/API/ProcessRequestMetadata/API"; + new ProcessRequestMetadataDispatcherBridgeTestClass().getProcessRequestMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processRequestMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processRequestMetadataMetric)); + } finally { + holder.close(); + } } @Test @@ -912,15 +938,22 @@ public void testBridgeProcessRequestMetadataNonDispatcherSupportabilityTracking( @Test public void testProcessResponseMetadataDispatcherSupportabilityTracking1() throws Exception { - String className = APISupportabilityTest.ProcessResponseMetadataDispatcherTestClass1.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; - new ProcessResponseMetadataDispatcherTestClass1().getProcessResponseMetadata(); + try { + String className = APISupportabilityTest.ProcessResponseMetadataDispatcherTestClass1.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; + new ProcessResponseMetadataDispatcherTestClass1().getProcessResponseMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + } finally { + holder.close(); + } } @Test @@ -939,15 +972,22 @@ public void testProcessResponseMetadataNonDispatcherSupportabilityTracking1() th @Test public void testBridgeProcessResponseMetadataDispatcherSupportabilityTracking1() throws Exception { - String className = APISupportabilityTest.ProcessResponseMetadataDispatcherBridgeTestClass1.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; - new ProcessResponseMetadataDispatcherBridgeTestClass1().getProcessResponseMetadata(); + try { + String className = APISupportabilityTest.ProcessResponseMetadataDispatcherBridgeTestClass1.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; + new ProcessResponseMetadataDispatcherBridgeTestClass1().getProcessResponseMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + } finally { + holder.close(); + } } @Test @@ -966,15 +1006,22 @@ public void testBridgeProcessResponseMetadataNonDispatcherSupportabilityTracking @Test public void testProcessResponseMetadataDispatcherSupportabilityTracking2() throws Exception { - String className = APISupportabilityTest.ProcessResponseMetadataDispatcherTestClass2.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; - new ProcessResponseMetadataDispatcherTestClass2().getProcessResponseMetadata(); + try { + String className = APISupportabilityTest.ProcessResponseMetadataDispatcherTestClass2.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; + new ProcessResponseMetadataDispatcherTestClass2().getProcessResponseMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + } finally { + holder.close(); + } } @Test @@ -993,15 +1040,22 @@ public void testProcessResponseMetadataNonDispatcherSupportabilityTracking2() th @Test public void testBridgeProcessResponseMetadataDispatcherSupportabilityTracking2() throws Exception { - String className = APISupportabilityTest.ProcessResponseMetadataDispatcherBridgeTestClass2.class.getName(); - InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); - final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; - new ProcessResponseMetadataDispatcherBridgeTestClass2().getProcessResponseMetadata(); + try { + String className = APISupportabilityTest.ProcessResponseMetadataDispatcherBridgeTestClass2.class.getName(); + InstrumentTestUtils.createTransformerAndRetransformClass(className, "getProcessResponseMetadata", "()V;"); - Map metricData = InstrumentTestUtils.getAndClearMetricData(); - Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); - Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + final String processResponseMetadataMetric = "Supportability/API/ProcessResponseMetadata/API"; + new ProcessResponseMetadataDispatcherBridgeTestClass2().getProcessResponseMetadata(); + + Map metricData = InstrumentTestUtils.getAndClearMetricData(); + Assert.assertNotNull(metricData.get(processResponseMetadataMetric)); + Assert.assertEquals(Integer.valueOf(1), metricData.get(processResponseMetadataMetric)); + } finally { + holder.close(); + } } @Test From 041e981f60657b1955d7e2875caf79b66a3132df Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Thu, 16 Sep 2021 19:59:36 -0700 Subject: [PATCH 16/32] Fix APITest by disabling DT and enabling CAT --- .../test/newrelic/test/agent/api/ApiTest.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java b/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java index 138484ca42..30530ff1f7 100644 --- a/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java +++ b/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java @@ -22,6 +22,7 @@ import com.newrelic.agent.environment.AgentIdentity; import com.newrelic.agent.errors.ErrorService; import com.newrelic.agent.errors.TracedError; +import com.newrelic.agent.instrumentation.APISupportabilityTest; import com.newrelic.agent.metric.MetricName; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.stats.ResponseTimeStats; @@ -66,6 +67,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import test.newrelic.EnvironmentHolderSettingsGenerator; +import test.newrelic.test.agent.EnvironmentHolder; import javax.servlet.http.HttpServletResponse; import java.io.IOException; @@ -89,6 +92,8 @@ public class ApiTest implements TransactionListener { ApiTestHelper apiTestHelper = new ApiTestHelper(); + private static final String CONFIG_FILE = "configs/cross_app_tracing_test.yml"; + private static final ClassLoader CLASS_LOADER = ApiTest.class.getClassLoader(); @Before public void before() { @@ -110,6 +115,13 @@ public void after() { ServiceFactory.getTransactionService().addTransactionListener(this); } + public EnvironmentHolder setupEnvironmentHolder(String environment) throws Exception { + EnvironmentHolderSettingsGenerator envHolderSettings = new EnvironmentHolderSettingsGenerator(CONFIG_FILE, environment, CLASS_LOADER); + EnvironmentHolder environmentHolder = new EnvironmentHolder(envHolderSettings); + environmentHolder.setupEnvironment(); + return environmentHolder; + } + @Override public void dispatcherTransactionFinished(TransactionData transactionData, TransactionStats transactionStats) { apiTestHelper.tranStats = transactionStats; @@ -1869,7 +1881,9 @@ private void runTestExternalAPI() { /* External/CAT - FIT to Public API */ @Test - public void testExternalCatAPI() { + public void testExternalCatAPI() throws Exception { + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); TestServer server = new TestServer(8088); try { @@ -1881,6 +1895,7 @@ public void testExternalCatAPI() { } finally { Transaction.clearTransaction(); server.closeAllConnections(); + holder.close(); } } @@ -2030,8 +2045,11 @@ public String toObfuscatedQueryString(BsonDocument query) { /* Messaging - FIT to Public API */ @Test - public void testMessagingAPI() { + public void testMessagingAPI() throws Exception { + // override default agent config to disabled distributed tracing and use CAT instead + EnvironmentHolder holder = setupEnvironmentHolder("cat_enabled_dt_disabled_test"); MessagingTestServer server = new MessagingTestServer(8088); + try { server.start(); runTestMessagingAPI(); @@ -2043,6 +2061,7 @@ public void testMessagingAPI() { } finally { Transaction.clearTransaction(); server.closeAllConnections(); + holder.close(); } } From 7efb3aff4b8517964d87423f500165fe2e189c15 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 13:36:16 -0700 Subject: [PATCH 17/32] Fix CrossProcessStateCatApiTest by disabling DT and enabling CAT --- .../agent/CrossProcessStateCatApiTest.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessStateCatApiTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessStateCatApiTest.java index 2b289a0262..4dad978f94 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessStateCatApiTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessStateCatApiTest.java @@ -15,6 +15,7 @@ import com.newrelic.agent.config.ConfigServiceFactory; import com.newrelic.agent.config.CrossProcessConfig; import com.newrelic.agent.config.CrossProcessConfigImpl; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.dispatchers.Dispatcher; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.service.ServiceUtils; @@ -125,7 +126,8 @@ public void getResponseMetadata() throws ParseException { String responseMetaData = cps.getResponseMetadata(); JSONParser parser = new JSONParser(); - Map deobfuscatedResponseMetadata = (Map) parser.parse(Obfuscator.deobfuscateNameUsingKey(responseMetaData, ENCODING_KEY)); + Map deobfuscatedResponseMetadata = (Map) parser.parse( + Obfuscator.deobfuscateNameUsingKey(responseMetaData, ENCODING_KEY)); String newRelicAppData = deobfuscatedResponseMetadata.get("NewRelicAppData"); ArrayList appDataValues = (ArrayList) parser.parse(newRelicAppData); @@ -210,7 +212,7 @@ public void testCatApiResponseExternalMetrics() throws UnsupportedEncodingExcept setUpTransaction(transactionOne, txaOne, new Object(), new MockDispatcher(), cpsOneConfig, "guid"); MetricNameFormat nameFormat = new ClassMethodMetricNameFormat(new ClassMethodSignature("className", "methodName", "()V"), null, ""); CatTestCustomTracer requestTracer = new CatTestCustomTracer(transactionOne, new ClassMethodSignature("className", "methodName", - "()V"), new Object(), nameFormat , TracerFlags.DISPATCHER); + "()V"), new Object(), nameFormat, TracerFlags.DISPATCHER); when(txaOne.getLastTracer()).thenReturn(requestTracer); CrossProcessTransactionStateImpl cpsOne = CrossProcessTransactionStateImpl.create(transactionOne); @@ -227,8 +229,6 @@ public void testCatApiResponseExternalMetrics() throws UnsupportedEncodingExcept CrossProcessTransactionStateImpl cpsTwo = CrossProcessTransactionStateImpl.create(transactionTwo); - - String requestMetadata = cpsOne.getRequestMetadata(); // Transaction one generates requestMetadata. This metadata is embedded in payload and sent to transaction two. @@ -247,7 +247,7 @@ public void testCatApiResponseExternalMetrics() throws UnsupportedEncodingExcept } private void setUpTransaction(Transaction tx, TransactionActivity txa, Object lock, Dispatcher dispatcher, CrossProcessConfig config, - String guid) { + String guid) { when(txa.getTransaction()).thenReturn(tx); when(tx.getLock()).thenReturn(lock); @@ -272,15 +272,19 @@ private void setUpTransaction(Transaction tx, TransactionActivity txa, Object lo "category", TransactionNamePriority.FRAMEWORK); when(tx.getPriorityTransactionName()).thenReturn(priorityTransactionName); - TransactionCounts txnCounts = Mockito.mock(TransactionCounts.class); when(txnCounts.isOverTracerSegmentLimit()).thenReturn(false); when(tx.getTransactionCounts()).thenReturn(txnCounts); } private void setUpServiceManager() throws Exception { + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + Map configMap = ImmutableMap.builder() .put(AgentConfigImpl.APP_NAME, "TransactionAppNamingTest") + .put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings) .build(); ConfigService configService = ConfigServiceFactory.createConfigServiceUsingSettings(configMap); @@ -297,9 +301,15 @@ private CatTestCustomTracer trustAppAndGetTracer(String accountID) { .put(CrossProcessConfigImpl.CROSS_PROCESS_ID, accountID + "#878") .build(); + // Disable DT for CAT specific tests + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + Map settings = new HashMap<>(); settings.put(AgentConfigImpl.CROSS_APPLICATION_TRACER, crossProcessSettings); settings.put(AgentConfigImpl.APP_NAME, "TransactionAppNamingTest"); + settings.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); ConfigService configService = ConfigServiceFactory.createConfigServiceUsingSettings(settings); serviceManager.setConfigService(configService); @@ -310,7 +320,8 @@ private CatTestCustomTracer trustAppAndGetTracer(String accountID) { ClassMethodSignature classMethodSignature = new ClassMethodSignature("className", "methodName", "methodDesc"); MetricNameFormat metricNameFormat = new DefaultMetricNameFormat(classMethodSignature, "", "something"); - CatTestCustomTracer tracer = new CatTestCustomTracer(transaction, classMethodSignature, null, metricNameFormat, TracerFlags.DISPATCHER | TracerFlags.GENERATE_SCOPED_METRIC); + CatTestCustomTracer tracer = new CatTestCustomTracer(transaction, classMethodSignature, null, metricNameFormat, + TracerFlags.DISPATCHER | TracerFlags.GENERATE_SCOPED_METRIC); transaction.getTransactionActivity().tracerStarted(tracer); cps = transaction.getCrossProcessState(); @@ -325,7 +336,6 @@ private String constructRequestMetadata(String accountID, String appID, String g return Obfuscator.obfuscateNameUsingKey(metadata, ENCODING_KEY); } - // Exposes rollup metrics and attributes for testing private class CatTestCustomTracer extends OtherRootTracer { CatTestCustomTracer(Transaction transaction, ClassMethodSignature sig, Object object, From 2a509435895b12e5b58c532841eed47c417ed9f5 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 14:20:36 -0700 Subject: [PATCH 18/32] Fix InboundHeaderStateTest by disabling DT and enabling CAT --- .../agent/InboundHeaderStateTest.java | 81 +++++++------------ 1 file changed, 30 insertions(+), 51 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/InboundHeaderStateTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/InboundHeaderStateTest.java index b5041e275f..21ec64b569 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/InboundHeaderStateTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/InboundHeaderStateTest.java @@ -8,8 +8,10 @@ package com.newrelic.agent; import com.newrelic.agent.bridge.TransactionNamePriority; +import com.newrelic.agent.config.AgentConfig; import com.newrelic.agent.config.AgentConfigFactory; import com.newrelic.agent.config.CrossProcessConfig; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.dispatchers.Dispatcher; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.transaction.PriorityTransactionName; @@ -24,6 +26,7 @@ import java.util.Collections; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -35,36 +38,41 @@ public class InboundHeaderStateTest { private final String encodingKey = getClass().getName(); private InboundHeaderState ihs; private ExtendedRequest request; - private CrossProcessConfig config; + private CrossProcessConfig crossProcessConfig; + private DistributedTracingConfig distributedTracingConfig; + private AgentConfig agentConfig; private Transaction tx; @Before public void setup() { - serviceManager.setConfigService(new MockConfigService(AgentConfigFactory.createAgentConfig( - Collections. emptyMap(), Collections. emptyMap(), null))); + Collections.emptyMap(), Collections.emptyMap(), null))); ServiceFactory.setServiceManager(serviceManager); OutboundHeaders outboundHeaders = mock(OutboundHeaders.class); InboundHeaders inboundHeaders = mock(InboundHeaders.class); request = mock(ExtendedRequest.class); - config = mock(CrossProcessConfig.class); + agentConfig = mock(AgentConfig.class); + crossProcessConfig = mock(CrossProcessConfig.class); + distributedTracingConfig = mock(DistributedTracingConfig.class); tx = mock(Transaction.class); when(inboundHeaders.getHeaderType()).thenReturn(HeaderType.HTTP); when(outboundHeaders.getHeaderType()).thenReturn(HeaderType.HTTP); - when(config.getEncodingKey()).thenReturn(encodingKey); - when(config.isCrossApplicationTracing()).thenReturn(true); + when(crossProcessConfig.getEncodingKey()).thenReturn(encodingKey); + when(crossProcessConfig.isCrossApplicationTracing()).thenReturn(true); + when(distributedTracingConfig.isEnabled()).thenReturn(false); + when(agentConfig.getDistributedTracingConfig()).thenReturn(distributedTracingConfig); when(tx.isIgnore()).thenReturn(false); when(tx.getDispatcher()).thenReturn(mock(Dispatcher.class)); when(tx.getDispatcher().getRequest()).thenReturn(request); when(tx.getDispatcher().getRequest().getHeaderType()).thenReturn(HeaderType.HTTP); - when(tx.getCrossProcessConfig()).thenReturn(config); + when(tx.getCrossProcessConfig()).thenReturn(crossProcessConfig); + when(tx.getAgentConfig()).thenReturn(agentConfig); when(tx.getPriorityTransactionName()).thenReturn( PriorityTransactionName.create("Test", "TEST", TransactionNamePriority.NONE)); when(tx.getApplicationName()).thenReturn("TestApp"); when(tx.getLock()).thenReturn(new Object()); - when(tx.getAgentConfig()).thenReturn(ServiceFactory.getConfigService().getDefaultAgentConfig()); } @Test @@ -79,12 +87,10 @@ public void testNullHeaders() throws UnsupportedEncodingException { @Test public void testNoNRHeaders() { - when(request.getHeader("X-Foo")).thenReturn("FOO"); - when(request.getHeader("X-Bar")).thenReturn("BAR"); - when(request.getHeader("X-Baz")).thenReturn("BAZ"); when(request.getHeader("Content-Length")).thenReturn("42"); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -97,15 +103,10 @@ public void testNoNRHeaders() { @Test public void testBadNRHeaders1() { when(request.getHeader("X-Foo")).thenReturn("FOO"); - when(request.getHeader("X-Bar")).thenReturn("BAR"); - when(request.getHeader("X-Baz")).thenReturn("BAZ"); - when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn("FOO"); - // when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); - // when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn("BAZ"); - // when(request.getHeader("Content-Length")).thenReturn("42"); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -117,16 +118,11 @@ public void testBadNRHeaders1() { @Test public void testBadNRHeaders2() { - when(request.getHeader("X-Foo")).thenReturn("FOO"); when(request.getHeader("X-Bar")).thenReturn("BAR"); - when(request.getHeader("X-Baz")).thenReturn("BAZ"); - - // when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn("FOO"); when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); - // when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn("BAZ"); - // when(request.getHeader("Content-Length")).thenReturn("42"); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -138,16 +134,11 @@ public void testBadNRHeaders2() { @Test public void testBadNRHeaders3() { - when(request.getHeader("X-Foo")).thenReturn("FOO"); - when(request.getHeader("X-Bar")).thenReturn("BAR"); when(request.getHeader("X-Baz")).thenReturn("BAZ"); - - // when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn("FOO"); - // when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn("BAZ"); - // when(request.getHeader("Content-Length")).thenReturn("42"); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -162,13 +153,10 @@ public void testBadNRHeaders4() { when(request.getHeader("X-Foo")).thenReturn("FOO"); when(request.getHeader("X-Bar")).thenReturn("BAR"); when(request.getHeader("X-Baz")).thenReturn("BAZ"); - - // when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn("FOO"); - // when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); - // when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn("BAZ"); when(request.getHeader("Content-Length")).thenReturn("One would expect a number here."); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -178,9 +166,6 @@ public void testBadNRHeaders4() { assertEquals(-1L, ihs.getRequestContentLength()); } - // APP_DATA "[\"6#66\",\"WebTransaction\\/test\\/test\",1.0,0.2,12345,\"56b0d429ee4730fe\"]" - // TRANSACTION "[\"ee8e5ef1a374c0ec\",false,\"ee8e5ef1a374c0ec\",\"8fc87490\"]"; - @Test public void testBadNRHeaders5() { when(request.getHeader("X-Foo")).thenReturn("FOO"); @@ -189,12 +174,9 @@ public void testBadNRHeaders5() { when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn( "[\"6#66\",\"WebTransaction\\/test\\/test\",Nan,NotAFloat,NotAnInt,\"NotAHexString\"]"); - // when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); - // when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn( - // "[\"ee8e5ef1a374c0ec\",false,\"ee8e5ef1a374c0ec\",\"8fc87490\"]"); - // when(request.getHeader("Content-Length")).thenReturn("One would expect a number here."); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); @@ -210,14 +192,11 @@ public void testBadNRHeaders6() { when(request.getHeader("X-Bar")).thenReturn("BAR"); when(request.getHeader("X-Baz")).thenReturn("BAZ"); - // when(request.getHeader(HeadersUtil.NEWRELIC_APP_DATA_HEADER)).thenReturn( - // "[\"6#66\",\"WebTransaction\\/test\\/test\",Nan,NotAFloat,NotAnInt,\"NotAHexString\"]"); - // when(request.getHeader(HeadersUtil.NEWRELIC_ID_HEADER)).thenReturn("BAR"); when(request.getHeader(HeadersUtil.NEWRELIC_TRANSACTION_HEADER)).thenReturn( "[\"ee8e5ef1a374c0ec\",NotABoolean,\"ee8eZZZZZZc0ec\",\"8fZZZZ90\"]"); - // when(request.getHeader("Content-Length")).thenReturn("One would expect a number here."); - assertTrue(config.isCrossApplicationTracing()); + assertFalse(distributedTracingConfig.isEnabled()); + assertTrue(crossProcessConfig.isCrossApplicationTracing()); ihs = new InboundHeaderState(tx, request); assertNull(ihs.getClientCrossProcessId()); From be7a61ce57fe6feedfde4097310e891e856b9cfd Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 16:11:33 -0700 Subject: [PATCH 19/32] Fix BrowserConfigTest by disabling DT and enabling CAT --- .../agent/browser/BrowserConfigTest.java | 210 +++++++++--------- 1 file changed, 110 insertions(+), 100 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java index 0867892045..ac69f6955f 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java @@ -7,6 +7,7 @@ package com.newrelic.agent.browser; +import com.google.common.collect.ImmutableMap; import com.newrelic.agent.MockRPMService; import com.newrelic.agent.MockRPMServiceManager; import com.newrelic.agent.MockServiceManager; @@ -14,7 +15,9 @@ import com.newrelic.agent.TransactionService; import com.newrelic.agent.attributes.AttributesService; import com.newrelic.agent.bridge.TransactionNamePriority; +import com.newrelic.agent.config.AgentConfigImpl; import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.errors.ErrorServiceImpl; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.trace.TransactionTraceService; @@ -36,7 +39,7 @@ import java.util.regex.Pattern; /* (non-javadoc) - * Note: the "beacon" was a predecessor technology for correlated transaction traces with the browser. + * Note: the "beacon" was a predecessor technology for correlated transaction traces with the browser. * Some appearances of the term could be changed to "browser" now. */ @@ -57,6 +60,104 @@ public class BrowserConfigTest { "\"licenseKey\":\"3969ca217b\"", "\"transactionName\":\"DxIJAw==\"", "\"agent\":\"\"", "\"errorBeacon\":\"staging-jserror.newrelic.com\"", }; + // Check the start and end sequences of the RUM footer and add the matched strings to the + // list of all matched strings for this test. + public static void checkFooter(String toCheck, List matched) { + Assert.assertTrue(toCheck.startsWith(BrowserFooter.FOOTER_START_SCRIPT)); + matched.add(BrowserFooter.FOOTER_START_SCRIPT); + Assert.assertTrue(toCheck.endsWith(BrowserFooter.FOOTER_END)); + matched.add(BrowserFooter.FOOTER_END); + } + + // Check that value contains the JSON specified in the found in the array of expected strings, + // plus the RUM footer start and end strings, and nothing else. + public static void checkStrings(String toCheck, List expectedFooterProperties, List matched) { + + for (String toMatch : expectedFooterProperties) { + Pattern p = Pattern.compile(toMatch); + Matcher m = p.matcher(toCheck); + Assert.assertTrue("Not found: " + toMatch, m.find()); + + String s = m.group(0).trim(); + Assert.assertTrue(s.length() > 0); + matched.add(s); + } + + // Check for unexpected content in the returned string by knocking all the matches out + // and looking just for the correct separators. There should be one fewer commas in the + // expected delimiters than the number of strings in the expected array. + + for (String s : matched) { + toCheck = toCheck.replace(s, ""); + } + + // We want to make sure what's left is just the two curly braces and the commas that separated + // the elements in the javascript. But we don't want to make a pattern that always makes all + // commas optional. So we choose a distinct pattern when the number of expected footer properties + // was one (so there were no commas). + Pattern expect = Pattern.compile(expectedFooterProperties.size() == 1 ? "\\{\\}" : "\\{[,]+\\}"); + Assert.assertTrue("Unexpected content: " + toCheck, expect.matcher(toCheck).matches()); + } + + // Validate the userParams in the value, then call checkStrings on the *remaining* values. + public static void checkStringsAndUserParams(String toCheck, List expectedFooterProperties, + List expectedUserAttributes, List expectedAgentAttributes, List matched) { + + // Validate the attributes by grabbing the attribute, deobfuscating the string, and + // then checking for expected values in the JSON. + // + // Initially, here, we are looking for something like this: + // \"userAttributes\":\"GlEUFAgMHwgYTVNXHQAjFgkDHQkfThI=\" + // + // Note the leading comma in the pattern. We need to match on it and then + // replace it away before calling checkStrings() or the assert on the last + // line of checkStrings() will fail. + Pattern p = Pattern.compile(",\"atts\":\"[A-Za-z0-9+/=]+\""); + + Matcher m = p.matcher(toCheck); + Assert.assertTrue(m.find()); + String attributes = m.group(0).trim(); + toCheck = toCheck.replace(attributes, ""); + + attributes = attributes.replace("\"atts\":\"", ""); + attributes = attributes.replace("\"", ""); + attributes = attributes.replace(",", ""); + String deobfuscatedAttributes = Obfuscator.deobfuscateNameUsingKey(attributes, LICENSE_KEY.substring(0, 13)); + + // Now we need to check each key:value pair in the "u" and "a" attribute collections separately. + // Here is one example of what deobfuscatedAttributes might look like at this point: + // {"u":{"theLong":22,"theInt":11,"theShort":1,"theDouble":11.22,"theString":"abc123"},"a":{"key_1":valX,"key_2",valY}} + // Either or both of the "u" and "a" attributes may be absent, but there should be *something*, at least in + // these tests. + Assert.assertTrue(deobfuscatedAttributes != null && deobfuscatedAttributes.length() > 0); + deobfuscatedAttributes.replace(" ", ""); + + if (expectedUserAttributes != null) { + // Get the "u" list if present + Pattern getUList = Pattern.compile("\"u\":\\{.+?\\}"); + Matcher matchUList = getUList.matcher(deobfuscatedAttributes); + Assert.assertTrue(matchUList.find()); + String uList = matchUList.group(0).trim(); + uList = uList.replace("\"u\":", ""); + // We don't need to accumulate matches, so the third arguments is throwaway. + checkStrings(uList, expectedUserAttributes, new ArrayList()); + } + + if (expectedAgentAttributes != null) { + // Get the "a" list if present + Pattern getAList = Pattern.compile("\"a\":\\{.+?\\}"); + Matcher matchAList = getAList.matcher(deobfuscatedAttributes); + Assert.assertTrue(matchAList.find()); + String aList = matchAList.group(0).trim(); + aList = aList.replace("\"a\":", ""); + // We don't need to accumulate matches, so the third arguments is throwaway. + checkStrings(aList, expectedAgentAttributes, new ArrayList()); + } + + // Finally we can checkStrings on the non-attribute key, value pairs. + checkStrings(toCheck, expectedFooterProperties, matched); + } + public void setupManager(boolean captureParams, boolean setSslForHttpToTrue) { MockServiceManager manager = new MockServiceManager(); ServiceFactory.setServiceManager(manager); @@ -75,6 +176,13 @@ public void setupManager(boolean captureParams, boolean setSslForHttpToTrue) { } } params.put("license_key", LICENSE_KEY); + + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + + params.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); + manager.setConfigService(ConfigServiceFactory.createConfigServiceUsingSettings(params)); manager.setTransactionService(new TransactionService()); manager.setTransactionTraceService(new TransactionTraceService()); @@ -167,104 +275,6 @@ public void testFooterNoCaptureParams() throws Exception { checkStrings(value, expectedFooterProperties, matched); } - // Check the start and end sequences of the RUM footer and add the matched strings to the - // list of all matched strings for this test. - public static void checkFooter(String toCheck, List matched) { - Assert.assertTrue(toCheck.startsWith(BrowserFooter.FOOTER_START_SCRIPT)); - matched.add(BrowserFooter.FOOTER_START_SCRIPT); - Assert.assertTrue(toCheck.endsWith(BrowserFooter.FOOTER_END)); - matched.add(BrowserFooter.FOOTER_END); - } - - // Check that value contains the JSON specified in the found in the array of expected strings, - // plus the RUM footer start and end strings, and nothing else. - public static void checkStrings(String toCheck, List expectedFooterProperties, List matched) { - - for (String toMatch : expectedFooterProperties) { - Pattern p = Pattern.compile(toMatch); - Matcher m = p.matcher(toCheck); - Assert.assertTrue("Not found: " + toMatch, m.find()); - - String s = m.group(0).trim(); - Assert.assertTrue(s.length() > 0); - matched.add(s); - } - - // Check for unexpected content in the returned string by knocking all the matches out - // and looking just for the correct separators. There should be one fewer commas in the - // expected delimiters than the number of strings in the expected array. - - for (String s : matched) { - toCheck = toCheck.replace(s, ""); - } - - // We want to make sure what's left is just the two curly braces and the commas that separated - // the elements in the javascript. But we don't want to make a pattern that always makes all - // commas optional. So we choose a distinct pattern when the number of expected footer properties - // was one (so there were no commas). - Pattern expect = Pattern.compile(expectedFooterProperties.size() == 1 ? "\\{\\}" : "\\{[,]+\\}"); - Assert.assertTrue("Unexpected content: " + toCheck, expect.matcher(toCheck).matches()); - } - - // Validate the userParams in the value, then call checkStrings on the *remaining* values. - public static void checkStringsAndUserParams(String toCheck, List expectedFooterProperties, - List expectedUserAttributes, List expectedAgentAttributes, List matched) { - - // Validate the attributes by grabbing the attribute, deobfuscating the string, and - // then checking for expected values in the JSON. - // - // Initially, here, we are looking for something like this: - // \"userAttributes\":\"GlEUFAgMHwgYTVNXHQAjFgkDHQkfThI=\" - // - // Note the leading comma in the pattern. We need to match on it and then - // replace it away before calling checkStrings() or the assert on the last - // line of checkStrings() will fail. - Pattern p = Pattern.compile(",\"atts\":\"[A-Za-z0-9+/=]+\""); - - Matcher m = p.matcher(toCheck); - Assert.assertTrue(m.find()); - String attributes = m.group(0).trim(); - toCheck = toCheck.replace(attributes, ""); - - attributes = attributes.replace("\"atts\":\"", ""); - attributes = attributes.replace("\"", ""); - attributes = attributes.replace(",", ""); - String deobfuscatedAttributes = Obfuscator.deobfuscateNameUsingKey(attributes, LICENSE_KEY.substring(0, 13)); - - // Now we need to check each key:value pair in the "u" and "a" attribute collections separately. - // Here is one example of what deobfuscatedAttributes might look like at this point: - // {"u":{"theLong":22,"theInt":11,"theShort":1,"theDouble":11.22,"theString":"abc123"},"a":{"key_1":valX,"key_2",valY}} - // Either or both of the "u" and "a" attributes may be absent, but there should be *something*, at least in - // these tests. - Assert.assertTrue(deobfuscatedAttributes != null && deobfuscatedAttributes.length() > 0); - deobfuscatedAttributes.replace(" ", ""); - - if (expectedUserAttributes != null) { - // Get the "u" list if present - Pattern getUList = Pattern.compile("\"u\":\\{.+?\\}"); - Matcher matchUList = getUList.matcher(deobfuscatedAttributes); - Assert.assertTrue(matchUList.find()); - String uList = matchUList.group(0).trim(); - uList = uList.replace("\"u\":", ""); - // We don't need to accumulate matches, so the third arguments is throwaway. - checkStrings(uList, expectedUserAttributes, new ArrayList()); - } - - if (expectedAgentAttributes != null) { - // Get the "a" list if present - Pattern getAList = Pattern.compile("\"a\":\\{.+?\\}"); - Matcher matchAList = getAList.matcher(deobfuscatedAttributes); - Assert.assertTrue(matchAList.find()); - String aList = matchAList.group(0).trim(); - aList = aList.replace("\"a\":", ""); - // We don't need to accumulate matches, so the third arguments is throwaway. - checkStrings(aList, expectedAgentAttributes, new ArrayList()); - } - - // Finally we can checkStrings on the non-attribute key, value pairs. - checkStrings(toCheck, expectedFooterProperties, matched); - } - @Test public void testFooterCaptureAtts() throws Exception { setupManager(true, false); @@ -301,7 +311,7 @@ public void testFooterCaptureAtts() throws Exception { @Test public void testFooterCaptureAttsOneAndSsl() throws Exception { setupManager(true, true); - Transaction tx = Transaction.getTransaction(); + Transaction tx = Transaction.getTransaction(); // getAgentConfig().getDistributedTracingConfig().isEnabled() is true BasicRequestRootTracer tracer = createDispatcherTracer(); tx.getTransactionActivity().tracerStarted(tracer); TransactionNamePriority expectedPriority = TransactionNamePriority.FILTER_NAME; From d928ee53e8c05e897c88ab287006c8831adb7c63 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 16:19:56 -0700 Subject: [PATCH 20/32] Fix ErrorServiceTest by setting correct intrinsics asserts when DT is enabled --- .../agent/errors/ErrorServiceTest.java | 134 +++++++++--------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/errors/ErrorServiceTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/errors/ErrorServiceTest.java index 148bb3fe55..3b14669060 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/errors/ErrorServiceTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/errors/ErrorServiceTest.java @@ -536,7 +536,7 @@ public void attributesDisabledWithIntrinsics() throws Exception { Assert.assertNotNull(params.get("stack_trace")); Assert.assertNotNull(params.get("intrinsics")); Map actual = (Map) params.get("intrinsics"); - Assert.assertEquals(2, actual.size()); + Assert.assertEquals(6, actual.size()); Assert.assertEquals("value5", actual.get("key5")); Assert.assertEquals(false, actual.get("error.expected")); } @@ -585,7 +585,7 @@ public void userParametersEnabled() throws Exception { Assert.assertNotNull(params.get("userAttributes")); Map actual = (Map) params.get("intrinsics"); - Assert.assertEquals(4, actual.size()); + Assert.assertEquals(8, actual.size()); Assert.assertEquals("value5", actual.get("key5")); Assert.assertEquals(7.77, (Double) actual.get("key6"), .001); Assert.assertEquals(18L, actual.get("key7")); @@ -647,7 +647,7 @@ public void userParametersEnabledRequestDisabled() throws Exception { Assert.assertNotNull(params.get("userAttributes")); Map actual = (Map) params.get("intrinsics"); - Assert.assertEquals(4, actual.size()); + Assert.assertEquals(8, actual.size()); Assert.assertEquals("value5", actual.get("key5")); Assert.assertEquals(7.77, (Double) actual.get("key6"), .001); Assert.assertEquals(18L, actual.get("key7")); @@ -711,70 +711,6 @@ public void dontReportException() throws Exception { Assert.assertTrue(tracedErrors.isEmpty()); } - // An object that knows how to add "stuff" to a test configuration. - private abstract class ConfigEnhancer { - abstract void enhance(Map config, Throwable expected); - - boolean shouldMatch() { - return false; - } - } - - // An enhancer that doesn't enhance - for support of legacy test case - private class NoOpConfigEnhancer extends ConfigEnhancer { - @Override - void enhance(Map config, Throwable expected) { - } - } - - // Add an error collector config that doesn't match anything the test does. - private class NonMatchingConfigEnhancer extends ConfigEnhancer { - @Override - void enhance(Map config, Throwable expected) { - List> ignoreErrorsList = new ArrayList<>(); - Map ignoreErrorMap = new HashMap<>(); - Map otherIgnoreErrorMap = new HashMap<>(); - ignoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.enron.power.MarketNotCorneredException"); - otherIgnoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.informix.cfo.GetOutOfJailFreeCard"); - ignoreErrorsList.add(ignoreErrorMap); - ignoreErrorsList.add(otherIgnoreErrorMap); - - Map errorCollectorMap = new HashMap<>(); - errorCollectorMap.put("enabled", true); - errorCollectorMap.put(ErrorCollectorConfigImpl.COLLECT_ERRORS, true); - errorCollectorMap.put(ErrorCollectorConfigImpl.IGNORE_CLASSES, ignoreErrorsList); - - config.put("error_collector", errorCollectorMap); - } - } - - // Add an error collector config that matches the exception thrown by the test, filtering it. - private class MatchingConfigEnhancer extends ConfigEnhancer { - @Override - void enhance(Map config, Throwable expected) { - List> ignoreErrorsList = new ArrayList<>(); - Map ignoreErrorMap = new HashMap<>(); - Map otherIgnoreErrorMap = new HashMap<>(); - ignoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, expected.getClass().getName()); - ignoreErrorMap.put(ErrorCollectorConfigImpl.MESSAGE, expected.getMessage()); - otherIgnoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.informix.cfo.GetOutOfJailFreeCard"); - ignoreErrorsList.add(ignoreErrorMap); - ignoreErrorsList.add(otherIgnoreErrorMap); - - Map errorCollectorMap = new HashMap<>(); - errorCollectorMap.put("enabled", true); - errorCollectorMap.put(ErrorCollectorConfigImpl.COLLECT_ERRORS, true); - errorCollectorMap.put(ErrorCollectorConfigImpl.IGNORE_CLASSES, ignoreErrorsList); - - config.put("error_collector", errorCollectorMap); - } - - @Override - boolean shouldMatch() { - return true; - } - } - /** * This test verifies all possible combinations of high security + strip exceptions + exceptions not to strip * This is the pre-expected-errors feature ("legacy") test that does not specify an error collector config. @@ -1165,4 +1101,68 @@ private TransactionData createTransactionData(boolean isWebTransaction, int resp .setIntrinsics(intrinsics) .build(); } + + // An object that knows how to add "stuff" to a test configuration. + private abstract class ConfigEnhancer { + abstract void enhance(Map config, Throwable expected); + + boolean shouldMatch() { + return false; + } + } + + // An enhancer that doesn't enhance - for support of legacy test case + private class NoOpConfigEnhancer extends ConfigEnhancer { + @Override + void enhance(Map config, Throwable expected) { + } + } + + // Add an error collector config that doesn't match anything the test does. + private class NonMatchingConfigEnhancer extends ConfigEnhancer { + @Override + void enhance(Map config, Throwable expected) { + List> ignoreErrorsList = new ArrayList<>(); + Map ignoreErrorMap = new HashMap<>(); + Map otherIgnoreErrorMap = new HashMap<>(); + ignoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.enron.power.MarketNotCorneredException"); + otherIgnoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.informix.cfo.GetOutOfJailFreeCard"); + ignoreErrorsList.add(ignoreErrorMap); + ignoreErrorsList.add(otherIgnoreErrorMap); + + Map errorCollectorMap = new HashMap<>(); + errorCollectorMap.put("enabled", true); + errorCollectorMap.put(ErrorCollectorConfigImpl.COLLECT_ERRORS, true); + errorCollectorMap.put(ErrorCollectorConfigImpl.IGNORE_CLASSES, ignoreErrorsList); + + config.put("error_collector", errorCollectorMap); + } + } + + // Add an error collector config that matches the exception thrown by the test, filtering it. + private class MatchingConfigEnhancer extends ConfigEnhancer { + @Override + void enhance(Map config, Throwable expected) { + List> ignoreErrorsList = new ArrayList<>(); + Map ignoreErrorMap = new HashMap<>(); + Map otherIgnoreErrorMap = new HashMap<>(); + ignoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, expected.getClass().getName()); + ignoreErrorMap.put(ErrorCollectorConfigImpl.MESSAGE, expected.getMessage()); + otherIgnoreErrorMap.put(ErrorCollectorConfigImpl.CLASS_NAME, "com.informix.cfo.GetOutOfJailFreeCard"); + ignoreErrorsList.add(ignoreErrorMap); + ignoreErrorsList.add(otherIgnoreErrorMap); + + Map errorCollectorMap = new HashMap<>(); + errorCollectorMap.put("enabled", true); + errorCollectorMap.put(ErrorCollectorConfigImpl.COLLECT_ERRORS, true); + errorCollectorMap.put(ErrorCollectorConfigImpl.IGNORE_CLASSES, ignoreErrorsList); + + config.put("error_collector", errorCollectorMap); + } + + @Override + boolean shouldMatch() { + return true; + } + } } From 7c4397638ce38290b80d21448f5a1910fd7229c5 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 17:10:01 -0700 Subject: [PATCH 21/32] Fix TransactionTest by setting correct intrinsics when DT is enabled --- .../newrelic/agent/metrics/testcases/multi_async.json | 8 ++++---- .../newrelic/agent/metrics/testcases/two_threads.json | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/multi_async.json b/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/multi_async.json index b71936ae8a..ad1980bc6d 100644 --- a/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/multi_async.json +++ b/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/multi_async.json @@ -31,7 +31,7 @@ [2, 6, "Java/java.lang.Object/seventhRootTracer", {"async_context":"*", "exclusive_duration_millis":4.0}, [], "clazz", "seventhRootTracer"], [2, 7, "Java/java.lang.Object/eigthRootTracer", {"async_context":"*", "exclusive_duration_millis":5.0}, [], "clazz", "eigthRootTracer"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.055, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.055, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 14, "total_time" : 55} }, @@ -67,7 +67,7 @@ [2, 6, "Java/java.lang.Object/seventhRootTracer", {"async_context":"*", "exclusive_duration_millis":4.0}, [], "clazz", "seventhRootTracer"], [2, 7, "Java/java.lang.Object/eigthRootTracer", {"async_context":"*", "exclusive_duration_millis":5.0}, [], "clazz", "eigthRootTracer"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.055, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.055, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 6, "total_time" : 55} }, @@ -109,7 +109,7 @@ ], "clazz", "thirdRootTracer"] ], "clazz", "secondRootTracer"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.012, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.012, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 12, "total_time" : 12} }, @@ -151,7 +151,7 @@ ], "clazz", "thirdRootTracer"] ], "clazz", "secondRootTracer"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.012, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.012, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 5, "total_time" : 12} }, diff --git a/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/two_threads.json b/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/two_threads.json index bba99b3679..b4d6432dfc 100644 --- a/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/two_threads.json +++ b/newrelic-agent/src/test/resources/com/newrelic/agent/metrics/testcases/two_threads.json @@ -21,7 +21,7 @@ ], "clazz","secondRootTracer"] ], "clazz","t1"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.014, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.014, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 10, "total_time" : 14} }, @@ -47,7 +47,7 @@ ], "clazz","secondRootTracer"] ], "clazz","t1"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.015, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.015, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 20, "total_time" : 15} }, @@ -172,7 +172,7 @@ ], "clazz","secondRootTracer"] ], "clazz","t1"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToFirstByte":0.008, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToFirstByte":0.008, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 10, "total_time" : 25, "ttfb": 8} }, @@ -209,7 +209,7 @@ ], "clazz","secondRootTracer"] ], "clazz","t1"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToFirstByte":0.007, "timeToLastByte":0.008, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToFirstByte":0.007, "timeToLastByte":0.008, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 20, "total_time" : 25, "ttlb": 8, "ttfb": 7} }, @@ -244,7 +244,7 @@ ], "clazz","secondRootTracer"] ], "clazz","t1"] ], "clazz", "mainRootTracer"] - ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToLastByte":0.009, "cpuTime":"*", "priority":"*"} + ], "clazz", "mainRootTracer" ], "intrinsics":{"totalTime":0.025, "timeToLastByte":0.009, "cpuTime":"*", "priority":"*", "traceId":"*", "guid":"*", "parent.transportType":"*", "sampled":"*"} }, "transaction_event" : {"duration": 20, "total_time" : 25, "ttlb": 9} } From eb0c0d419dfc3e4cf21b1814bd512b13afda63eb Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 17:33:19 -0700 Subject: [PATCH 22/32] Fix TransactionEventTest by asserting that CAT attributes aren't present when DT is enabled --- .../agent/service/analytics/TransactionEventTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/TransactionEventTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/TransactionEventTest.java index 96375ddb09..122d868849 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/TransactionEventTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/TransactionEventTest.java @@ -402,10 +402,11 @@ public void testJsonWithPathHashes() throws Exception { .build(); JSONArray jsonArray = (JSONArray) AgentHelper.serializeJSON(event); JSONObject jsonObject = (JSONObject) jsonArray.get(0); - assertEquals(12, jsonObject.size()); - assertEquals("0000000c", jsonObject.get("nr.pathHash")); - assertEquals("0000000d", jsonObject.get("nr.referringPathHash")); - assertEquals("14", jsonObject.get("nr.alternatePathHashes")); + assertEquals(9, jsonObject.size()); + // These should only be set when using CAT, not DT as the agent does by default as of 7.3.0 + assertNull(jsonObject.get("nr.pathHash")); + assertNull(jsonObject.get("nr.referringPathHash")); + assertNull(jsonObject.get("nr.alternatePathHashes")); } @Test From d6753cf3e2ccdc953beabb2a1fc15bb7f3576478 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 17:38:49 -0700 Subject: [PATCH 23/32] Remove comment left by mistake --- .../test/java/com/newrelic/agent/browser/BrowserConfigTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java index ac69f6955f..a36fce44de 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java @@ -311,7 +311,7 @@ public void testFooterCaptureAtts() throws Exception { @Test public void testFooterCaptureAttsOneAndSsl() throws Exception { setupManager(true, true); - Transaction tx = Transaction.getTransaction(); // getAgentConfig().getDistributedTracingConfig().isEnabled() is true + Transaction tx = Transaction.getTransaction(); BasicRequestRootTracer tracer = createDispatcherTracer(); tx.getTransactionActivity().tracerStarted(tracer); TransactionNamePriority expectedPriority = TransactionNamePriority.FILTER_NAME; From 3866aa698a1ba091ee94ed6e701496fb036fb98c Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 17:54:44 -0700 Subject: [PATCH 24/32] Fix TransactionTraceTest by disabling DT and enabling CAT --- .../com/newrelic/agent/trace/TransactionTraceTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/trace/TransactionTraceTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/trace/TransactionTraceTest.java index bfcc7249fd..782dbe3990 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/trace/TransactionTraceTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/trace/TransactionTraceTest.java @@ -7,6 +7,7 @@ package com.newrelic.agent.trace; +import com.google.common.collect.ImmutableMap; import com.newrelic.agent.AgentHelper; import com.newrelic.agent.MockConfigService; import com.newrelic.agent.MockDispatcher; @@ -23,6 +24,7 @@ import com.newrelic.agent.config.AgentConfigImpl; import com.newrelic.agent.config.ConfigService; import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.config.TransactionTracerConfig; import com.newrelic.agent.database.SqlObfuscator; import com.newrelic.agent.dispatchers.Dispatcher; @@ -86,7 +88,12 @@ private void setUp(boolean isCaptureAtts, boolean captureRequestAtts, boolean re MockServiceManager manager = new MockServiceManager(); ServiceFactory.setServiceManager(manager); + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + Map settings = new HashMap<>(); + settings.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); setConfigAttributes(settings, isCaptureAtts, captureRequestAtts, requestUri, simpleCompression); ConfigService cService = new MockConfigService(AgentConfigImpl.createAgentConfig(settings)); manager.setConfigService(cService); From 17522cababf1a83ac8cb7e88a64acb1a725e9e65 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 17:59:21 -0700 Subject: [PATCH 25/32] Fix AbstractPriorityTransactionNamingPolicyTest by disabling DT and enabling CAT --- ...ctPriorityTransactionNamingPolicyTest.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/AbstractPriorityTransactionNamingPolicyTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/AbstractPriorityTransactionNamingPolicyTest.java index 0147caabb0..c8e7911c16 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/AbstractPriorityTransactionNamingPolicyTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/AbstractPriorityTransactionNamingPolicyTest.java @@ -7,12 +7,7 @@ package com.newrelic.agent.transaction; -import java.util.Collections; - -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - +import com.google.common.collect.ImmutableMap; import com.newrelic.agent.HarvestService; import com.newrelic.agent.MetricNames; import com.newrelic.agent.MockHarvestService; @@ -27,6 +22,7 @@ import com.newrelic.agent.config.AgentConfigImpl; import com.newrelic.agent.config.ConfigService; import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.sql.SqlTraceService; import com.newrelic.agent.sql.SqlTraceServiceImpl; @@ -39,6 +35,12 @@ import com.newrelic.agent.tracers.servlet.BasicRequestRootTracer; import com.newrelic.agent.tracers.servlet.MockHttpRequest; import com.newrelic.agent.tracers.servlet.MockHttpResponse; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; public class AbstractPriorityTransactionNamingPolicyTest { @@ -57,8 +59,15 @@ private MockServiceManager createServiceManager() throws Exception { ThreadService threadService = new ThreadService(); serviceManager.setThreadService(threadService); - AgentConfig config = AgentConfigImpl.createAgentConfig(Collections.EMPTY_MAP); - ConfigService configService = ConfigServiceFactory.createConfigService(config, Collections.EMPTY_MAP); + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + + Map settings = new HashMap<>(); + settings.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); + + AgentConfig config = AgentConfigImpl.createAgentConfig(settings); + ConfigService configService = ConfigServiceFactory.createConfigService(config, settings); serviceManager.setConfigService(configService); HarvestService harvestService = new MockHarvestService(); From 50a8650b569562faf95921ce8e9a950c20b15254 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 18:01:38 -0700 Subject: [PATCH 26/32] Fix HigherPriorityTransactionNamingPolicyTest by disabling DT and enabling CAT --- ...erPriorityTransactionNamingPolicyTest.java | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/HigherPriorityTransactionNamingPolicyTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/HigherPriorityTransactionNamingPolicyTest.java index af52ef19a1..a91a27339d 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/HigherPriorityTransactionNamingPolicyTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/HigherPriorityTransactionNamingPolicyTest.java @@ -7,12 +7,7 @@ package com.newrelic.agent.transaction; -import java.util.Collections; - -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - +import com.google.common.collect.ImmutableMap; import com.newrelic.agent.HarvestService; import com.newrelic.agent.MockHarvestService; import com.newrelic.agent.MockRPMServiceManager; @@ -20,15 +15,22 @@ import com.newrelic.agent.ThreadService; import com.newrelic.agent.Transaction; import com.newrelic.agent.TransactionService; +import com.newrelic.agent.bridge.TransactionNamePriority; import com.newrelic.agent.config.AgentConfig; import com.newrelic.agent.config.AgentConfigImpl; import com.newrelic.agent.config.ConfigService; import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.sql.SqlTraceService; import com.newrelic.agent.sql.SqlTraceServiceImpl; import com.newrelic.agent.trace.TransactionTraceService; -import com.newrelic.agent.bridge.TransactionNamePriority; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; public class HigherPriorityTransactionNamingPolicyTest { @@ -47,8 +49,15 @@ private MockServiceManager createServiceManager() throws Exception { ThreadService threadService = new ThreadService(); serviceManager.setThreadService(threadService); - AgentConfig config = AgentConfigImpl.createAgentConfig(Collections.EMPTY_MAP); - ConfigService configService = ConfigServiceFactory.createConfigService(config, Collections.EMPTY_MAP); + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + + Map settings = new HashMap<>(); + settings.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); + + AgentConfig config = AgentConfigImpl.createAgentConfig(settings); + ConfigService configService = ConfigServiceFactory.createConfigService(config, settings); serviceManager.setConfigService(configService); HarvestService harvestService = new MockHarvestService(); From 4cb11bfab3cb1210f6644332a540875471080301 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 18:04:19 -0700 Subject: [PATCH 27/32] Fix SameOrHigherPriorityTransactionNamingPolicyTest by disabling DT and enabling CAT --- ...erPriorityTransactionNamingPolicyTest.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/SameOrHigherPriorityTransactionNamingPolicyTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/SameOrHigherPriorityTransactionNamingPolicyTest.java index a0a820fec0..e3587de154 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/transaction/SameOrHigherPriorityTransactionNamingPolicyTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/transaction/SameOrHigherPriorityTransactionNamingPolicyTest.java @@ -7,12 +7,7 @@ package com.newrelic.agent.transaction; -import java.util.Collections; - -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - +import com.google.common.collect.ImmutableMap; import com.newrelic.agent.HarvestService; import com.newrelic.agent.MockHarvestService; import com.newrelic.agent.MockRPMServiceManager; @@ -24,10 +19,17 @@ import com.newrelic.agent.config.AgentConfigImpl; import com.newrelic.agent.config.ConfigService; import com.newrelic.agent.config.ConfigServiceFactory; +import com.newrelic.agent.config.DistributedTracingConfig; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.sql.SqlTraceService; import com.newrelic.agent.sql.SqlTraceServiceImpl; import com.newrelic.agent.trace.TransactionTraceService; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; public class SameOrHigherPriorityTransactionNamingPolicyTest { @@ -46,8 +48,15 @@ private MockServiceManager createServiceManager() throws Exception { ThreadService threadService = new ThreadService(); serviceManager.setThreadService(threadService); + ImmutableMap distributedTracingSettings = ImmutableMap.builder() + .put(DistributedTracingConfig.ENABLED, Boolean.FALSE) + .build(); + + Map settings = new HashMap<>(); + settings.put(AgentConfigImpl.DISTRIBUTED_TRACING, distributedTracingSettings); + ConfigService configService = ConfigServiceFactory.createConfigService( - AgentConfigImpl.createAgentConfig(Collections.EMPTY_MAP), Collections.EMPTY_MAP); + AgentConfigImpl.createAgentConfig(settings), settings); serviceManager.setConfigService(configService); HarvestService harvestService = new MockHarvestService(); From ee3e25beda0e4c384ff0345a7ee03aac58d44aec Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 20 Sep 2021 18:08:55 -0700 Subject: [PATCH 28/32] Trigger tests --- .../test/java/com/newrelic/agent/browser/BrowserConfigTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java index a36fce44de..8d016b7cc0 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/browser/BrowserConfigTest.java @@ -42,7 +42,6 @@ * Note: the "beacon" was a predecessor technology for correlated transaction traces with the browser. * Some appearances of the term could be changed to "browser" now. */ - public class BrowserConfigTest { private static final String LOADER = "window.NREUM||(NREUM={}),__nr_require=function a(b,c,d){function e(f){if(!c[f]){var g=c[f]={exports:{}};b[f][0].call(g.exports,function(a){var c=b[f][1][a];return e(c?c:a)},g,g.exports,a,b,c,d)}return c[f].exports}for(var f=0;f Date: Mon, 27 Sep 2021 11:34:00 -0700 Subject: [PATCH 29/32] Add span_events config to default yaml --- .../src/main/resources/newrelic.yml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/newrelic-agent/src/main/resources/newrelic.yml b/newrelic-agent/src/main/resources/newrelic.yml index e659f5f022..94f3e83db1 100644 --- a/newrelic-agent/src/main/resources/newrelic.yml +++ b/newrelic-agent/src/main/resources/newrelic.yml @@ -221,6 +221,33 @@ common: &default_settings # Default is false. exclude_newrelic_header: false + # New Relic's distributed tracing UI uses Span events to display traces across different services. + # Span events capture attributes that describe execution context and provide linking metadata. + # Span events require distributed tracing to be enabled. + span_events: + + # Set to false to disable Span events. + # Default is true. + enabled: true + + # Determines the number of Span events that can be captured during an agent harvest cycle. + # Increasing the number of Span events can lead to additional agent overhead. A maximum value may be imposed server side by New Relic. + # Default is 2000 + max_samples_stored: 2000 + + # Provides the ability to filter the attributes attached to Span events. + # Custom attributes can be added to Span events using the NewRelic.getAgent().getTracedMethod().addCustomAttribute(...) API. + attributes: + + # When true, attributes will be sent to New Relic. The default is true. + enabled: true + + # A comma separated list of attribute keys whose values should be sent to New Relic. + #include: + + # A comma separated list of attribute keys whose values should not be sent to New Relic. + #exclude: + # Cross Application Tracing adds request and response headers to # external calls using supported HTTP libraries to provide better # performance data when calling applications monitored by other New Relic Agents. From 1530a7afce4580c13e74c1da51c4d7ff60bf2fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Onuki?= Date: Mon, 11 Oct 2021 14:16:59 -0400 Subject: [PATCH 30/32] Deprecate CAT (#465) * Deprecating TracedMethod.addOutboundRequestHeaders * Disabling CAT by default --- .../com/newrelic/agent/config/ConfigServiceImpl.java | 7 +++++++ .../newrelic/agent/config/CrossProcessConfigImpl.java | 2 +- newrelic-agent/src/main/resources/newrelic.yml | 11 ++++++++--- .../agent/CrossProcessAndSyntheticsConfigTest.java | 4 ++-- .../newrelic/agent/config/AgentConfigFactoryTest.java | 8 +++++++- .../agent/config/CrossProcessConfigImplTest.java | 4 ++++ .../config/newrelicWithBrowserMonitoringFalse.yml | 3 +++ .../newrelicWithBrowserMonitoringNotSpecified.yml | 3 +++ .../java/com/newrelic/api/agent/TracedMethod.java | 3 +++ 9 files changed, 38 insertions(+), 7 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigServiceImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigServiceImpl.java index ef07c5f651..9bc07ae9b3 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigServiceImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/ConfigServiceImpl.java @@ -97,6 +97,13 @@ protected void doStart() { getLogger().warning(msg); } + boolean isCrossApplicationTracing = localAgentConfig.getCrossProcessConfig().isCrossApplicationTracing(); + if (isCrossApplicationTracing) { + String msg = "Distributed tracing is replacing cross application tracing as the default means of tracing between services. " + + "To continue using cross application tracing, enable it with cross_application_tracer.enabled=true and distributed_tracing.enabled=false. "; + getLogger().info(msg); + } + localAgentConfig.logDeprecatedProperties(fileSettings); ServiceFactory.getHarvestService().addHarvestListener(this); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/CrossProcessConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/CrossProcessConfigImpl.java index 286c46221e..842bf6adfe 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/CrossProcessConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/CrossProcessConfigImpl.java @@ -22,7 +22,7 @@ public final class CrossProcessConfigImpl extends BaseConfig implements CrossPro public static final String ENCODING_KEY = "encoding_key"; public static final String TRUSTED_ACCOUNT_IDS = "trusted_account_ids"; public static final String APPLICATION_ID = "application_id"; - public static final boolean DEFAULT_ENABLED = true; + public static final boolean DEFAULT_ENABLED = false; public static final String SYSTEM_PROPERTY_ROOT = "newrelic.config.cross_application_tracer."; private final boolean isCrossApplicationTracing; diff --git a/newrelic-agent/src/main/resources/newrelic.yml b/newrelic-agent/src/main/resources/newrelic.yml index 94f3e83db1..a7d6bf2445 100644 --- a/newrelic-agent/src/main/resources/newrelic.yml +++ b/newrelic-agent/src/main/resources/newrelic.yml @@ -251,11 +251,16 @@ common: &default_settings # Cross Application Tracing adds request and response headers to # external calls using supported HTTP libraries to provide better # performance data when calling applications monitored by other New Relic Agents. + # + # Distributed tracing is replacing cross application tracing as the default + # means of tracing between services. To continue using cross application + # tracing, enable it with `cross_application_tracer.enabled = true` and + # `distributed_tracing.enabled = false` cross_application_tracer: - # Set to false to disable cross application tracing. - # Default is true. - enabled: true + # Set to true to enable cross application tracing. + # Default is false. + enabled: false # Thread profiler measures wall clock time, CPU time, and method call counts # in your application's threads as they run. diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessAndSyntheticsConfigTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessAndSyntheticsConfigTest.java index 0428a2f91a..911caa7976 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessAndSyntheticsConfigTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/CrossProcessAndSyntheticsConfigTest.java @@ -120,7 +120,7 @@ private String makeYmlFilePath(String fileName) { @Test public void testConfig1() throws Exception { - testConfig(makeYmlFilePath("newrelic.yml"), true, true); + testConfig(makeYmlFilePath("newrelic.yml"), false, true); } @Test @@ -140,7 +140,7 @@ public void testConfig4() throws Exception { @Test public void testConfig5() throws Exception { - testConfig(makeYmlFilePath("newrelicWithCrossAppTracingNotSpecified.yml"), true, true); + testConfig(makeYmlFilePath("newrelicWithCrossAppTracingNotSpecified.yml"), false, true); } // The "expected value" arguments are all about the newrelic.yml. We're not varying the collector JSON here. diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigFactoryTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigFactoryTest.java index 89c8000c7a..6842128be0 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigFactoryTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/AgentConfigFactoryTest.java @@ -187,10 +187,16 @@ public void serverSideOverride() throws Exception { @Test public void crossProcess() throws Exception { Map localSettings = createMap(); + Map catSettings = createMap(); Map serverData = createMap(); + + // cat must be enabled for the other properties to be returned + catSettings.put(CrossProcessConfigImpl.ENABLED, true); + localSettings.put(AgentConfigImpl.CROSS_APPLICATION_TRACER, catSettings); + List trustedIds = new ArrayList<>(); trustedIds.add("12345"); - serverData.put(CrossProcessConfigImpl.CROSS_APPLICATION_TRACING, false); // deprecated setting + serverData.put(CrossProcessConfigImpl.CROSS_APPLICATION_TRACING, false); // deprecated setting is ignored when coming from the server serverData.put(CrossProcessConfigImpl.CROSS_PROCESS_ID, "1234#5678"); serverData.put(CrossProcessConfigImpl.TRUSTED_ACCOUNT_IDS, trustedIds); serverData.put(CrossProcessConfigImpl.ENCODING_KEY, "test"); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/CrossProcessConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/CrossProcessConfigImplTest.java index c3cc6e5689..1d41c2070a 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/CrossProcessConfigImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/CrossProcessConfigImplTest.java @@ -84,6 +84,10 @@ public void trustedIds() throws Exception { @Test public void encodingKey() throws Exception { Map serverSettings = new HashMap<>(); + + // CAT must be enabled, or the encoding key will be null + serverSettings.put(CrossProcessConfigImpl.ENABLED, true); + String encodingKey = "test"; ServerProp serverProp = ServerProp.createPropObject(encodingKey); serverSettings.put(CrossProcessConfigImpl.ENCODING_KEY, serverProp); diff --git a/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringFalse.yml b/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringFalse.yml index 67710706b5..5745a9d4d7 100644 --- a/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringFalse.yml +++ b/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringFalse.yml @@ -16,6 +16,9 @@ common: &default_settings wait_for_rpm_connect: false + cross_application_tracer: + enabled: true + # activemerchant instrumentation: only report dollar amounts (business data) # if the following is present and true; otherwise, only response times # and throughput to the gateway are reported diff --git a/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringNotSpecified.yml b/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringNotSpecified.yml index 7f9ecff11d..7cbb947c99 100644 --- a/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringNotSpecified.yml +++ b/newrelic-agent/src/test/resources/com/newrelic/agent/config/newrelicWithBrowserMonitoringNotSpecified.yml @@ -16,6 +16,9 @@ common: &default_settings wait_for_rpm_connect: false + cross_application_tracer: + enabled: true + # activemerchant instrumentation: only report dollar amounts (business data) # if the following is present and true; otherwise, only response times # and throughput to the gateway are reported diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java b/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java index 5c825df608..d5207ee0e0 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/TracedMethod.java @@ -69,7 +69,10 @@ public interface TracedMethod extends AttributeHolder { * @param outboundHeaders The headers that will be written to the output stream for the external request. This also * determines if the external call is HTTP or JMS. * @since 3.36.0 + * @deprecated Instead, use the Distributed Tracing API {@link Transaction#insertDistributedTraceHeaders(Headers)} to create a + * distributed tracing payload */ + @Deprecated void addOutboundRequestHeaders(OutboundHeaders outboundHeaders); } From ca75a4741fd7ced4d083e91b54baeae5e6916402 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 18 Oct 2021 12:42:41 -0700 Subject: [PATCH 31/32] Clean up HarvestServiceImpl --- .../newrelic/agent/HarvestServiceImpl.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java index ae249a1a6c..bcf4b564e8 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/HarvestServiceImpl.java @@ -94,9 +94,11 @@ public void startHarvestables(IRPMService rpmService, AgentConfig config) { if (tracker.harvestable.getAppName().equals(rpmService.getApplicationName())) { int maxSamplesStored = tracker.harvestable.getMaxSamplesStored(); long reportPeriodInMillis = HarvestServiceImpl.REPORTING_PERIOD_IN_MILLISECONDS; + boolean isSpanEventEndpoint = tracker.harvestable.getEndpointMethodName().equals(SPAN_EVENT_DATA); - if (eventHarvestConfig != null) { - Agent.LOG.log(Level.FINE, "event_harvest_config from collector is: {0} for: {1}", maxSamplesStored, + // The event_harvest_config received from server-side during the connect lifecycle contains config for error_event_data, analytic_event_data, and custom_event_data + if (eventHarvestConfig != null && !isSpanEventEndpoint) { + Agent.LOG.log(Level.FINE, "event_harvest_config from collector is: {0} samples stored for {1}", maxSamplesStored, tracker.harvestable.getEndpointMethodName()); Map harvestLimits = (Map) eventHarvestConfig.get(HARVEST_LIMITS); Long harvestLimit = (Long) harvestLimits.get(tracker.harvestable.getEndpointMethodName()); @@ -106,19 +108,25 @@ public void startHarvestables(IRPMService rpmService, AgentConfig config) { ServiceFactory.getStatsService().doStatsWork( StatsWorks.getRecordMetricWork(MetricNames.SUPPORTABILITY_EVENT_HARVEST_REPORT_PERIOD_IN_SECONDS, reportPeriodInMillis / 1000)); } - } else { - Agent.LOG.log(Level.FINE, "event_harvest_config from collector was null. Using default value: {0} for: {1}", maxSamplesStored, + } else if (!isSpanEventEndpoint) { + Agent.LOG.log(Level.FINE, "event_harvest_config from collector was null. Using default value: {0} samples stored for {1}", maxSamplesStored, tracker.harvestable.getEndpointMethodName()); } - if (spanHarvestConfig != null && tracker.harvestable.getEndpointMethodName().equals(SPAN_EVENT_DATA)) { + // The span_event_harvest_config received from server-side during the connect lifecycle contains config for span_event_data + if (spanHarvestConfig != null && isSpanEventEndpoint) { + Agent.LOG.log(Level.FINE, "span_event_harvest_config from collector is: {0} samples stored for {1}", maxSamplesStored, + tracker.harvestable.getEndpointMethodName()); Long harvestLimit = (Long) spanHarvestConfig.get(SERVER_SPAN_HARVEST_LIMIT); - reportPeriodInMillis = (Long) spanHarvestConfig.get(REPORT_PERIOD_MS); if (harvestLimit != null) { maxSamplesStored = harvestLimit.intValue(); reportPeriodInMillis = (long) spanHarvestConfig.get(REPORT_PERIOD_MS); } + } else if (isSpanEventEndpoint) { + Agent.LOG.log(Level.FINE, "span_event_harvest_config from collector was null. Using default value: {0} samples stored for {1}", maxSamplesStored, + tracker.harvestable.getEndpointMethodName()); } + tracker.start(reportPeriodInMillis, maxSamplesStored); } } From d49a02e90e48eef1441877d7ea49669e9638bd25 Mon Sep 17 00:00:00 2001 From: jasonjkeller Date: Mon, 18 Oct 2021 13:59:33 -0700 Subject: [PATCH 32/32] Update default yaml --- newrelic-agent/src/main/resources/newrelic.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/newrelic-agent/src/main/resources/newrelic.yml b/newrelic-agent/src/main/resources/newrelic.yml index a7d6bf2445..8c1be7c8c8 100644 --- a/newrelic-agent/src/main/resources/newrelic.yml +++ b/newrelic-agent/src/main/resources/newrelic.yml @@ -205,8 +205,7 @@ common: &default_settings max_samples_stored: 2000 # Distributed tracing lets you see the path that a request takes through your distributed system. - # This replaces the deprecated legacy Cross Application Tracing. - # https://docs.newrelic.com/docs/apm/transactions/cross-application-traces/introduction-cross-application-traces/ + # This replaces the legacy Cross Application Tracing feature. distributed_tracing: # Set to false to disable distributed tracing.