ref(core): Hold the hostname cache on SentryOptions (JAVA-579)

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Internal
 
 - Measure the hostname cache TTL on a monotonic ticker, so that a device time change no longer shortens or extends it ([#6100](https://github.com/getsentry/sentry-java/pull/6100))
+- Hold the hostname cache on `SentryOptions` instead of a static singleton, so that it measures its TTL on the ticker the options provide ([#6117](https://github.com/getsentry/sentry-java/pull/6117))
 
 ## 8.60.0
 

sentry/api/sentry.api

@@ -694,7 +694,6 @@ public final class io/sentry/Hint {
 
 public final class io/sentry/HostnameCache {
 	public fun getHostname ()Ljava/lang/String;
-	public static fun getInstance ()Lio/sentry/HostnameCache;
 }
 
 public final class io/sentry/HttpBodyType : java/lang/Enum {
@@ -3808,6 +3807,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig {
 	public fun getFlushTimeoutMillis ()J
 	public fun getFullyDisplayedReporter ()Lio/sentry/FullyDisplayedReporter;
 	public fun getGestureTargetLocators ()Ljava/util/List;
+	public fun getHostnameCache ()Lio/sentry/HostnameCache;
 	public fun getIdleTimeout ()Ljava/lang/Long;
 	public fun getIgnoredCheckIns ()Ljava/util/List;
 	public fun getIgnoredErrors ()Ljava/util/List;

sentry/src/main/java/io/sentry/HostnameCache.java

@@ -1,9 +1,7 @@
 package io.sentry;
 
 import io.sentry.time.Deadline;
-import io.sentry.time.JavaMonotonicTicker;
 import io.sentry.time.MonotonicTicker;
-import io.sentry.util.AutoClosableReentrantLock;
 import io.sentry.util.Objects;
 import java.net.InetAddress;
 import java.util.concurrent.Callable;
@@ -29,8 +27,8 @@
  * performance purposes, the operation of retrieving the hostname will automatically fail after a
  * period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
  *
- * <p>HostnameCache is a singleton and its instance should be obtained through {@link
- * HostnameCache#getInstance()}.
+ * <p>One instance is held per {@link SentryOptions} and should be obtained through {@link
+ * SentryOptions#getHostnameCache()}.
  */
 @ApiStatus.Internal
 public final class HostnameCache {
@@ -42,10 +40,6 @@ public final class HostnameCache {
   /** How long the worker thread may stay idle before it self-terminates. */
   private static final long THREAD_KEEP_ALIVE_SECONDS = 30;
 
-  private static volatile @Nullable HostnameCache INSTANCE;
-  private static final @NotNull AutoClosableReentrantLock staticLock =
-      new AutoClosableReentrantLock();
-
   private final @NotNull MonotonicTicker ticker;
 
   /** Current value for hostname (might change over time). */
@@ -61,22 +55,16 @@ public final class HostnameCache {
 
   private final @NotNull ExecutorService executorService;
 
-  public static @NotNull HostnameCache getInstance() {
-    if (INSTANCE == null) {
-      try (final @NotNull ISentryLifecycleToken ignored = staticLock.acquire()) {
-        if (INSTANCE == null) {
-          INSTANCE = new HostnameCache();
-        }
-      }
-    }
-
-    return INSTANCE;
-  }
-
-  private HostnameCache() {
+  /**
+   * Names the only collaborator a hostname cache reads, rather than taking the whole {@link
+   * SentryOptions}.
+   *
+   * @param ticker the ticker the cache lifetime is measured on
+   */
+  HostnameCache(final @NotNull MonotonicTicker ticker) {
     // avoid method refs on Android due to some issues with older AGP setups
     // noinspection Convert2MethodRef
-    this(() -> InetAddress.getLocalHost(), JavaMonotonicTicker.getInstance());
+    this(() -> InetAddress.getLocalHost(), ticker);
   }
 
   /**

sentry/src/main/java/io/sentry/MainEventProcessor.java

@@ -21,7 +21,6 @@ public final class MainEventProcessor implements EventProcessor {
   private final @NotNull SentryOptions options;
   private final @NotNull SentryThreadFactory sentryThreadFactory;
   private final @NotNull SentryExceptionFactory sentryExceptionFactory;
-  private volatile @Nullable HostnameCache hostnameCache = null;
 
   public MainEventProcessor(final @NotNull SentryOptions options) {
     this.options = options;
@@ -160,16 +159,7 @@ private void setServerName(final @NotNull SentryBaseEvent event) {
     }
 
     if (options.isAttachServerName() && event.getServerName() == null) {
-      ensureHostnameCache();
-      if (hostnameCache != null) {
-        event.setServerName(hostnameCache.getHostname());
-      }
-    }
-  }
-
-  private void ensureHostnameCache() {
-    if (hostnameCache == null) {
-      hostnameCache = HostnameCache.getInstance();
+      event.setServerName(options.getHostnameCache().getHostname());
     }
   }
 

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -535,6 +535,17 @@ public class SentryOptions implements RateLimiterConfig {
   private final @NotNull LazyEvaluator<SentryDateProvider> dateProvider =
       new LazyEvaluator<>(() -> new SentryAutoDateProvider());
 
+  /**
+   * Cache of the local hostname, used when {@link #isAttachServerName()} is enabled.
+   *
+   * <p>Evaluated lazily because resolving the hostname blocks on {@code
+   * InetAddress.getLocalHost()}, which no {@code Sentry.init} should pay for up front. Deferring
+   * also means {@link #getMonotonicTicker()} is read after subclasses have overridden it.
+   */
+  @ApiStatus.Internal
+  private final @NotNull LazyEvaluator<HostnameCache> hostnameCache =
+      new LazyEvaluator<>(() -> new HostnameCache(getMonotonicTicker()));
+
   private final @NotNull List<IPerformanceCollector> performanceCollectors = new ArrayList<>();
 
   /** Performance collector that collect performance stats while transactions run. */
@@ -3140,6 +3151,12 @@ public void setDateProvider(final @NotNull SentryDateProvider dateProvider) {
     return JavaMonotonicTicker.getInstance();
   }
 
+  /** Returns the hostname cache, resolving the hostname on first use. */
+  @ApiStatus.Internal
+  public @NotNull HostnameCache getHostnameCache() {
+    return hostnameCache.getValue();
+  }
+
   /**
    * Adds a ICollector.
    *

sentry/src/main/java/io/sentry/logger/LoggerApi.java

@@ -1,6 +1,5 @@
 package io.sentry.logger;
 
-import io.sentry.HostnameCache;
 import io.sentry.IScope;
 import io.sentry.ISpan;
 import io.sentry.PropagationContext;
@@ -256,7 +255,7 @@ private void setServerName(
           "server.address",
           new SentryLogEventAttributeValue(SentryAttributeType.STRING, optionsServerName));
     } else if (options.isAttachServerName()) {
-      final @Nullable String hostname = HostnameCache.getInstance().getHostname();
+      final @Nullable String hostname = options.getHostnameCache().getHostname();
       if (hostname != null) {
         attributes.put(
             "server.address",

sentry/src/main/java/io/sentry/metrics/MetricsApi.java

@@ -1,6 +1,5 @@
 package io.sentry.metrics;
 
-import io.sentry.HostnameCache;
 import io.sentry.IScope;
 import io.sentry.ISpan;
 import io.sentry.PropagationContext;
@@ -241,7 +240,7 @@ private void setServerName(
           "server.address",
           new SentryLogEventAttributeValue(SentryAttributeType.STRING, optionsServerName));
     } else if (options.isAttachServerName()) {
-      final @Nullable String hostname = HostnameCache.getInstance().getHostname();
+      final @Nullable String hostname = options.getHostnameCache().getHostname();
       if (hostname != null) {
         attributes.put(
             "server.address",

sentry/src/test/java/io/sentry/MainEventProcessorTest.kt

@@ -12,15 +12,13 @@ import io.sentry.util.HintUtils
 import java.lang.RuntimeException
 import java.net.InetAddress
 import java.util.concurrent.TimeUnit
-import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertSame
 import kotlin.test.assertTrue
-import org.mockito.Mockito
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.reset
 import org.mockito.kotlin.times
@@ -29,18 +27,24 @@ import org.mockito.kotlin.whenever
 
 class MainEventProcessorTest {
   class Fixture {
-    val sentryOptions: SentryOptions =
-      SentryOptions().apply {
-        dsn = dsnString
-        release = "release"
-        dist = "dist"
-        sdkVersion = SdkVersion("test", "1.2.3")
-      }
     val scopes = mock<IScopes>()
     val getLocalhost = mock<InetAddress>()
     val hostnameCacheTicker = TestMonotonicTicker()
+    // Built in getSut() rather than here: the constructor resolves the hostname straight away,
+    // so it has to run after getLocalhost is stubbed.
+    lateinit var hostnameCache: HostnameCache
+    val sentryOptions: SentryOptions =
+      object : SentryOptions() {
+          // Qualified: an unqualified name here would resolve to this override, not the field.
+          override fun getHostnameCache(): HostnameCache = this@Fixture.hostnameCache
+        }
+        .apply {
+          dsn = dsnString
+          release = "release"
+          dist = "dist"
+          sdkVersion = SdkVersion("test", "1.2.3")
+        }
     lateinit var sentryTracer: SentryTracer
-    private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)
 
     fun getSut(
       attachThreads: Boolean = true,
@@ -77,21 +81,10 @@ class MainEventProcessorTest {
       }
       whenever(scopes.options).thenReturn(sentryOptions)
       sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
-
-      val hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker)
-      hostnameCacheMock.`when`<Any> { HostnameCache.getInstance() }.thenReturn(hostnameCache)
+      hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker)
 
       return MainEventProcessor(sentryOptions)
     }
-
-    fun teardown() {
-      hostnameCacheMock.close()
-    }
-  }
-
-  @AfterTest
-  fun teardown() {
-    fixture.teardown()
   }
 
   private val fixture = Fixture()

sentry/src/test/java/io/sentry/SentryOptionsTest.kt

@@ -3,6 +3,10 @@ package io.sentry
 import com.google.common.truth.Truth.assertThat
 import io.sentry.SentryOptions.RequestSize
 import io.sentry.logger.ILoggerBatchProcessorFactory
+import io.sentry.test.getProperty
+import io.sentry.time.MonotonicTicker
+import io.sentry.time.TestMonotonicTicker
+import io.sentry.util.LazyEvaluator
 import io.sentry.util.StringUtils
 import java.io.File
 import java.net.Proxy
@@ -1331,4 +1335,28 @@ class SentryOptionsTest {
     options.scopesStorageFactory = null
     assertNull(options.scopesStorageFactory)
   }
+
+  private fun SentryOptions.peekHostnameCache(): HostnameCache? =
+    getProperty<LazyEvaluator<HostnameCache>>("hostnameCache").getProperty("value")
+
+  /** Options whose hostname cache resolves against a ticker the test controls. */
+  private fun optionsWithTicker(ticker: MonotonicTicker): SentryOptions =
+    object : SentryOptions() {
+      override fun getMonotonicTicker(): MonotonicTicker = ticker
+    }
+
+  @Test
+  fun `hostname is resolved on first use, not when options are constructed`() {
+    val ticker = TestMonotonicTicker()
+    val options = optionsWithTicker(ticker)
+
+    assertThat(options.peekHostnameCache()).isNull()
+
+    val cache = options.hostnameCache
+
+    // Also keeps the assertion above honest: peek does report a cache once one exists.
+    assertThat(options.peekHostnameCache()).isSameInstanceAs(cache)
+    assertThat(options.hostnameCache).isSameInstanceAs(cache)
+    assertThat(cache.getProperty<MonotonicTicker>("ticker")).isSameInstanceAs(ticker)
+  }
 }