Merge branch 'develop' into jbaker/faster_strings

.baseline/checkstyle/checkstyle.xml

@@ -469,7 +469,7 @@
         <module name="NestedTryDepth"/> <!-- Java Coding Guide: Try/catch blocks: never nested -->
         <module name="NonEmptyAtclauseDescription"/> <!-- Java Style Guide: At-clauses -->
         <module name="ParameterName"> <!-- Java Style Guide: Parameter names -->
-            <property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
+            <property name="format" value="^_?[a-z][a-zA-Z0-9]+$"/>
             <message key="name.invalidPattern" value="Parameter name ''{0}'' must match pattern ''{1}''."/>
         </module>
         <module name="SingleLineJavadoc"/> <!-- Java Style Guide: General form -->

.bulldozer.yml

@@ -1,3 +1,5 @@
+# Excavator auto-updates this file. Please contribute improvements to the central template.
+
 version: 1
 merge:
   whitelist:

.excavator.yml

@@ -1,6 +1,9 @@
+# Excavator auto-updates this file. Please contribute improvements to the central template.
+
 auto-label:
   names:
     versions-props/upgrade-all: [ "merge when ready" ]
     circleci/manage-circleci: [ "merge when ready" ]
   tags:
     roomba: [ "merge when ready" ]
+    automerge: [ "merge when ready" ]

.policy.yml

@@ -1,12 +1,15 @@
+# Excavator auto-updates this file. Please contribute improvements to the central template.
+
 policy:
   approval:
     - or:
       - one admin has approved (PR contributors not allowed)
       - two admins have approved
       - changelog only and contributor approval
       - fixing excavator
-      - excavator only touched baseline, circle, gradle files, docker-compose-rule config or versions.props
+      - excavator only touched baseline, circle, gradle files, godel files, docker-compose-rule config or versions.props
       - excavator only touched package.json and lock files
+      - excavator only touched config files
   disapproval:
     requires:
       organizations: [ "palantir" ]
@@ -48,7 +51,7 @@ approval_rules:
       has_author_in:
         users: [ "svc-excavator-bot" ]
 
-  - name: excavator only touched baseline, circle, gradle files, docker-compose-rule config or versions.props
+  - name: excavator only touched baseline, circle, gradle files, godel files, docker-compose-rule config or versions.props
     requires:
       count: 0
     if:
@@ -68,9 +71,22 @@ approval_rules:
           - "^gradlew.bat$"
           - "^gradle.properties$"
           - "^settings.gradle$"
+          - "^godelw$"
+          - "^godel/config/godel.properties$"
           - "^versions.props$"
           - "^versions.lock$"
 
+  - name: excavator only touched config files
+    requires:
+      count: 0
+    if:
+      has_author_in:
+        users: [ "svc-excavator-bot" ]
+      only_changed_files:
+        paths:
+          - "^\\..*.yml$"
+          - "^\\.github/.*$"
+
   - name: excavator only touched package.json and lock files
     requires:
       count: 0

build.gradle

@@ -10,10 +10,12 @@ buildscript {
     }
 
     dependencies {
+        classpath 'com.palantir.gradle.revapi:gradle-revapi:0.4.1'
         classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.4'
-        classpath 'com.netflix.nebula:nebula-publishing-plugin:13.3.0'
-        classpath 'com.palantir.baseline:gradle-baseline-java:2.0.0'
-        classpath 'com.palantir.gradle.consistentversions:gradle-consistent-versions:1.12.1'
+        classpath 'com.netflix.nebula:gradle-info-plugin:5.0.3'
+        classpath 'com.netflix.nebula:nebula-publishing-plugin:13.5.1'
+        classpath 'com.palantir.baseline:gradle-baseline-java:2.7.0'
+        classpath 'com.palantir.gradle.consistentversions:gradle-consistent-versions:1.12.4'
         classpath 'com.palantir.gradle.gitversion:gradle-git-version:0.12.2'
         classpath 'gradle.plugin.org.inferred:gradle-processors:3.1.0'
         classpath 'com.google.errorprone:error_prone_core:2.3.3'

changelog/0.15.5/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

changelog/@unreleased/pr-407.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Use simple string concatenation over MetricRegistry.name
+  links:
+  - https://github.com/palantir/tritium/pull/407

gradle/publish-jar.gradle

@@ -7,6 +7,7 @@ apply plugin: 'nebula.maven-nebula-publish'
 apply plugin: 'nebula.maven-base-publish'
 apply plugin: 'nebula.maven-developer'
 apply plugin: 'nebula.maven-manifest'
+apply plugin: 'nebula.info-scm'
 apply plugin: 'nebula.maven-scm'
 
 apply plugin: 'nebula.javadoc-jar'
@@ -47,3 +48,8 @@ publishing {
         }
     }
 }
+
+// Turning off module metadata so that all consumers just use regular POMs
+tasks.withType(GenerateModuleMetadata) {
+    enabled = false
+}

tritium-api/build.gradle

@@ -1,4 +1,5 @@
 apply from: "${rootDir}/gradle/publish-jar.gradle"
