fix #794 Socket handshake metrics are not recorded by default (#793)

Socket handshake metrics are not recorded by default to prevent new threads for each handshake

Metrics and logging can be enabled by either enabling debug logging
on `com.palantir.tritium.metrics.HandshakeInstrumentation` or
by setting the `instrument.tls.socket` property to `true`.

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

@@ -0,0 +1,10 @@
+type: improvement
+improvement:
+  description: |-
+    Socket handshake metrics are not recorded by default to prevent new threads for each handshake
+
+    Metrics and logging can be enabled by either enabling debug logging
+    on `com.palantir.tritium.metrics.HandshakeInstrumentation` or
+    by setting the `instrument.tls.socket` property to `true`.
+  links:
+  - https://github.com/palantir/tritium/pull/792

tritium-metrics/src/main/java/com/palantir/tritium/metrics/HandshakeInstrumentation.java

@@ -17,6 +17,7 @@
 package com.palantir.tritium.metrics;
 
 import com.palantir.logsafe.SafeArg;
+import com.palantir.tritium.event.InstrumentationProperties;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,5 +45,14 @@ static void record(TlsMetrics metrics, String contextName, String cipherSuite, S
         }
     }
 
+    /**
+     * Socket metrics are more expensive than SSLEngine instrumentation due to HandshakeCompletedListener
+     * instances running on a new short-lived thread. When no HandshakeCompletedListeners are registered,
+     * the thread is completely avoided.
+     */
+    static boolean isSocketInstrumentationEnabled() {
+        return log.isDebugEnabled() || InstrumentationProperties.isSpecificEnabled("tls.socket", false);
+    }
+
     private HandshakeInstrumentation() {}
 }

tritium-metrics/src/main/java/com/palantir/tritium/metrics/InstrumentedSslServerSocketFactory.java

@@ -222,7 +222,7 @@ public Socket accept() throws IOException {
         }
 
         private Socket wrap(Socket socket) {
-            if (socket instanceof SSLSocket && InstrumentedSslSocketFactory.tlsMetricsEnabled.getAsBoolean()) {
+            if (socket instanceof SSLSocket && HandshakeInstrumentation.isSocketInstrumentationEnabled()) {
                 ((SSLSocket) socket).addHandshakeCompletedListener(listener);
             }
             return socket;

tritium-metrics/src/main/java/com/palantir/tritium/metrics/InstrumentedSslSocketFactory.java

@@ -16,20 +16,17 @@
 
 package com.palantir.tritium.metrics;
 
-import com.palantir.tritium.event.InstrumentationProperties;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.Socket;
 import java.util.Objects;
-import java.util.function.BooleanSupplier;
 import javax.net.ssl.HandshakeCompletedListener;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.SSLSocketFactory;
 
 final class InstrumentedSslSocketFactory extends SSLSocketFactory {
 
-    static final BooleanSupplier tlsMetricsEnabled = InstrumentationProperties.getSystemPropertySupplier("tls.socket");
     private final SSLSocketFactory delegate;
     private final HandshakeCompletedListener listener;
     private final String name;
@@ -109,7 +106,7 @@ public int hashCode() {
     }
 
     private Socket wrap(Socket socket) {
-        if (socket instanceof SSLSocket && tlsMetricsEnabled.getAsBoolean()) {
+        if (socket instanceof SSLSocket && HandshakeInstrumentation.isSocketInstrumentationEnabled()) {
             ((SSLSocket) socket).addHandshakeCompletedListener(listener);
         }
         return socket;

tritium-metrics/src/test/java/com/palantir/tritium/metrics/InstrumentedSslContextTest.java

@@ -20,6 +20,7 @@
 import static org.assertj.core.api.Assumptions.assumeThat;
 
 import com.google.common.collect.MoreCollectors;
+import com.palantir.tritium.event.InstrumentationProperties;
 import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
 import com.palantir.tritium.metrics.registry.MetricName;
 import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
@@ -47,6 +48,8 @@
 import okhttp3.Protocol;
 import okhttp3.Request;
 import okhttp3.Response;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.xnio.Options;
 import org.xnio.Sequence;
@@ -59,6 +62,18 @@ final class InstrumentedSslContextTest {
 
     private static final int PORT = 4483;
 
+    @BeforeEach
+    void beforeEach() {
+        System.setProperty("instrument.tls.socket", "true");
+        InstrumentationProperties.reload();
+    }
+
+    @AfterEach
+    void afterEach() {
+        System.clearProperty("instrument.tls.socket");
+        InstrumentationProperties.reload();
+    }
+
     @Test
     void testClientInstrumentationHttpsUrlConnection() throws Exception {
         TaggedMetricRegistry metrics = new DefaultTaggedMetricRegistry();