From 124f7b5636601531913aa2b89f79a3b26663d243 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Mon, 9 Sep 2019 13:26:22 -0400 Subject: [PATCH] Use Byte Buddy for Instrumentation by default (#389) Use Byte Buddy for Instrumentation by default, reducing excess stack frames from instrumentation. --- changelog/@unreleased/pr-389.v2.yml | 6 ++ .../event/InstrumentationProperties.java | 20 ++++- .../event/InstrumentationPropertiesTest.java | 78 +++++++++++++++++++ .../tritium/proxy/Instrumentation.java | 2 +- 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 changelog/@unreleased/pr-389.v2.yml diff --git a/changelog/@unreleased/pr-389.v2.yml b/changelog/@unreleased/pr-389.v2.yml new file mode 100644 index 000000000..c003f5256 --- /dev/null +++ b/changelog/@unreleased/pr-389.v2.yml @@ -0,0 +1,6 @@ +type: feature +feature: + description: Use Byte Buddy for Instrumentation by default, reducing excess stack + frames from instrumentation. + links: + - https://github.com/palantir/tritium/pull/389 diff --git a/tritium-core/src/main/java/com/palantir/tritium/event/InstrumentationProperties.java b/tritium-core/src/main/java/com/palantir/tritium/event/InstrumentationProperties.java index a132a863f..500f0279c 100644 --- a/tritium-core/src/main/java/com/palantir/tritium/event/InstrumentationProperties.java +++ b/tritium-core/src/main/java/com/palantir/tritium/event/InstrumentationProperties.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,8 +47,22 @@ public static BooleanSupplier getSystemPropertySupplier(String name) { @SuppressWarnings("WeakerAccess") // public API public static boolean isSpecificEnabled(String name) { - String qualifiedValue = instrumentationProperties().get(INSTRUMENT_PREFIX + "." + name); - return "true".equalsIgnoreCase(qualifiedValue) || qualifiedValue == null; + return isSpecificEnabled(name, true); + } + + @SuppressWarnings("WeakerAccess") // public API + public static boolean isSpecificEnabled(String name, boolean defaultValue) { + String qualifiedValue = getSpecific(name); + if (qualifiedValue == null) { + return defaultValue; + } + return "true".equalsIgnoreCase(qualifiedValue); + } + + /** Applies the {@link #INSTRUMENT_PREFIX} and returns the current value. */ + @Nullable + private static String getSpecific(String name) { + return instrumentationProperties().get(INSTRUMENT_PREFIX + "." + name); } @SuppressWarnings("WeakerAccess") // public API @@ -100,5 +115,4 @@ private static Map createInstrumentationSystemProperties() { log.debug("Reloaded instrumentation properties {}", map); return map; } - } diff --git a/tritium-core/src/test/java/com/palantir/tritium/event/InstrumentationPropertiesTest.java b/tritium-core/src/test/java/com/palantir/tritium/event/InstrumentationPropertiesTest.java index 0e7158283..e759eca20 100644 --- a/tritium-core/src/test/java/com/palantir/tritium/event/InstrumentationPropertiesTest.java +++ b/tritium-core/src/test/java/com/palantir/tritium/event/InstrumentationPropertiesTest.java @@ -173,4 +173,82 @@ public void onFailure(Throwable throwable) { assertThat(barrier.getNumberWaiting()).isZero(); }); } + + @Test + void testIsSpecificEnabled_notSet() { + assertThat(InstrumentationProperties.isSpecificEnabled("not_set")).isTrue(); + } + + @Test + void testIsSpecificEnabled_notSet_defaultTrue() { + assertThat(InstrumentationProperties.isSpecificEnabled("not_set", true)).isTrue(); + } + + @Test + void testIsSpecificEnabled_notSet_defaultFalse() { + assertThat(InstrumentationProperties.isSpecificEnabled("not_set", false)).isFalse(); + } + + @Test + void testIsSpecificEnabled_setGarbage() { + System.setProperty("instrument.garbage", "garbage"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("garbage")).isFalse(); + } + + @Test + void testIsSpecificEnabled_setGarbage_defaultTrue() { + System.setProperty("instrument.garbage", "garbage"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("garbage", true)).isFalse(); + } + + @Test + void testIsSpecificEnabled_setGarbage_defaultFalse() { + System.setProperty("instrument.garbage", "garbage"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("garbage", false)).isFalse(); + } + + @Test + void testIsSpecificEnabled_setTrue() { + System.setProperty("instrument.true", "true"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("true")).isTrue(); + } + + @Test + void testIsSpecificEnabled_setTrue_defaultTrue() { + System.setProperty("instrument.true", "true"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("true", true)).isTrue(); + } + + @Test + void testIsSpecificEnabled_setTrue_defaultFalse() { + System.setProperty("instrument.true", "true"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("true", false)).isTrue(); + } + + @Test + void testIsSpecificEnabled_setFalse() { + System.setProperty("instrument.false", "false"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("false")).isFalse(); + } + + @Test + void testIsSpecificEnabled_setFalse_defaultTrue() { + System.setProperty("instrument.false", "false"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("false", true)).isFalse(); + } + + @Test + void testIsSpecificEnabled_setFalse_defaultFalse() { + System.setProperty("instrument.false", "false"); + InstrumentationProperties.reload(); + assertThat(InstrumentationProperties.isSpecificEnabled("false", false)).isFalse(); + } } diff --git a/tritium-lib/src/main/java/com/palantir/tritium/proxy/Instrumentation.java b/tritium-lib/src/main/java/com/palantir/tritium/proxy/Instrumentation.java index b008cefaf..848ff2ecc 100644 --- a/tritium-lib/src/main/java/com/palantir/tritium/proxy/Instrumentation.java +++ b/tritium-lib/src/main/java/com/palantir/tritium/proxy/Instrumentation.java @@ -59,7 +59,7 @@ static T wrap(Class interfaceClass, return delegate; } - if (InstrumentationProperties.isSpecificEnabled("dynamic-proxy")) { + if (InstrumentationProperties.isSpecificEnabled("dynamic-proxy", false)) { return Proxies.newProxy(interfaceClass, delegate, new InstrumentationProxy<>(instrumentationFilter, handlers, delegate)); } else {