From 8e5bb70a5812e0ec96f275c403e9baf497c84d2d Mon Sep 17 00:00:00 2001 From: Alexey Veklov Date: Sun, 7 Mar 2021 12:09:27 +0300 Subject: [PATCH 1/2] Fix of transaction propagation for empty mono/flux and retry operators --- .../reactor/netty/TokenLinkingSubscriber.java | 6 +- .../TransactionPropagationTest.java | 254 ++++++++++++++++++ .../reactor/netty/TokenLinkingSubscriber.java | 6 +- .../TransactionPropagationTest.java | 254 ++++++++++++++++++ 4 files changed, 516 insertions(+), 4 deletions(-) create mode 100644 instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java create mode 100644 instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java diff --git a/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java b/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java index b178c459c0..0d5905d00f 100644 --- a/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java +++ b/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java @@ -48,7 +48,7 @@ public void onError(Throwable throwable) { @Override public void onComplete() { - subscriber.onComplete(); + withNRToken(subscriber::onComplete); } @Override @@ -67,8 +67,10 @@ private void withNRToken(Runnable runnable) { @Trace(async = true, excludeFromTransactionTrace = true) private void withNRError(Runnable runnable, Throwable throwable) { if (token != null && token.isActive()) { - token.linkAndExpire(); + token.link(); if (NettyReactorConfig.errorsEnabled) { + // I believe 100% of users disable this as it makes NewRelic to report caught and handled + // exceptions as errors in APM. Is there a real use case for this? NewRelic.noticeError(throwable); } } diff --git a/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java b/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java new file mode 100644 index 0000000000..df6ca82757 --- /dev/null +++ b/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java @@ -0,0 +1,254 @@ +package com.nr.agent.instrumentation; + +import com.newrelic.agent.DebugFlag; +import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.agent.bridge.Token; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; +import com.newrelic.agent.introspec.Introspector; +import com.newrelic.api.agent.Trace; +import com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Hooks; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; +import reactor.util.context.Context; + +import java.time.Duration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; + +import static com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber.tokenLift; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("deprecation") +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = {"reactor"}) +public class TransactionPropagationTest { + + public static final String A_VALUE = ""; + + @BeforeClass + public static void init() { + Hooks.onEachOperator(TokenLinkingSubscriber.class.getName(), tokenLift()); + DebugFlag.tokenEnabled.set(true); + } + + @Test + @Ignore("A test harness check") + public void testSync() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> + checkTransaction(hadTransaction)); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A test harness check") + public void testAsync() throws InterruptedException { + AtomicBoolean hadTransaction = new AtomicBoolean(); + CountDownLatch done = new CountDownLatch(1); + inTransaction(() -> { + Token token = createToken(); + inAnotherThread(() -> + inAnnotatedWithTraceAsync(() -> { + token.linkAndExpire(); + checkTransaction(hadTransaction); + done.countDown(); + }) + ); + }); + done.await(); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testAsyncSchedulers() throws InterruptedException { + AtomicBoolean hadTransaction = new AtomicBoolean(); + CountDownLatch done = new CountDownLatch(1); + inTransaction(() -> { + Token token = createToken(); + Schedulers.elastic().schedule(() -> { +// trace_async(() -> { it is not need as Tasks are instrumented and annotated with @Trace(async = ture) + token.linkAndExpire(); + checkTransaction(hadTransaction); + done.countDown(); +// }); + }); + }); + done.await(); + assertCapturedData(hadTransaction); + } + + @Test + public void testEmptyMonoOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.empty() + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testEmptyFluxOnComplete() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Flux.empty() + .subscribeOn(Schedulers.elastic()) + .doOnComplete(() -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .blockFirst(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testMonoOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testMonoRetryOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + AtomicBoolean firstCall = new AtomicBoolean(true); + Mono + .create(monoSink -> + inAnotherThread(() -> { + if (firstCall.getAndSet(false)) + monoSink.error(new RuntimeException("failing the first call")); + else + monoSink.success(A_VALUE); + }) + ) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .retry(2) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testMonoRetryBackoffOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + AtomicBoolean firstCall = new AtomicBoolean(true); + Mono + .create(monoSink -> + inAnotherThread(() -> { + if (firstCall.getAndSet(false)) + monoSink.error(new RuntimeException("failing the first call")); + else + monoSink.success(A_VALUE); + }) + ) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .retryBackoff(2, Duration.ZERO) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testMonoNestedInFlatMap() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .flatMap(v -> + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v2 -> + checkTransaction(hadTransaction))) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Trace(dispatcher = true) + public void inTransaction(Runnable actions) { + actions.run(); + } + + public void inAnotherThread(Runnable runnable) { + new Thread(runnable).start(); + } + + @Trace(async = true) + public void inAnnotatedWithTraceAsync(Runnable runnable) { + runnable.run(); + } + + public Token createToken() { + return AgentBridge.getAgent().getTransaction(false).getToken(); + } + + public Context with(Token token) { + return Context.empty().put("newrelic-token", token); + } + + @Trace + public void checkTransaction(AtomicBoolean hadTransaction) { + hadTransaction.set(AgentBridge.getAgent().getTransaction(false) != null); + } + + private void assertCapturedData(AtomicBoolean hadTransaction) { + assertTrue("Did not have transaction", hadTransaction.get()); + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertThat("No finished transactions", introspector.getFinishedTransactionCount(), + is(greaterThan(0))); + + assertThat("Transaction names", introspector.getTransactionNames(), contains( + "OtherTransaction/Custom/" + getClass().getName() + "/inTransaction" + )); + + assertThat("Unscoped metrics", introspector.getUnscopedMetrics().keySet(), hasItems( + "Java/" + getClass().getName() + "/inTransaction", + "Custom/" + getClass().getName() + "/checkTransaction" + )); + } +} diff --git a/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java b/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java index b178c459c0..0d5905d00f 100644 --- a/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java +++ b/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java @@ -48,7 +48,7 @@ public void onError(Throwable throwable) { @Override public void onComplete() { - subscriber.onComplete(); + withNRToken(subscriber::onComplete); } @Override @@ -67,8 +67,10 @@ private void withNRToken(Runnable runnable) { @Trace(async = true, excludeFromTransactionTrace = true) private void withNRError(Runnable runnable, Throwable throwable) { if (token != null && token.isActive()) { - token.linkAndExpire(); + token.link(); if (NettyReactorConfig.errorsEnabled) { + // I believe 100% of users disable this as it makes NewRelic to report caught and handled + // exceptions as errors in APM. Is there a real use case for this? NewRelic.noticeError(throwable); } } diff --git a/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java b/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java new file mode 100644 index 0000000000..df6ca82757 --- /dev/null +++ b/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java @@ -0,0 +1,254 @@ +package com.nr.agent.instrumentation; + +import com.newrelic.agent.DebugFlag; +import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.agent.bridge.Token; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; +import com.newrelic.agent.introspec.Introspector; +import com.newrelic.api.agent.Trace; +import com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Hooks; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; +import reactor.util.context.Context; + +import java.time.Duration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; + +import static com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber.tokenLift; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("deprecation") +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = {"reactor"}) +public class TransactionPropagationTest { + + public static final String A_VALUE = ""; + + @BeforeClass + public static void init() { + Hooks.onEachOperator(TokenLinkingSubscriber.class.getName(), tokenLift()); + DebugFlag.tokenEnabled.set(true); + } + + @Test + @Ignore("A test harness check") + public void testSync() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> + checkTransaction(hadTransaction)); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A test harness check") + public void testAsync() throws InterruptedException { + AtomicBoolean hadTransaction = new AtomicBoolean(); + CountDownLatch done = new CountDownLatch(1); + inTransaction(() -> { + Token token = createToken(); + inAnotherThread(() -> + inAnnotatedWithTraceAsync(() -> { + token.linkAndExpire(); + checkTransaction(hadTransaction); + done.countDown(); + }) + ); + }); + done.await(); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testAsyncSchedulers() throws InterruptedException { + AtomicBoolean hadTransaction = new AtomicBoolean(); + CountDownLatch done = new CountDownLatch(1); + inTransaction(() -> { + Token token = createToken(); + Schedulers.elastic().schedule(() -> { +// trace_async(() -> { it is not need as Tasks are instrumented and annotated with @Trace(async = ture) + token.linkAndExpire(); + checkTransaction(hadTransaction); + done.countDown(); +// }); + }); + }); + done.await(); + assertCapturedData(hadTransaction); + } + + @Test + public void testEmptyMonoOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.empty() + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testEmptyFluxOnComplete() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Flux.empty() + .subscribeOn(Schedulers.elastic()) + .doOnComplete(() -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .blockFirst(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testMonoOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testMonoRetryOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + AtomicBoolean firstCall = new AtomicBoolean(true); + Mono + .create(monoSink -> + inAnotherThread(() -> { + if (firstCall.getAndSet(false)) + monoSink.error(new RuntimeException("failing the first call")); + else + monoSink.success(A_VALUE); + }) + ) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .retry(2) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + public void testMonoRetryBackoffOnSuccess() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + AtomicBoolean firstCall = new AtomicBoolean(true); + Mono + .create(monoSink -> + inAnotherThread(() -> { + if (firstCall.getAndSet(false)) + monoSink.error(new RuntimeException("failing the first call")); + else + monoSink.success(A_VALUE); + }) + ) + .doOnSuccess(v -> + checkTransaction(hadTransaction)) + .retryBackoff(2, Duration.ZERO) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Test + @Ignore("A sanity check") + public void testMonoNestedInFlatMap() { + AtomicBoolean hadTransaction = new AtomicBoolean(); + inTransaction(() -> { + Token token = createToken(); + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .flatMap(v -> + Mono.just(A_VALUE) + .subscribeOn(Schedulers.elastic()) + .doOnSuccess(v2 -> + checkTransaction(hadTransaction))) + .subscriberContext(with(token)) + .block(); + token.expire(); + }); + assertCapturedData(hadTransaction); + } + + @Trace(dispatcher = true) + public void inTransaction(Runnable actions) { + actions.run(); + } + + public void inAnotherThread(Runnable runnable) { + new Thread(runnable).start(); + } + + @Trace(async = true) + public void inAnnotatedWithTraceAsync(Runnable runnable) { + runnable.run(); + } + + public Token createToken() { + return AgentBridge.getAgent().getTransaction(false).getToken(); + } + + public Context with(Token token) { + return Context.empty().put("newrelic-token", token); + } + + @Trace + public void checkTransaction(AtomicBoolean hadTransaction) { + hadTransaction.set(AgentBridge.getAgent().getTransaction(false) != null); + } + + private void assertCapturedData(AtomicBoolean hadTransaction) { + assertTrue("Did not have transaction", hadTransaction.get()); + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertThat("No finished transactions", introspector.getFinishedTransactionCount(), + is(greaterThan(0))); + + assertThat("Transaction names", introspector.getTransactionNames(), contains( + "OtherTransaction/Custom/" + getClass().getName() + "/inTransaction" + )); + + assertThat("Unscoped metrics", introspector.getUnscopedMetrics().keySet(), hasItems( + "Java/" + getClass().getName() + "/inTransaction", + "Custom/" + getClass().getName() + "/checkTransaction" + )); + } +} From 5c32061164de3e08f60081e43e6175bfe3166a0f Mon Sep 17 00:00:00 2001 From: Alexey Veklov Date: Thu, 11 Mar 2021 17:30:28 +0300 Subject: [PATCH 2/2] Minor reactor-netty tests cleanup --- .../reactor/netty/TokenLinkingSubscriber.java | 2 -- .../TransactionPropagationTest.java | 14 +++----------- .../reactor/netty/TokenLinkingSubscriber.java | 2 -- .../TransactionPropagationTest.java | 14 +++----------- 4 files changed, 6 insertions(+), 26 deletions(-) diff --git a/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java b/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java index 0d5905d00f..04595191dd 100644 --- a/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java +++ b/instrumentation/netty-reactor-0.8.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java @@ -69,8 +69,6 @@ private void withNRError(Runnable runnable, Throwable throwable) { if (token != null && token.isActive()) { token.link(); if (NettyReactorConfig.errorsEnabled) { - // I believe 100% of users disable this as it makes NewRelic to report caught and handled - // exceptions as errors in APM. Is there a real use case for this? NewRelic.noticeError(throwable); } } diff --git a/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java b/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java index df6ca82757..14d498ebd2 100644 --- a/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java +++ b/instrumentation/netty-reactor-0.8.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java @@ -1,6 +1,5 @@ package com.nr.agent.instrumentation; -import com.newrelic.agent.DebugFlag; import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.Token; import com.newrelic.agent.introspec.InstrumentationTestConfig; @@ -9,7 +8,6 @@ import com.newrelic.api.agent.Trace; import com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import reactor.core.publisher.Flux; @@ -38,12 +36,10 @@ public class TransactionPropagationTest { @BeforeClass public static void init() { Hooks.onEachOperator(TokenLinkingSubscriber.class.getName(), tokenLift()); - DebugFlag.tokenEnabled.set(true); } @Test - @Ignore("A test harness check") - public void testSync() { + public void syncPropagationSanityCheck() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> checkTransaction(hadTransaction)); @@ -51,8 +47,7 @@ public void testSync() { } @Test - @Ignore("A test harness check") - public void testAsync() throws InterruptedException { + public void asyncPropagationSanityCheck() throws InterruptedException { AtomicBoolean hadTransaction = new AtomicBoolean(); CountDownLatch done = new CountDownLatch(1); inTransaction(() -> { @@ -70,8 +65,7 @@ public void testAsync() throws InterruptedException { } @Test - @Ignore("A sanity check") - public void testAsyncSchedulers() throws InterruptedException { + public void testReactorSchedulersInstrumentation() throws InterruptedException { AtomicBoolean hadTransaction = new AtomicBoolean(); CountDownLatch done = new CountDownLatch(1); inTransaction(() -> { @@ -121,7 +115,6 @@ public void testEmptyFluxOnComplete() { } @Test - @Ignore("A sanity check") public void testMonoOnSuccess() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> { @@ -188,7 +181,6 @@ public void testMonoRetryBackoffOnSuccess() { } @Test - @Ignore("A sanity check") public void testMonoNestedInFlatMap() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> { diff --git a/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java b/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java index 0d5905d00f..04595191dd 100644 --- a/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java +++ b/instrumentation/netty-reactor-0.9.0/src/main/java/com/nr/instrumentation/reactor/netty/TokenLinkingSubscriber.java @@ -69,8 +69,6 @@ private void withNRError(Runnable runnable, Throwable throwable) { if (token != null && token.isActive()) { token.link(); if (NettyReactorConfig.errorsEnabled) { - // I believe 100% of users disable this as it makes NewRelic to report caught and handled - // exceptions as errors in APM. Is there a real use case for this? NewRelic.noticeError(throwable); } } diff --git a/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java b/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java index df6ca82757..14d498ebd2 100644 --- a/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java +++ b/instrumentation/netty-reactor-0.9.0/src/test/java/com/nr/agent/instrumentation/TransactionPropagationTest.java @@ -1,6 +1,5 @@ package com.nr.agent.instrumentation; -import com.newrelic.agent.DebugFlag; import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.agent.bridge.Token; import com.newrelic.agent.introspec.InstrumentationTestConfig; @@ -9,7 +8,6 @@ import com.newrelic.api.agent.Trace; import com.nr.instrumentation.reactor.netty.TokenLinkingSubscriber; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import reactor.core.publisher.Flux; @@ -38,12 +36,10 @@ public class TransactionPropagationTest { @BeforeClass public static void init() { Hooks.onEachOperator(TokenLinkingSubscriber.class.getName(), tokenLift()); - DebugFlag.tokenEnabled.set(true); } @Test - @Ignore("A test harness check") - public void testSync() { + public void syncPropagationSanityCheck() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> checkTransaction(hadTransaction)); @@ -51,8 +47,7 @@ public void testSync() { } @Test - @Ignore("A test harness check") - public void testAsync() throws InterruptedException { + public void asyncPropagationSanityCheck() throws InterruptedException { AtomicBoolean hadTransaction = new AtomicBoolean(); CountDownLatch done = new CountDownLatch(1); inTransaction(() -> { @@ -70,8 +65,7 @@ public void testAsync() throws InterruptedException { } @Test - @Ignore("A sanity check") - public void testAsyncSchedulers() throws InterruptedException { + public void testReactorSchedulersInstrumentation() throws InterruptedException { AtomicBoolean hadTransaction = new AtomicBoolean(); CountDownLatch done = new CountDownLatch(1); inTransaction(() -> { @@ -121,7 +115,6 @@ public void testEmptyFluxOnComplete() { } @Test - @Ignore("A sanity check") public void testMonoOnSuccess() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> { @@ -188,7 +181,6 @@ public void testMonoRetryBackoffOnSuccess() { } @Test - @Ignore("A sanity check") public void testMonoNestedInFlatMap() { AtomicBoolean hadTransaction = new AtomicBoolean(); inTransaction(() -> {