diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a8eea2d9c..7e0d8bd82d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,7 @@
### Internal
- Deprecate `AndroidCurrentDateProvider.getInstance()` in favor of `MonotonicTicker`, which counts time spent in deep sleep and cannot be confused with the epoch-based `CurrentDateProvider` ([#6103](https://github.com/getsentry/sentry-java/pull/6103))
+- 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))
## 8.56.0
diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java
index 56cc0c2e84..a5ae15258c 100644
--- a/sentry/src/main/java/io/sentry/HostnameCache.java
+++ b/sentry/src/main/java/io/sentry/HostnameCache.java
@@ -1,5 +1,8 @@
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;
@@ -21,9 +24,9 @@
* Time sensitive cache in charge of keeping track of the hostname. The {@code
* InetAddress.getLocalHost().getCanonicalHostName()} call can be quite expensive and could be
* called for the creation of each {@link SentryEvent}. This system will prevent unnecessary costs
- * by keeping track of the hostname for a period defined during the construction. For performance
- * purposes, the operation of retrieving the hostname will automatically fail after a period of time
- * defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
+ * by keeping track of the hostname for a period defined by {@link #HOSTNAME_CACHE_DURATION}. For
+ * performance purposes, the operation of retrieving the hostname will automatically fail after a
+ * period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
*
*
HostnameCache is a singleton and its instance should be obtained through {@link
* HostnameCache#getInstance()}.
@@ -42,14 +45,13 @@ public final class HostnameCache {
private static final @NotNull AutoClosableReentrantLock staticLock =
new AutoClosableReentrantLock();
- /** Time for which the cache is kept. */
- private final long cacheDuration;
+ private final @NotNull MonotonicTicker ticker;
/** Current value for hostname (might change over time). */
@Nullable private volatile String hostname;
- /** Time at which the cache should expire. */
- private volatile long expirationTimestamp;
+ /** When the cached hostname goes stale. */
+ private volatile @NotNull Deadline cacheFreshUntil;
/** Whether a cache update thread is currently running or not. */
private final @NotNull AtomicBoolean updateRunning = new AtomicBoolean(false);
@@ -71,25 +73,25 @@ public final class HostnameCache {
}
private HostnameCache() {
- this(HOSTNAME_CACHE_DURATION);
- }
-
- HostnameCache(long cacheDuration) {
// avoid method refs on Android due to some issues with older AGP setups
// noinspection Convert2MethodRef
- this(cacheDuration, () -> InetAddress.getLocalHost());
+ this(() -> InetAddress.getLocalHost(), JavaMonotonicTicker.getInstance());
}
/**
* Sets up a cache for the hostname.
*
- * @param cacheDuration cache duration in milliseconds.
* @param getLocalhost a callback to obtain the localhost address - this is mostly here because of
* testability
+ * @param ticker the ticker the cache lifetime is measured on
*/
- HostnameCache(long cacheDuration, final @NotNull Callable getLocalhost) {
- this.cacheDuration = cacheDuration;
+ HostnameCache(
+ final @NotNull Callable getLocalhost, final @NotNull MonotonicTicker ticker) {
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
+ this.ticker = Objects.requireNonNull(ticker, "ticker is required");
+ // Nothing resolved yet, so the cache is stale rather than fresh until updateCache says
+ // otherwise.
+ this.cacheFreshUntil = Deadline.passed(ticker);
// A single thread executor whose worker thread times out while idle, so no thread is kept
// alive between the infrequent cache refreshes.
final @NotNull ThreadPoolExecutor executor =
@@ -122,8 +124,7 @@ boolean isClosed() {
*/
@Nullable
public String getHostname() {
- if (expirationTimestamp < System.currentTimeMillis()
- && updateRunning.compareAndSet(false, true)) {
+ if (cacheFreshUntil.hasPassed() && updateRunning.compareAndSet(false, true)) {
updateCache();
}
@@ -136,7 +137,8 @@ private void updateCache() {
() -> {
try {
hostname = getLocalhost.call().getCanonicalHostName();
- expirationTimestamp = System.currentTimeMillis() + cacheDuration;
+ cacheFreshUntil =
+ Deadline.after(ticker, HOSTNAME_CACHE_DURATION, TimeUnit.MILLISECONDS);
} finally {
updateRunning.set(false);
}
@@ -156,7 +158,7 @@ private void updateCache() {
}
private void handleCacheUpdateFailure() {
- expirationTimestamp = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1);
+ cacheFreshUntil = Deadline.after(ticker, 1, TimeUnit.SECONDS);
}
private static final class HostnameCacheThreadFactory implements ThreadFactory {
diff --git a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt
index 3cc3a52aa2..93cb993696 100644
--- a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt
+++ b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt
@@ -2,6 +2,7 @@ package io.sentry
import com.google.common.truth.Truth.assertThat
import io.sentry.test.getProperty
+import io.sentry.time.TestMonotonicTicker
import java.net.InetAddress
import java.util.concurrent.ThreadPoolExecutor
import java.util.concurrent.TimeUnit
@@ -14,7 +15,7 @@ class HostnameCacheTest {
private fun getSut(): HostnameCache {
val address = mock()
whenever(address.canonicalHostName).thenReturn("myhost")
- return HostnameCache(TimeUnit.HOURS.toMillis(1)) { address }
+ return HostnameCache({ address }, TestMonotonicTicker())
}
@Test
@@ -23,6 +24,22 @@ class HostnameCacheTest {
assertThat(cache.hostname).isEqualTo("myhost")
}
+ @Test
+ fun `hostname is re-resolved only once the cache duration has elapsed`() {
+ val ticker = TestMonotonicTicker()
+ val address = mock()
+ whenever(address.canonicalHostName).thenReturn("first", "second")
+ val cache = HostnameCache({ address }, ticker)
+
+ assertThat(cache.hostname).isEqualTo("first")
+
+ ticker.advance(4, TimeUnit.HOURS)
+ assertThat(cache.hostname).isEqualTo("first")
+
+ ticker.advance(1, TimeUnit.HOURS)
+ assertThat(cache.hostname).isEqualTo("second")
+ }
+
@Test
fun `worker thread times out while idle instead of staying alive`() {
val cache = getSut()
diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
index fe5c835c90..ee88f06ae2 100644
--- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
+++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
@@ -6,9 +6,11 @@ import io.sentry.protocol.DebugMeta
import io.sentry.protocol.SdkVersion
import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
+import io.sentry.time.TestMonotonicTicker
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
@@ -17,7 +19,6 @@ import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertSame
import kotlin.test.assertTrue
-import org.awaitility.kotlin.await
import org.mockito.Mockito
import org.mockito.kotlin.mock
import org.mockito.kotlin.reset
@@ -36,6 +37,7 @@ class MainEventProcessorTest {
}
val scopes = mock()
val getLocalhost = mock()
+ val hostnameCacheTicker = TestMonotonicTicker()
lateinit var sentryTracer: SentryTracer
private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)
@@ -48,7 +50,6 @@ class MainEventProcessorTest {
serverName: String? = "server",
host: String? = null,
resolveHostDelay: Long? = null,
- hostnameCacheDuration: Long = 10,
proguardUuid: String? = null,
bundleIds: List? = null,
modules: Map? = null,
@@ -76,7 +77,7 @@ class MainEventProcessorTest {
whenever(scopes.options).thenReturn(sentryOptions)
sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
- val hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost }
+ val hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker)
hostnameCacheMock.`when` { HostnameCache.getInstance() }.thenReturn(hostnameCache)
return MainEventProcessor(sentryOptions)
@@ -401,7 +402,7 @@ class MainEventProcessorTest {
@Test
fun `uses cache to retrieve servername for subsequent events`() {
- val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000)
+ val processor = fixture.getSut(serverName = null, host = "aHost")
val firstEvent = SentryEvent()
processor.process(firstEvent, Hint())
assertEquals("aHost", firstEvent.serverName)
@@ -420,12 +421,11 @@ class MainEventProcessorTest {
reset(fixture.getLocalhost)
whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost")
+ fixture.hostnameCacheTicker.advance(6, TimeUnit.HOURS)
- await.untilAsserted {
- val secondEvent = SentryEvent()
- processor.process(secondEvent, Hint())
- assertEquals("newHost", secondEvent.serverName)
- }
+ val secondEvent = SentryEvent()
+ processor.process(secondEvent, Hint())
+ assertEquals("newHost", secondEvent.serverName)
}
@Test