Skip to content

Commit fbec6c7

Browse files
runningcodeclaude
andcommitted
ref(core): Measure the hostname cache TTL on a monotonic ticker (JAVA-579)
HostnameCache stored an absolute expiry built from System.currentTimeMillis() and compared it against a fresh reading of the same clock, so a device time change resized the 5 hour TTL: a backward step extended it, a forward step expired the cache early. It is now a Deadline on a MonotonicTicker. The ticker comes from SentryOptions.getMonotonicTicker(), the same way RateLimiter takes it, so on Android the TTL also counts time spent in deep sleep. The cache names the ticker as its only collaborator rather than taking the whole options. The LazyEvaluator that holds the cache defers reading the ticker until after SentryAndroidOptions has overridden it. The expiry no longer starts at 0: on a boot-relative ticker 0 is a real and very recent instant, so it would read as freshly set rather than unset. It starts as Deadline.passed(ticker) instead. The TTL is not a serialized value, so this is not gated behind v9. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent b2f9813 commit fbec6c7

6 files changed

Lines changed: 76 additions & 38 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
### Internal
2323

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

2627
### Dependencies
2728

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

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.sentry;
22

3+
import io.sentry.time.Deadline;
4+
import io.sentry.time.MonotonicTicker;
35
import io.sentry.util.Objects;
46
import java.net.InetAddress;
57
import java.util.concurrent.Callable;
@@ -21,9 +23,9 @@
2123
* Time sensitive cache in charge of keeping track of the hostname. The {@code
2224
* InetAddress.getLocalHost().getCanonicalHostName()} call can be quite expensive and could be
2325
* called for the creation of each {@link SentryEvent}. This system will prevent unnecessary costs
24-
* by keeping track of the hostname for a period defined during the construction. For performance
25-
* purposes, the operation of retrieving the hostname will automatically fail after a period of time
26-
* defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
26+
* by keeping track of the hostname for a period defined by {@link #HOSTNAME_CACHE_DURATION}. For
27+
* performance purposes, the operation of retrieving the hostname will automatically fail after a
28+
* period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
2729
*
2830
* <p>One instance is held per {@link SentryOptions} and should be obtained through {@link
2931
* SentryOptions#getHostnameCache()}.
@@ -38,14 +40,13 @@ public final class HostnameCache {
3840
/** How long the worker thread may stay idle before it self-terminates. */
3941
private static final long THREAD_KEEP_ALIVE_SECONDS = 30;
4042

41-
/** Time for which the cache is kept. */
42-
private final long cacheDuration;
43+
private final @NotNull MonotonicTicker ticker;
4344

4445
/** Current value for hostname (might change over time). */
4546
@Nullable private volatile String hostname;
4647

47-
/** Time at which the cache should expire. */
48-
private volatile long expirationTimestamp;
48+
/** When the cached hostname goes stale. */
49+
private volatile @NotNull Deadline cacheFreshUntil;
4950

5051
/** Whether a cache update thread is currently running or not. */
5152
private final @NotNull AtomicBoolean updateRunning = new AtomicBoolean(false);
@@ -54,26 +55,32 @@ public final class HostnameCache {
5455

5556
private final @NotNull ExecutorService executorService;
5657

57-
HostnameCache() {
58-
this(HOSTNAME_CACHE_DURATION);
59-
}
60-
61-
HostnameCache(long cacheDuration) {
58+
/**
59+
* Names the only collaborator a hostname cache reads, rather than taking the whole {@link
60+
* SentryOptions}.
61+
*
62+
* @param ticker the ticker the cache lifetime is measured on
63+
*/
64+
HostnameCache(final @NotNull MonotonicTicker ticker) {
6265
// avoid method refs on Android due to some issues with older AGP setups
6366
// noinspection Convert2MethodRef
64-
this(cacheDuration, () -> InetAddress.getLocalHost());
67+
this(() -> InetAddress.getLocalHost(), ticker);
6568
}
6669

6770
/**
6871
* Sets up a cache for the hostname.
6972
*
70-
* @param cacheDuration cache duration in milliseconds.
7173
* @param getLocalhost a callback to obtain the localhost address - this is mostly here because of
7274
* testability
75+
* @param ticker the ticker the cache lifetime is measured on
7376
*/
74-
HostnameCache(long cacheDuration, final @NotNull Callable<InetAddress> getLocalhost) {
75-
this.cacheDuration = cacheDuration;
77+
HostnameCache(
78+
final @NotNull Callable<InetAddress> getLocalhost, final @NotNull MonotonicTicker ticker) {
7679
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
80+
this.ticker = Objects.requireNonNull(ticker, "ticker is required");
81+
// Nothing resolved yet, so the cache is stale rather than fresh until updateCache says
82+
// otherwise.
83+
this.cacheFreshUntil = Deadline.passed(ticker);
7784
// A single thread executor whose worker thread times out while idle, so no thread is kept
7885
// alive between the infrequent cache refreshes and nothing has to shut it down.
7986
final @NotNull ThreadPoolExecutor executor =
@@ -98,8 +105,7 @@ public final class HostnameCache {
98105
*/
99106
@Nullable
100107
public String getHostname() {
101-
if (expirationTimestamp < System.currentTimeMillis()
102-
&& updateRunning.compareAndSet(false, true)) {
108+
if (cacheFreshUntil.hasPassed() && updateRunning.compareAndSet(false, true)) {
103109
updateCache();
104110
}
105111

@@ -112,7 +118,8 @@ private void updateCache() {
112118
() -> {
113119
try {
114120
hostname = getLocalhost.call().getCanonicalHostName();
115-
expirationTimestamp = System.currentTimeMillis() + cacheDuration;
121+
cacheFreshUntil =
122+
Deadline.after(ticker, HOSTNAME_CACHE_DURATION, TimeUnit.MILLISECONDS);
116123
} finally {
117124
updateRunning.set(false);
118125
}
@@ -145,7 +152,7 @@ private void updateCache() {
145152
}
146153

147154
private void handleCacheUpdateFailure() {
148-
expirationTimestamp = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1);
155+
cacheFreshUntil = Deadline.after(ticker, 1, TimeUnit.SECONDS);
149156
}
150157

151158
private static final class HostnameCacheThreadFactory implements ThreadFactory {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,12 @@ public class SentryOptions implements RateLimiterConfig {
539539
* Cache of the local hostname, used when {@link #isAttachServerName()} is enabled.
540540
*
541541
* <p>Evaluated lazily because resolving the hostname blocks on {@code
542-
* InetAddress.getLocalHost()}, which no {@code Sentry.init} should pay for up front.
542+
* InetAddress.getLocalHost()}, which no {@code Sentry.init} should pay for up front. Deferring
543+
* also means {@link #getMonotonicTicker()} is read after subclasses have overridden it.
543544
*/
544545
@ApiStatus.Internal
545546
private final @NotNull LazyEvaluator<HostnameCache> hostnameCache =
546-
new LazyEvaluator<>(() -> new HostnameCache());
547+
new LazyEvaluator<>(() -> new HostnameCache(getMonotonicTicker()));
547548

548549
private final @NotNull List<IPerformanceCollector> performanceCollectors = new ArrayList<>();
549550

‎sentry/src/test/java/io/sentry/HostnameCacheTest.kt‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package io.sentry
22

33
import com.google.common.truth.Truth.assertThat
44
import io.sentry.test.getProperty
5-
import io.sentry.test.injectForField
5+
import io.sentry.time.TestMonotonicTicker
66
import java.net.InetAddress
77
import java.util.concurrent.ThreadPoolExecutor
88
import java.util.concurrent.TimeUnit
@@ -16,7 +16,7 @@ class HostnameCacheTest {
1616
private fun getSut(): HostnameCache {
1717
val address = mock<InetAddress>()
1818
whenever(address.canonicalHostName).thenReturn("myhost")
19-
return HostnameCache(TimeUnit.HOURS.toMillis(1)) { address }
19+
return HostnameCache({ address }, TestMonotonicTicker())
2020
}
2121

2222
@Test
@@ -25,13 +25,32 @@ class HostnameCacheTest {
2525
assertThat(cache.hostname).isEqualTo("myhost")
2626
}
2727

28+
@Test
29+
fun `hostname is re-resolved only once the cache duration has elapsed`() {
30+
val ticker = TestMonotonicTicker()
31+
val address = mock<InetAddress>()
32+
whenever(address.canonicalHostName).thenReturn("first", "second")
33+
val cache = HostnameCache({ address }, ticker)
34+
35+
assertThat(cache.hostname).isEqualTo("first")
36+
37+
ticker.advance(4, TimeUnit.HOURS)
38+
assertThat(cache.hostname).isEqualTo("first")
39+
40+
ticker.advance(1, TimeUnit.HOURS)
41+
assertThat(cache.hostname).isEqualTo("second")
42+
}
43+
2844
@Test
2945
fun `a refresh that cannot be queued does not stop later refreshes`() {
30-
val cache = getSut()
31-
// Reject the next submit the way an executor that could not start a thread would, and mark the
32-
// cache stale so that reading the hostname attempts a refresh.
46+
val ticker = TestMonotonicTicker()
47+
val address = mock<InetAddress>()
48+
whenever(address.canonicalHostName).thenReturn("myhost")
49+
val cache = HostnameCache({ address }, ticker)
50+
// Reject the next submit the way an executor that could not start a thread would, and let the
51+
// cache go stale so that reading the hostname attempts a refresh.
3352
cache.getProperty<ThreadPoolExecutor>("executorService").shutdown()
34-
cache.injectForField("expirationTimestamp", 0L)
53+
ticker.advance(5, TimeUnit.HOURS)
3554

3655
assertThat(cache.hostname).isEqualTo("myhost")
3756

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,18 @@ import io.sentry.protocol.DebugMeta
77
import io.sentry.protocol.SdkVersion
88
import io.sentry.protocol.SentryTransaction
99
import io.sentry.protocol.User
10+
import io.sentry.time.TestMonotonicTicker
1011
import io.sentry.util.HintUtils
1112
import java.lang.RuntimeException
1213
import java.net.InetAddress
14+
import java.util.concurrent.TimeUnit
1315
import kotlin.test.Test
1416
import kotlin.test.assertEquals
1517
import kotlin.test.assertFalse
1618
import kotlin.test.assertNotNull
1719
import kotlin.test.assertNull
1820
import kotlin.test.assertSame
1921
import kotlin.test.assertTrue
20-
import org.awaitility.kotlin.await
2122
import org.mockito.kotlin.mock
2223
import org.mockito.kotlin.reset
2324
import org.mockito.kotlin.times
@@ -28,6 +29,7 @@ class MainEventProcessorTest {
2829
class Fixture {
2930
val scopes = mock<IScopes>()
3031
val getLocalhost = mock<InetAddress>()
32+
val hostnameCacheTicker = TestMonotonicTicker()
3133
// Built in getSut() rather than here: the constructor resolves the hostname straight away,
3234
// so it has to run after getLocalhost is stubbed.
3335
lateinit var hostnameCache: HostnameCache
@@ -53,7 +55,6 @@ class MainEventProcessorTest {
5355
serverName: String? = "server",
5456
host: String? = null,
5557
resolveHostDelay: Long? = null,
56-
hostnameCacheDuration: Long = 10,
5758
proguardUuid: String? = null,
5859
bundleIds: List<String>? = null,
5960
modules: Map<String, String>? = null,
@@ -80,7 +81,7 @@ class MainEventProcessorTest {
8081
}
8182
whenever(scopes.options).thenReturn(sentryOptions)
8283
sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
83-
hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost }
84+
hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker)
8485

8586
return MainEventProcessor(sentryOptions)
8687
}
@@ -428,7 +429,7 @@ class MainEventProcessorTest {
428429

429430
@Test
430431
fun `uses cache to retrieve servername for subsequent events`() {
431-
val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000)
432+
val processor = fixture.getSut(serverName = null, host = "aHost")
432433
val firstEvent = SentryEvent()
433434
processor.process(firstEvent, Hint())
434435
assertEquals("aHost", firstEvent.serverName)
@@ -447,12 +448,11 @@ class MainEventProcessorTest {
447448

448449
reset(fixture.getLocalhost)
449450
whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost")
451+
fixture.hostnameCacheTicker.advance(6, TimeUnit.HOURS)
450452

451-
await.untilAsserted {
452-
val secondEvent = SentryEvent()
453-
processor.process(secondEvent, Hint())
454-
assertEquals("newHost", secondEvent.serverName)
455-
}
453+
val secondEvent = SentryEvent()
454+
processor.process(secondEvent, Hint())
455+
assertEquals("newHost", secondEvent.serverName)
456456
}
457457

458458
@Test

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import com.google.common.truth.Truth.assertThat
44
import io.sentry.SentryOptions.RequestSize
55
import io.sentry.logger.ILoggerBatchProcessorFactory
66
import io.sentry.test.getProperty
7+
import io.sentry.time.MonotonicTicker
8+
import io.sentry.time.TestMonotonicTicker
79
import io.sentry.util.LazyEvaluator
810
import io.sentry.util.StringUtils
911
import java.io.File
@@ -1337,9 +1339,16 @@ class SentryOptionsTest {
13371339
private fun SentryOptions.peekHostnameCache(): HostnameCache? =
13381340
getProperty<LazyEvaluator<HostnameCache>>("hostnameCache").getProperty("value")
13391341

1342+
/** Options whose hostname cache resolves against a ticker the test controls. */
1343+
private fun optionsWithTicker(ticker: MonotonicTicker): SentryOptions =
1344+
object : SentryOptions() {
1345+
override fun getMonotonicTicker(): MonotonicTicker = ticker
1346+
}
1347+
13401348
@Test
13411349
fun `hostname is resolved on first use, not when options are constructed`() {
1342-
val options = SentryOptions()
1350+
val ticker = TestMonotonicTicker()
1351+
val options = optionsWithTicker(ticker)
13431352

13441353
assertThat(options.peekHostnameCache()).isNull()
13451354

@@ -1348,5 +1357,6 @@ class SentryOptionsTest {
13481357
// Also keeps the assertion above honest: peek does report a cache once one exists.
13491358
assertThat(options.peekHostnameCache()).isSameInstanceAs(cache)
13501359
assertThat(options.hostnameCache).isSameInstanceAs(cache)
1360+
assertThat(cache.getProperty<MonotonicTicker>("ticker")).isSameInstanceAs(ticker)
13511361
}
13521362
}

0 commit comments

Comments
 (0)