diff --git a/CHANGELOG.md b/CHANGELOG.md index 891afe0c0e1..aa677d1b124 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,8 @@ * Fix: Do not crash when passing null values to @Nullable methods, eg User and Scope * Enhancement: Send user.ip_address = {{auto}} when sendDefaultPii is true (#1015) * Fix: Resolving dashed properties from external configuration -* Feat: Read `uncaught.handler.enabled` property from the external configuration +* Feat: Read `uncaught.handler.enabled` property from the external configuration +* Feat: Resolve servername from the localhost address * Fix: Consider {{ auto }} as a default ip address (#1015) # 4.0.0-alpha.2 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index bbf7a26a328..931bc1928eb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -39,6 +39,7 @@ public final class SentryAndroidOptions extends SentryOptions { public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); + setAttachServerName(false); } private @NotNull SdkVersion createSdkVersion() { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a48ff370cf5..566fd7be785 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -699,6 +699,7 @@ public class io/sentry/SentryOptions { public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback; public fun getTransport ()Lio/sentry/transport/ITransport; public fun getTransportGate ()Lio/sentry/transport/ITransportGate; + public fun isAttachServerName ()Z public fun isAttachStacktrace ()Z public fun isAttachThreads ()Z public fun isDebug ()Z @@ -708,6 +709,7 @@ public class io/sentry/SentryOptions { public fun isEnableSessionTracking ()Z public fun isEnableUncaughtExceptionHandler ()Z public fun isSendDefaultPii ()Z + public fun setAttachServerName (Z)V public fun setAttachStacktrace (Z)V public fun setAttachThreads (Z)V public fun setBeforeBreadcrumb (Lio/sentry/SentryOptions$BeforeBreadcrumbCallback;)V diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java new file mode 100644 index 00000000000..11743105dca --- /dev/null +++ b/sentry/src/main/java/io/sentry/HostnameCache.java @@ -0,0 +1,121 @@ +package io.sentry; + +import io.sentry.util.Objects; +import java.net.InetAddress; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * 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. + */ +final class HostnameCache { + private static final long HOSTNAME_CACHE_DURATION = TimeUnit.HOURS.toMillis(5); + + /** Time before the get hostname operation times out (in ms). */ + private static final long GET_HOSTNAME_TIMEOUT = TimeUnit.SECONDS.toMillis(1); + /** Time for which the cache is kept. */ + private final long cacheDuration; + /** Current value for hostname (might change over time). */ + @Nullable private volatile String hostname; + /** Time at which the cache should expire. */ + private volatile long expirationTimestamp; + /** Whether a cache update thread is currently running or not. */ + private final @NotNull AtomicBoolean updateRunning = new AtomicBoolean(false); + + private final @NotNull Callable getLocalhost; + + private final @NotNull ExecutorService executorService = + Executors.newSingleThreadExecutor(new HostnameCacheThreadFactory()); + + public HostnameCache() { + this(HOSTNAME_CACHE_DURATION); + } + + HostnameCache(long cacheDuration) { + this(cacheDuration, () -> InetAddress.getLocalHost()); + } + + /** + * 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 + */ + HostnameCache(long cacheDuration, final @NotNull Callable getLocalhost) { + this.cacheDuration = cacheDuration; + this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required"); + updateCache(); + } + + /** + * Gets the hostname of the current machine. + * + *

Gets the value from the cache if possible otherwise calls {@link #updateCache()}. + * + * @return the hostname of the current machine. + */ + @Nullable + String getHostname() { + if (expirationTimestamp < System.currentTimeMillis() + && updateRunning.compareAndSet(false, true)) { + updateCache(); + } + + return hostname; + } + + /** Force an update of the cache to get the current value of the hostname. */ + private void updateCache() { + final Callable hostRetriever = + () -> { + try { + hostname = getLocalhost.call().getCanonicalHostName(); + expirationTimestamp = System.currentTimeMillis() + cacheDuration; + } finally { + updateRunning.set(false); + } + + return null; + }; + + try { + final Future futureTask = executorService.submit(hostRetriever); + futureTask.get(GET_HOSTNAME_TIMEOUT, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + handleCacheUpdateFailure(); + } catch (ExecutionException | TimeoutException | RuntimeException e) { + handleCacheUpdateFailure(); + } + } + + private void handleCacheUpdateFailure() { + expirationTimestamp = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1); + } + + private static final class HostnameCacheThreadFactory implements ThreadFactory { + private int cnt; + + @Override + public @NotNull Thread newThread(final @NotNull Runnable r) { + final Thread ret = new Thread(r, "SentryHostnameCache-" + cnt++); + ret.setDaemon(true); + return ret; + } + } +} diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 034bcb4a143..cbc188be3b2 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -29,9 +29,16 @@ public final class MainEventProcessor implements EventProcessor { private final @NotNull SentryOptions options; private final @NotNull SentryThreadFactory sentryThreadFactory; private final @NotNull SentryExceptionFactory sentryExceptionFactory; + private final @Nullable HostnameCache hostnameCache; MainEventProcessor(final @NotNull SentryOptions options) { + this(options, options.isAttachServerName() ? new HostnameCache() : null); + } + + MainEventProcessor( + final @NotNull SentryOptions options, final @Nullable HostnameCache hostnameCache) { this.options = Objects.requireNonNull(options, "The SentryOptions is required."); + this.hostnameCache = hostnameCache; final SentryStackTraceFactory sentryStackTraceFactory = new SentryStackTraceFactory( @@ -44,12 +51,14 @@ public final class MainEventProcessor implements EventProcessor { MainEventProcessor( final @NotNull SentryOptions options, final @NotNull SentryThreadFactory sentryThreadFactory, - final @NotNull SentryExceptionFactory sentryExceptionFactory) { + final @NotNull SentryExceptionFactory sentryExceptionFactory, + final @NotNull HostnameCache hostnameCache) { this.options = Objects.requireNonNull(options, "The SentryOptions is required."); this.sentryThreadFactory = Objects.requireNonNull(sentryThreadFactory, "The SentryThreadFactory is required."); this.sentryExceptionFactory = Objects.requireNonNull(sentryExceptionFactory, "The SentryExceptionFactory is required."); + this.hostnameCache = Objects.requireNonNull(hostnameCache, "The HostnameCache is required"); } @Override @@ -139,5 +148,8 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) { event.getUser().setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); } } + if (options.isAttachServerName() && hostnameCache != null && event.getServerName() == null) { + event.setServerName(hostnameCache.getHostname()); + } } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index d403e27be04..3db7283d83b 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -203,6 +203,9 @@ public class SentryOptions { /** The server name used in the Sentry messages. */ private String serverName; + /** Automatically resolve server name. */ + private boolean attachServerName = true; + /* When enabled, Sentry installs UncaughtExceptionHandlerIntegration. */ @@ -852,6 +855,24 @@ public void setServerName(@Nullable String serverName) { this.serverName = serverName; } + /** + * Returns if SDK automatically resolves and attaches server name to events. + * + * @return true if enabled false if otherwise + */ + public boolean isAttachServerName() { + return attachServerName; + } + + /** + * Sets if SDK should automatically resolve and attache server name to events. + * + * @param attachServerName true if enabled false if otherwise + */ + public void setAttachServerName(boolean attachServerName) { + this.attachServerName = attachServerName; + } + /** * Returns the session tracking interval in millis * diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index ca898018bc9..d7ec234a734 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -1,11 +1,16 @@ package io.sentry import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.reset +import com.nhaarman.mockitokotlin2.times +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever import io.sentry.hints.ApplyScopeData import io.sentry.hints.Cached import io.sentry.protocol.SdkVersion import io.sentry.protocol.User import java.lang.RuntimeException +import java.net.InetAddress import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -13,6 +18,7 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue +import org.awaitility.kotlin.await class MainEventProcessorTest { class Fixture { @@ -20,21 +26,30 @@ class MainEventProcessorTest { dsn = dsnString release = "release" dist = "dist" - serverName = "server" sdkVersion = SdkVersion().apply { name = "test" version = "1.2.3" } } - fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map = emptyMap(), sendDefaultPii: Boolean? = null): MainEventProcessor { + val getLocalhost = mock() + + fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map = emptyMap(), sendDefaultPii: Boolean? = null, serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10): MainEventProcessor { sentryOptions.isAttachThreads = attachThreads sentryOptions.isAttachStacktrace = attachStackTrace sentryOptions.environment = environment + sentryOptions.serverName = serverName if (sendDefaultPii != null) { sentryOptions.isSendDefaultPii = sendDefaultPii } tags.forEach { sentryOptions.setTag(it.key, it.value) } - return MainEventProcessor(sentryOptions) + whenever(getLocalhost.canonicalHostName).thenAnswer { + if (resolveHostDelay != null) { + Thread.sleep(resolveHostDelay) + } + host + } + val hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost } + return MainEventProcessor(sentryOptions, hostnameCache) } } @@ -261,6 +276,68 @@ class MainEventProcessorTest { assertEquals("event-tag-value", event.tags["tag2"]) } + @Test + fun `sets servername retrieved from the local address`() { + val processor = fixture.getSut(serverName = null, host = "aHost") + val event = SentryEvent() + processor.process(event, null) + assertEquals("aHost", event.serverName) + } + + @Test + fun `sets servername to null if retrieving takes longer time`() { + val processor = fixture.getSut(serverName = null, host = "aHost", resolveHostDelay = 2000) + val event = SentryEvent() + processor.process(event, null) + assertNull(event.serverName) + } + + @Test + fun `uses cache to retrieve servername for subsequent events`() { + val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000) + val firstEvent = SentryEvent() + processor.process(firstEvent, null) + assertEquals("aHost", firstEvent.serverName) + val secondEvent = SentryEvent() + processor.process(secondEvent, null) + assertEquals("aHost", secondEvent.serverName) + verify(fixture.getLocalhost, times(1)).canonicalHostName + } + + @Test + fun `when cache expires, retrieves new host name from the local address`() { + val processor = fixture.getSut(serverName = null, host = "aHost") + val firstEvent = SentryEvent() + processor.process(firstEvent, null) + assertEquals("aHost", firstEvent.serverName) + + reset(fixture.getLocalhost) + whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost") + + await.untilAsserted { + val secondEvent = SentryEvent() + processor.process(secondEvent, null) + assertEquals("newHost", secondEvent.serverName) + } + } + + @Test + fun `does not set serverName on events that already have server names`() { + val processor = fixture.getSut(serverName = null, host = "aHost") + val event = SentryEvent() + event.serverName = "eventHost" + processor.process(event, null) + assertEquals("eventHost", event.serverName) + } + + @Test + fun `does not set serverName on events if serverName is set on SentryOptions`() { + val processor = fixture.getSut(serverName = "optionsHost", host = "aHost") + val event = SentryEvent() + processor.process(event, null) + assertEquals("optionsHost", event.serverName) + } + private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply { val mockThrowable = mock() val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable)