Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazily retrieve HostnameCache in MainEventProcessor #2170

Merged
merged 3 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Lazily retrieve HostnameCache in MainEventProcessor ([#2170](https://github.com/getsentry/sentry-java/pull/2170))

## 6.2.1

### Fixes
Expand Down
29 changes: 17 additions & 12 deletions sentry/src/main/java/io/sentry/MainEventProcessor.java
Expand Up @@ -31,16 +31,10 @@ public final class MainEventProcessor implements EventProcessor, Closeable {
private final @NotNull SentryOptions options;
private final @NotNull SentryThreadFactory sentryThreadFactory;
private final @NotNull SentryExceptionFactory sentryExceptionFactory;
private final @Nullable HostnameCache hostnameCache;
private volatile @Nullable HostnameCache hostnameCache = null;

public MainEventProcessor(final @NotNull SentryOptions options) {
this(options, options.isAttachServerName() ? HostnameCache.getInstance() : 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(
Expand All @@ -53,14 +47,12 @@ public MainEventProcessor(final @NotNull SentryOptions options) {
MainEventProcessor(
final @NotNull SentryOptions options,
final @NotNull SentryThreadFactory sentryThreadFactory,
final @NotNull SentryExceptionFactory sentryExceptionFactory,
final @NotNull HostnameCache hostnameCache) {
final @NotNull SentryExceptionFactory sentryExceptionFactory) {
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
Expand Down Expand Up @@ -164,8 +156,21 @@ private void setServerName(final @NotNull SentryBaseEvent event) {
event.setServerName(options.getServerName());
}

if (options.isAttachServerName() && hostnameCache != null && event.getServerName() == null) {
event.setServerName(hostnameCache.getHostname());
if (options.isAttachServerName() && event.getServerName() == null) {
ensureHostnameCache();
if (hostnameCache != null) {
event.setServerName(hostnameCache.getHostname());
}
}
}

private void ensureHostnameCache() {
if (hostnameCache == null) {
synchronized (this) {
if (hostnameCache == null) {
hostnameCache = HostnameCache.getInstance();
}
}
}
}

Expand Down
22 changes: 20 additions & 2 deletions sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
Expand Up @@ -12,8 +12,10 @@ import io.sentry.protocol.SentryTransaction
import io.sentry.protocol.User
import io.sentry.util.HintUtils
import org.awaitility.kotlin.await
import org.mockito.Mockito
import java.lang.RuntimeException
import java.net.InetAddress
import kotlin.test.AfterTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
Expand All @@ -32,10 +34,12 @@ class MainEventProcessorTest {
}
val getLocalhost = mock<InetAddress>()
val sentryTracer = SentryTracer(TransactionContext("", ""), mock())
private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)

fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map<String, String> = emptyMap(), sendDefaultPii: Boolean? = null, serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10, proguardUuid: String? = null): MainEventProcessor {
sentryOptions.isAttachThreads = attachThreads
sentryOptions.isAttachStacktrace = attachStackTrace
sentryOptions.isAttachServerName = true
sentryOptions.environment = environment
sentryOptions.serverName = serverName
if (sendDefaultPii != null) {
Expand All @@ -52,10 +56,21 @@ class MainEventProcessorTest {
host
}
val hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost }
return MainEventProcessor(sentryOptions, hostnameCache)
hostnameCacheMock.`when`<Any> { HostnameCache.getInstance() }.thenReturn(hostnameCache)

return MainEventProcessor(sentryOptions)
}

fun teardown() {
hostnameCacheMock.close()
}
}

@AfterTest
fun teardown() {
fixture.teardown()
}

private val fixture = Fixture()

@Test
Expand Down Expand Up @@ -448,7 +463,10 @@ class MainEventProcessorTest {

@Test
fun `when processor is closed, closes hostname cache`() {
val sut = fixture.getSut()
val sut = fixture.getSut(serverName = null)

sut.process(SentryTransaction(fixture.sentryTracer), Hint())

sut.close()
assertNotNull(sut.hostnameCache) {
assertTrue(it.isClosed())
Expand Down