+apply plugin: 'com.palantir.revapi'
 
 dependencies {
 

tritium-caffeine/build.gradle

@@ -1,4 +1,5 @@
 apply from: "${rootDir}/gradle/publish-jar.gradle"
+apply plugin: 'com.palantir.revapi'
 
 sourceCompatibility = 1.8
 targetCompatibility = 1.8

tritium-core/build.gradle

@@ -1,4 +1,5 @@
 apply from: "${rootDir}/gradle/publish-jar.gradle"
+apply plugin: 'com.palantir.revapi'
 
 dependencies {
 

tritium-core/src/main/java/com/palantir/tritium/event/InstrumentationFilters.java

@@ -24,14 +24,20 @@ public enum InstrumentationFilters implements InstrumentationFilter {
 
     INSTRUMENT_ALL {
         @Override
-        public boolean shouldInstrument(@Nonnull Object instance, @Nonnull Method method, @Nonnull Object[] args) {
+        public boolean shouldInstrument(
+                @Nonnull Object unusedInstance,
+                @Nonnull Method unusedMethod,
+                @Nonnull Object[] unusedArgs) {
             return true;
         }
     },
 
     INSTRUMENT_NONE {
         @Override
-        public boolean shouldInstrument(@Nonnull Object instance, @Nonnull Method method, @Nonnull Object[] args) {
+        public boolean shouldInstrument(
+                @Nonnull Object unusedInstance,
+                @Nonnull Method unusedMethod,
+                @Nonnull Object[] unusedArgs) {
             return false;
         }
     };

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<String, String> createInstrumentationSystemProperties() {
         log.debug("Reloaded instrumentation properties {}", map);
         return map;
     }
-
 }

tritium-core/src/main/java/com/palantir/tritium/event/NoOpInvocationEventHandler.java

@@ -38,12 +38,12 @@ public InvocationContext preInvocation(@Nonnull Object instance, @Nonnull Method
     }
 
     @Override
-    public void onSuccess(@Nullable InvocationContext context, @Nullable Object result) {
+    public void onSuccess(@Nullable InvocationContext unusedContext, @Nullable Object unusedResult) {
         // no-op
     }
 
     @Override
-    public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause) {
+    public void onFailure(@Nullable InvocationContext unusedContext, @Nonnull Throwable unusedCause) {
         // no-op
     }
 }

tritium-core/src/test/java/com/palantir/tritium/event/CompositeInvocationEventHandlerTest.java

@@ -221,10 +221,10 @@ public InvocationContext preInvocation(
         }
 
         @Override
-        public void onSuccess(@Nullable InvocationContext context, @Nullable Object result) {}
+        public void onSuccess(@Nullable InvocationContext unusedContext, @Nullable Object unusedResult) {}
 
         @Override
-        public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause) {}
+        public void onFailure(@Nullable InvocationContext unusedContext, @Nonnull Throwable unusedCause) {}
     }
 
 }

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();
+    }
 }

tritium-jmh/src/jmh/java/com/palantir/tritium/microbenchmarks/NestedMetricsBenchmark.java

@@ -1,5 +1,9 @@
 /*
+<<<<<<< HEAD
  * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+=======
+ * Copyright 2019 Palantir Technologies, Inc. All rights reserved.
+>>>>>>> develop
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,10 +36,13 @@
 import org.openjdk.jmh.annotations.State;
 import org.openjdk.jmh.annotations.Warmup;
 import org.openjdk.jmh.infra.Blackhole;
+<<<<<<< HEAD
 import org.openjdk.jmh.profile.GCProfiler;
 import org.openjdk.jmh.runner.Runner;
 import org.openjdk.jmh.runner.options.Options;
 import org.openjdk.jmh.runner.options.OptionsBuilder;
+=======
+>>>>>>> develop
 
 @BenchmarkMode(Mode.AverageTime)
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
@@ -53,13 +60,18 @@ public void before() {
     }
 
     @Benchmark
+<<<<<<< HEAD
     public void benchmarkGetMetrics(Blackhole blackhole) {
+=======
+    public void benchmark(Blackhole blackhole) {
+>>>>>>> develop
         metrics.getMetrics().forEach((name, metric) -> {
             blackhole.consume(name);
             blackhole.consume(metric);
         });
     }
 
+<<<<<<< HEAD
     @Benchmark
     public void benchmarkForEach(Blackhole blackhole) {
         metrics.forEachMetric((name, metric) -> {
@@ -68,6 +80,8 @@ public void benchmarkForEach(Blackhole blackhole) {
         });
     }
 
+=======
+>>>>>>> develop
     private static TaggedMetricRegistry constructBaseRegistry() {
         TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry();
         for (int i = 0; i < 50; i++) {
@@ -79,16 +93,21 @@ private static TaggedMetricRegistry constructBaseRegistry() {
     private static TaggedMetricRegistry constructSubRegistry() {
         TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry();
         for (int i = 0; i < 100; i++) {
+<<<<<<< HEAD
             Meter meter = registry.meter(MetricName.builder()
                     .safeName("some metric " + i)
                     .putSafeTags("some tag", "some tag value")
                     .build());
+=======
+            Meter meter = registry.meter(MetricName.builder().safeName("some metric " + i).build());
+>>>>>>> develop
             for (int j = 0; j < 1000; j++) {
                 meter.mark();
             }
         }
         return registry;
     }
+<<<<<<< HEAD
 
     public static void main(String[] args) throws Exception {
         Options opt = new OptionsBuilder()
@@ -97,4 +116,6 @@ public static void main(String[] args) throws Exception {
                 .build();
         new Runner(opt).run();
     }
+=======
+>>>>>>> develop
 }