diff --git a/agent-bridge/src/main/java/com/newrelic/agent/bridge/NoOpSegment.java b/agent-bridge/src/main/java/com/newrelic/agent/bridge/NoOpSegment.java index 7461875eee..9d440f33e0 100644 --- a/agent-bridge/src/main/java/com/newrelic/agent/bridge/NoOpSegment.java +++ b/agent-bridge/src/main/java/com/newrelic/agent/bridge/NoOpSegment.java @@ -10,6 +10,8 @@ import com.newrelic.api.agent.ExternalParameters; import com.newrelic.api.agent.OutboundHeaders; +import java.util.Map; + public class NoOpSegment implements TracedActivity { public static final NoOpSegment INSTANCE = new NoOpSegment(); @@ -67,4 +69,20 @@ public void end(){ public void endAsync() { } + @Override + public void addCustomAttribute(String key, Number value) { + } + + @Override + public void addCustomAttribute(String key, String value) { + } + + @Override + public void addCustomAttribute(String key, boolean value) { + } + + @Override + public void addCustomAttributes(Map attributes) { + } + } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/Segment.java b/newrelic-agent/src/main/java/com/newrelic/agent/Segment.java index 4863d7e832..bbb0d91392 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/Segment.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/Segment.java @@ -7,19 +7,19 @@ package com.newrelic.agent; -import com.newrelic.agent.bridge.NoOpDistributedTracePayload; import com.newrelic.agent.bridge.NoOpTracedMethod; import com.newrelic.agent.bridge.TracedMethod; import com.newrelic.agent.service.ServiceFactory; import com.newrelic.agent.tracers.Tracer; -import com.newrelic.api.agent.DistributedTracePayload; +import com.newrelic.api.agent.AttributeHolder; import com.newrelic.api.agent.ExternalParameters; import com.newrelic.api.agent.OutboundHeaders; import com.newrelic.api.agent.Transaction; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -public class Segment implements com.newrelic.agent.bridge.TracedActivity { +public class Segment implements com.newrelic.agent.bridge.TracedActivity, AttributeHolder { private volatile Tracer underlyingTracer; private volatile Tracer parent; private volatile WeakRefTransaction weakRefTransaction; @@ -149,6 +149,34 @@ public void endAsync() { finish(null, true); } + @Override + public void addCustomAttribute(String key, Number value) { + if (underlyingTracer != null) { + underlyingTracer.addCustomAttribute(key, value); + } + } + + @Override + public void addCustomAttribute(String key, String value) { + if (underlyingTracer != null) { + underlyingTracer.addCustomAttribute(key, value); + } + } + + @Override + public void addCustomAttribute(String key, boolean value) { + if (underlyingTracer != null) { + underlyingTracer.addCustomAttribute(key, value); + } + } + + @Override + public void addCustomAttributes(Map attributes) { + if (underlyingTracer != null) { + underlyingTracer.addCustomAttributes(attributes); + } + } + /** * {@inheritDoc} */ @@ -169,7 +197,9 @@ private void finish(final Throwable t, boolean async) { Runnable expireSegmentRunnable = new Runnable() { @Override public void run() { - tracer.getTransactionActivity().getTransaction().finishSegment(segment, t, parent, endThreadName); + tracer.getTransactionActivity() + .getTransaction() + .finishSegment(segment, t, parent, endThreadName); // Remove references to underlying and parent tracer to prevent GC issues underlyingTracer = null; @@ -195,7 +225,8 @@ public String getInitiatingThread() { public void setTruncated() { Tracer tracer = underlyingTracer; if (tracer != null) { - tracer.setMetricNameFormatInfo(tracer.getMetricName(), "Truncated/" + tracer.getMetricName(), tracer.getTransactionSegmentUri()); + tracer.setMetricNameFormatInfo(tracer.getMetricName(), "Truncated/" + tracer.getMetricName(), + tracer.getTransactionSegmentUri()); } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeValidator.java b/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeValidator.java index c2ecf118a0..13d76d4518 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeValidator.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeValidator.java @@ -16,7 +16,11 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.StandardCharsets; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; @@ -34,7 +38,8 @@ private String getAttributeType() { private boolean isTransactional = true; // Set of APIs that can report attributes outside of a transaction - private static final Collection sendParametersOutsideOfTxn = Arrays.asList("noticeError", "Span.addCustomParameter", "Span.addCustomParameters"); + private static final Collection sendParametersOutsideOfTxn = Arrays.asList("noticeError", + "Span.addCustomParameter", "Span.addCustomParameters", "TracedMethod.addCustomAttributes"); public AttributeValidator(String attributeType) { this.attributeType = attributeType; @@ -133,7 +138,8 @@ protected Map verifyParametersAndReturnValues(Map finishTime = new AtomicReference<>(null); - private final String ATTRIBUTE_API_METHOD_NAME = "TracedMethod addCustomAttributes"; + private final String ATTRIBUTE_API_METHOD_NAME = "TracedMethod.addCustomAttributes"; // Tracers MUST NOT store references to the Transaction. Why: tracers are stored in the TransactionActivity, // and Activities can be reparented from one Transaction to another by the public APIs that support async. diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/SegmentTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/SegmentTest.java index 8191db35fb..a7dff36e5e 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/SegmentTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/SegmentTest.java @@ -72,6 +72,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class SegmentTest implements ExtendedTransactionListener { @@ -187,7 +188,9 @@ public void testEndSameThread() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Sync Segment"); + Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Sync Segment"); Assert.assertNotNull(segment); Thread.sleep(1); @@ -199,7 +202,8 @@ public void testEndSameThread() throws InterruptedException { root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(2, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -216,7 +220,9 @@ public void testAsync() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Async Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Async Segment"); Assert.assertNotNull(segment); Thread t = new Thread() { @Override @@ -243,7 +249,8 @@ public void run() { // assert num children == 2 assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(2, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -336,13 +343,17 @@ public void testAsyncSegmentWithTimeout() throws InterruptedException { */ @Test public void testAsyncWithoutTimeout() throws InterruptedException { - final long configTimeoutMillis = ServiceFactory.getConfigService().getDefaultAgentConfig().getValue("traced_activity_timeout", 10 * 60) * 1000; + final long configTimeoutMillis = + ServiceFactory.getConfigService().getDefaultAgentConfig().getValue("traced_activity_timeout", 10 * 60) * + 1000; final Tracer root = makeTransaction(); Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); final long startTs = System.currentTimeMillis(); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Async Without Timeout"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Async Without Timeout"); Assert.assertNotNull(segment); ServiceFactory.getTransactionService().processQueue(); @@ -353,10 +364,12 @@ public void testAsyncWithoutTimeout() throws InterruptedException { // this will almost always be true since the configTimeout is 3 seconds. if (durationMs < configTimeoutMillis) { // the TA was running less than the timeout when the harvest completed. Should not have been timed out. - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); } else { // the TA was running more than the timeout when the harvest completed. Could have been timed out correctly. - System.err.println("Skipping timeout assert. duration " + durationMs + " exceeds timeout " + configTimeoutMillis); + System.err.println( + "Skipping timeout assert. duration " + durationMs + " exceeds timeout " + configTimeoutMillis); } Assert.assertSame("Segment must be child of root tracer", root, segment.getTracedMethod().getParentTracedMethod()); @@ -409,7 +422,8 @@ public void run() { assertEquals("Segment name not set properly", "Custom/Unnamed Segment", ((Tracer) tracedMethod.get()).getMetricName()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(2, getNumTracers(root.getTransactionActivity().getTransaction())); Assert.assertSame("Segment must be child of root tracer", root, tracedMethod.get().getParentTracedMethod()); @@ -431,7 +445,9 @@ public void testPsuedoAsync1() throws InterruptedException { Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Psuedo Async 1"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Psuedo Async 1"); Assert.assertNotNull(segment); root.finish(Opcodes.ARETURN, null); Thread.sleep(1); @@ -443,7 +459,8 @@ public void testPsuedoAsync1() throws InterruptedException { segment.end(); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(2, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -462,7 +479,9 @@ public void testSyncNamedTracedActivity() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); Assert.assertNotNull(segment); root.finish(Opcodes.ARETURN, null); Thread.sleep(1); @@ -475,7 +494,8 @@ public void testSyncNamedTracedActivity() throws InterruptedException { assertEquals("Segment name was not properly set", "activity", ((Tracer) tracedMethod.get()).getTransactionActivity().getAsyncContext()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(2, getNumTracers(root.getTransactionActivity().getTransaction())); Assert.assertSame("Segment must be child of root tracer", root, tracedMethod.get().getParentTracedMethod()); @@ -498,7 +518,9 @@ public void testPsuedoAsync2() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Psuedo Async2"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Psuedo Async2"); Assert.assertNotNull(segment); ExitTracer child = AgentBridge.instrumentation.createTracer(null, 0, "iamyourchild", DefaultTracer.DEFAULT_TRACER_FLAGS); @@ -514,7 +536,8 @@ public void testPsuedoAsync2() throws InterruptedException { root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(2, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(2, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(3, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -527,10 +550,14 @@ public void testTracedActivityHoldsTransactionOpen() throws InterruptedException Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, Segment.UNNAMED_SEGMENT); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, Segment.UNNAMED_SEGMENT); Assert.assertNotNull(segment); Thread.sleep(1); - final Segment segment2 = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, Segment.UNNAMED_SEGMENT); + final Segment segment2 = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, Segment.UNNAMED_SEGMENT); root.finish(Opcodes.ARETURN, null); Thread.sleep(1); assertFalse(root.getTransactionActivity().getTransaction().isFinished()); @@ -549,7 +576,8 @@ public void testTracedActivityHoldsTransactionOpen() throws InterruptedException Thread.sleep(1); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(3, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(3, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(3, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -563,7 +591,9 @@ public void testCorrectParenting() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); { - final Segment segmentUnderRoot = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Under Root"); + final Segment segmentUnderRoot = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Under Root"); Assert.assertNotNull(segmentUnderRoot); Assert.assertSame("Segment has the wrong parent", root, segmentUnderRoot.getTracedMethod().getParentTracedMethod()); @@ -573,7 +603,9 @@ public void testCorrectParenting() throws InterruptedException { ExitTracer child1 = AgentBridge.instrumentation.createTracer(null, 0, "iamyourchild1", DefaultTracer.DEFAULT_TRACER_FLAGS); { - final Segment underChild1 = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Under Child"); + final Segment underChild1 = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Under Child"); Assert.assertNotNull(underChild1); Assert.assertSame("Segment has the wrong parent", child1, underChild1.getTracedMethod().getParentTracedMethod()); @@ -583,7 +615,9 @@ public void testCorrectParenting() throws InterruptedException { ExitTracer child2 = AgentBridge.instrumentation.createTracer(null, 0, "iamyourchild2", DefaultTracer.DEFAULT_TRACER_FLAGS); { - final Segment underChild2 = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Under Child 2"); + final Segment underChild2 = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Under Child 2"); Assert.assertNotNull(underChild2); Assert.assertSame("Segment has the wrong parent", child2, underChild2.getTracedMethod().getParentTracedMethod()); @@ -598,7 +632,8 @@ public void testCorrectParenting() throws InterruptedException { assertTrue(root.getTransactionActivity().getTransaction().isFinished()); // 4 txas: 1 for each of the 3 segments + 1 tracer - assertEquals(4, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(4, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(6, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -607,19 +642,27 @@ public void testMetricMigration() throws InterruptedException { Tracer root = makeTransaction(); Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); segment.getTracedMethod().addRollupMetricName("rollupMetric"); segment.getTracedMethod().addExclusiveRollupMetricName("exclusiveRollupMetric"); segment.end(); root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - ResponseTimeStats rollupMetric = root.getTransactionActivity().getTransactionStats().getUnscopedStats().getOrCreateResponseTimeStats( - "rollupMetric"); + ResponseTimeStats rollupMetric = root.getTransactionActivity() + .getTransactionStats() + .getUnscopedStats() + .getOrCreateResponseTimeStats( + "rollupMetric"); assertTrue(rollupMetric.getCallCount() == 1); - ResponseTimeStats exclusiveRollupMetric = root.getTransactionActivity().getTransactionStats().getUnscopedStats().getOrCreateResponseTimeStats( - "exclusiveRollupMetric"); + ResponseTimeStats exclusiveRollupMetric = root.getTransactionActivity() + .getTransactionStats() + .getUnscopedStats() + .getOrCreateResponseTimeStats( + "exclusiveRollupMetric"); assertTrue(exclusiveRollupMetric.getCallCount() == 1); } @@ -628,7 +671,9 @@ public void testMetricMigrationAsync() throws InterruptedException { Tracer root = makeTransaction(); Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); segment.getTracedMethod().addRollupMetricName("rollupMetric"); Thread finishThread = new Thread(new Runnable() { @@ -647,12 +692,18 @@ public void run() { root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - ResponseTimeStats rollupMetric = root.getTransactionActivity().getTransactionStats().getUnscopedStats().getOrCreateResponseTimeStats( - "rollupMetric"); + ResponseTimeStats rollupMetric = root.getTransactionActivity() + .getTransactionStats() + .getUnscopedStats() + .getOrCreateResponseTimeStats( + "rollupMetric"); assertEquals(1, rollupMetric.getCallCount()); - ResponseTimeStats exclusiveRollupMetric = root.getTransactionActivity().getTransactionStats().getUnscopedStats().getOrCreateResponseTimeStats( - "rollupMetric2"); + ResponseTimeStats exclusiveRollupMetric = root.getTransactionActivity() + .getTransactionStats() + .getUnscopedStats() + .getOrCreateResponseTimeStats( + "rollupMetric2"); assertEquals(1, exclusiveRollupMetric.getCallCount()); } @@ -662,7 +713,9 @@ public void testExclusiveTime() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); - final com.newrelic.agent.bridge.TracedActivity tracedActivity = AgentBridge.getAgent().getTransaction().createAndStartTracedActivity(); + final com.newrelic.agent.bridge.TracedActivity tracedActivity = AgentBridge.getAgent() + .getTransaction() + .createAndStartTracedActivity(); DefaultTracer underlyingTracer = (DefaultTracer) tracedActivity.getTracedMethod(); Thread.sleep(10); tracedActivity.end(); @@ -682,7 +735,9 @@ public void testExclusiveTimeAsync() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); Thread finishThread = new Thread(new Runnable() { @Override public void run() { @@ -716,14 +771,17 @@ public void testIgnoreTransaction() throws InterruptedException { root.getTransactionActivity().getTransaction().ignore(); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); - Assert.assertNull(segment); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); + assertNull(segment); Thread.sleep(1); root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); //I think reporting a 0 instead of a 1 is fine here - assertEquals(0, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(0, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(1, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -736,14 +794,17 @@ public void testIgnore() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); Assert.assertNotNull(segment); Thread.sleep(1); segment.ignoreIfUnfinished(); root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(1, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(1, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(1, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -757,7 +818,9 @@ public void testIgnoreAsync() throws InterruptedException { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); Thread.sleep(1); - final Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + final Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); root.finish(Opcodes.ARETURN, null); Thread t = new Thread() { @Override @@ -776,7 +839,8 @@ public void run() { t.join(); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - assertEquals(1, root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); + assertEquals(1, + root.getTransactionActivity().getTransaction().getCountOfRunningAndFinishedTransactionActivities()); assertEquals(1, getNumTracers(root.getTransactionActivity().getTransaction())); } @@ -786,7 +850,9 @@ public void testAsyncContextAttributeEndSameThread() { Assert.assertNotNull(root); Assert.assertNotNull(root.getTransactionActivity().getTransaction()); - Segment segment = root.getTransactionActivity().getTransaction().startSegment(MetricNames.CUSTOM, "Custom Segment"); + Segment segment = root.getTransactionActivity() + .getTransaction() + .startSegment(MetricNames.CUSTOM, "Custom Segment"); Assert.assertNotNull(segment); segment.end(); @@ -799,7 +865,7 @@ public void testAsyncContextAttributeEndSameThread() { assertEquals(0, tracers.size()); TransactionActivity segmentTxa = findSegmentTxa(root.getTransactionActivity().getTransaction()); - Assert.assertNull(segmentTxa.getRootTracer().getAgentAttribute("async_context")); + assertNull(segmentTxa.getRootTracer().getAgentAttribute("async_context")); } @Test @@ -851,7 +917,8 @@ private static Map createConfigMap() { return map; } - private static void createServiceManager(Map map, ExpirationService expirationService) throws Exception { + private static void createServiceManager(Map map, ExpirationService expirationService) + throws Exception { ConfigService configService = ConfigServiceFactory.createConfigServiceUsingSettings(map); MockServiceManager serviceManager = new MockServiceManager(configService); ServiceFactory.setServiceManager(serviceManager); @@ -894,8 +961,10 @@ private static void createServiceManager(Map map, ExpirationServ distributedTraceService.connected(null, AgentConfigFactory.createAgentConfig(configMap, null, null)); serviceManager.setDistributedTraceService(distributedTraceService); - TransactionDataToDistributedTraceIntrinsics transactionDataToDistributedTraceIntrinsics = new TransactionDataToDistributedTraceIntrinsics(distributedTraceService); - serviceManager.setTransactionEventsService(new TransactionEventsService(transactionDataToDistributedTraceIntrinsics)); + TransactionDataToDistributedTraceIntrinsics transactionDataToDistributedTraceIntrinsics = new TransactionDataToDistributedTraceIntrinsics( + distributedTraceService); + serviceManager.setTransactionEventsService( + new TransactionEventsService(transactionDataToDistributedTraceIntrinsics)); MockRPMServiceManager rpmServiceManager = new MockRPMServiceManager(); serviceManager.setRPMServiceManager(rpmServiceManager); @@ -963,11 +1032,11 @@ public void confirmTxnGc() { // Please, please, run the gc System.gc(); - Assert.assertNull(seg.getParent()); + assertNull(seg.getParent()); // Need to find if the underlying txn is still alive. com.newrelic.api.agent.Transaction tx = seg.getTransaction(); - Assert.assertNull("Transaction was not garbage collected", tx); + assertNull("Transaction was not garbage collected", tx); } @Test @@ -1031,9 +1100,9 @@ public void testSameThreadSegmentTracerAttributes() throws InterruptedException root.finish(Opcodes.ARETURN, null); assertTrue(root.getTransactionActivity().getTransaction().isFinished()); - Assert.assertNull(segmentTracer.getAgentAttribute(Segment.START_THREAD)); - Assert.assertNull(segmentTracer.getAgentAttribute(Segment.END_THREAD)); - Assert.assertNull(segmentTracer.getAgentAttribute("async_context")); + assertNull(segmentTracer.getAgentAttribute(Segment.START_THREAD)); + assertNull(segmentTracer.getAgentAttribute(Segment.END_THREAD)); + assertNull(segmentTracer.getAgentAttribute("async_context")); } @Test(timeout = 30000) @@ -1140,6 +1209,67 @@ public void setHeader(String name, String value) { assertEquals(segmentSpanEvent.getGuid(), secondRootTracerSpanEvent.getParentId()); } + @Test + public void testSegmentAddCustomAttributeSync() { + Transaction.clearTransaction(); + Tracer rootTracer = makeTransaction(); + Transaction tx = rootTracer.getTransactionActivity().getTransaction(); + Segment segment = tx.startSegment("custom", "segment"); + Tracer tracer = segment.getTracer(); + + segment.addCustomAttribute("redbeans", "rice"); + segment.addCustomAttribute("numBeans", 400); + segment.addCustomAttribute("sausage", true); + segment.addCustomAttribute(null, "Keys cant be null"); + Map extras = new HashMap<>(); + extras.put("pickles", null); + extras.put("hotSauce", true); + segment.addCustomAttributes(extras); + segment.end(); + + assertEquals(4, tracer.getCustomAttributes().size()); + assertEquals("rice", tracer.getCustomAttributes().get("redbeans")); + assertEquals(400, tracer.getCustomAttributes().get("numBeans")); + assertTrue((Boolean) tracer.getCustomAttributes().get("sausage")); + assertTrue((Boolean) tracer.getCustomAttributes().get("hotSauce")); + } + + @Test + public void testSegmentAddCustomAttributeAsync() throws InterruptedException { + Tracer root = makeTransaction(); + final Transaction txn = root.getTransactionActivity().getTransaction(); + final Segment segment = txn.startSegment("custom", "Segment Name"); + final AtomicReference segmentTracerRef = new AtomicReference<>(); + + Thread finishThread = new Thread(new Runnable() { + + @Override + public void run() { + segment.addCustomAttribute("redbeans", "rice"); + segment.addCustomAttribute("numBeans", 400); + segment.addCustomAttribute("sausage", true); + segment.addCustomAttribute(null, "Keys cant be null"); + Map extras = new HashMap<>(); + extras.put("pickles", null); + extras.put("hotSauce", true); + segment.addCustomAttributes(extras); + + Thread.currentThread().setName("Second Thread"); + segmentTracerRef.set(segment.getTracer()); + segment.end(); + } + }); + + finishThread.start(); + finishThread.join(); + Tracer tracer = segmentTracerRef.get(); + assertEquals(4, tracer.getCustomAttributes().size()); + assertEquals("rice", tracer.getCustomAttributes().get("redbeans")); + assertEquals(400, tracer.getCustomAttributes().get("numBeans")); + assertTrue((Boolean) tracer.getCustomAttributes().get("sausage")); + assertTrue((Boolean) tracer.getCustomAttributes().get("hotSauce")); + } + private SpanEvent findSpanByName(List spanEvents, String spanName) { for (SpanEvent spanEvent : spanEvents) { if (spanEvent.getName().equals(spanName)) { diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/AttributeHolder.java b/newrelic-api/src/main/java/com/newrelic/api/agent/AttributeHolder.java new file mode 100644 index 0000000000..d4c0f36006 --- /dev/null +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/AttributeHolder.java @@ -0,0 +1,36 @@ +package com.newrelic.api.agent; + +import java.util.Map; + + +public interface AttributeHolder { + + /** + * Adds/Replaces a numerical attribute. + * + * @since 6.1.0 + */ + void addCustomAttribute(String key, Number value); + + /** + * Adds/Replaces a string attribute. + * + * @since 6.1.0 + */ + void addCustomAttribute(String key, String value); + + /** + * Adds/Replaces a boolean attribute. + * + * @since 6.1.0 + */ + void addCustomAttribute(String key, boolean value); + + /** + * Adds/Replaces key/value pairs. + * + * @since 6.1.0 + */ + void addCustomAttributes(Map attributes); + +} diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/NoOpAgent.java b/newrelic-api/src/main/java/com/newrelic/api/agent/NoOpAgent.java index 65499cbd50..4da1d34fe7 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/NoOpAgent.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/NoOpAgent.java @@ -366,6 +366,22 @@ public void end() { @Override public void endAsync() { } + + @Override + public void addCustomAttribute(String key, Number value) { + } + + @Override + public void addCustomAttribute(String key, String value) { + } + + @Override + public void addCustomAttribute(String key, boolean value) { + } + + @Override + public void addCustomAttributes(Map attributes) { + } }; private static final TraceMetadata TRACE_METADATA = new TraceMetadata() { diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/Segment.java b/newrelic-api/src/main/java/com/newrelic/api/agent/Segment.java index 17dcffdc52..d5f9108fc1 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/Segment.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/Segment.java @@ -23,7 +23,7 @@ * A {@link Segment} will show up in the Transaction Breakdown table, as well as the Transaction Trace page in APM. *

*/ -public interface Segment { +public interface Segment extends AttributeHolder { /** * Sets the metric name by concatenating all given metricNameParts with a '/' separating each part. 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 22f8a7ebd0..5c825df608 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 @@ -7,43 +7,13 @@ package com.newrelic.api.agent; -import java.util.Map; - /** * Represents a single instance of the timing mechanism associated with a method that is instrumented using the * {@link Trace} annotation. * * @see Agent#getTracedMethod() */ -public interface TracedMethod { - - /** - * Adds/Replaces a numerical attribute on the current traced method. - * - * @since 5.13.0 - */ - void addCustomAttribute(String key, Number value); - - /** - * Adds/Replaces a string attribute on the current traced method. - * - * @since 5.13.0 - */ - void addCustomAttribute(String key, String value); - - /** - * Adds/Replaces a boolean attribute on the current traced method. - * - * @since 5.13.0 - */ - void addCustomAttribute(String key, boolean value); - - /** - * Adds/Replaces key/value pairs on the current traced method. - * - * @since 5.13.0 - */ - void addCustomAttributes(Map attributes); +public interface TracedMethod extends AttributeHolder { /** * Returns the traced method metric name.