Skip to content

Commit 0bd817c

Browse files
committed
Create custom Android Logger API, address PR feedback
1 parent 2f205b3 commit 0bd817c

File tree

16 files changed

+219
-64
lines changed

16 files changed

+219
-64
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package io.sentry.android.core;
2+
3+
import io.sentry.Scopes;
4+
import io.sentry.SentryLevel;
5+
import io.sentry.logger.LoggerApi;
6+
import io.sentry.logger.LoggerBatchProcessor;
7+
import java.io.Closeable;
8+
import java.io.IOException;
9+
import org.jetbrains.annotations.ApiStatus;
10+
import org.jetbrains.annotations.NotNull;
11+
12+
@ApiStatus.Internal
13+
public class AndroidLoggerApi extends LoggerApi implements Closeable, AppState.AppStateListener {
14+
15+
public AndroidLoggerApi(final @NotNull Scopes scopes) {
16+
super(scopes);
17+
AppState.getInstance().addAppStateListener(this);
18+
}
19+
20+
@Override
21+
@ApiStatus.Internal
22+
public void close() throws IOException {
23+
AppState.getInstance().removeAppStateListener(this);
24+
}
25+
26+
@Override
27+
public void onForeground() {}
28+
29+
@Override
30+
public void onBackground() {
31+
try {
32+
scopes
33+
.getOptions()
34+
.getExecutorService()
35+
.submit(
36+
new Runnable() {
37+
@Override
38+
public void run() {
39+
scopes.getClient().flushLogs(LoggerBatchProcessor.FLUSH_AFTER_MS);
40+
}
41+
});
42+
} catch (Throwable t) {
43+
scopes
44+
.getOptions()
45+
.getLogger()
46+
.log(SentryLevel.ERROR, t, "Failed to submit log flush runnable");
47+
}
48+
}
49+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.sentry.android.core;
2+
3+
import io.sentry.Scopes;
4+
import io.sentry.logger.ILoggerApiFactory;
5+
import io.sentry.logger.LoggerApi;
6+
import org.jetbrains.annotations.ApiStatus;
7+
import org.jetbrains.annotations.NotNull;
8+
9+
@ApiStatus.Internal
10+
public final class AndroidLoggerApiFactory implements ILoggerApiFactory {
11+
12+
@Override
13+
@NotNull
14+
public LoggerApi create(@NotNull Scopes scopes) {
15+
return new AndroidLoggerApi(scopes);
16+
}
17+
}

sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ static void loadDefaultAndMetadataOptions(
139139

140140
readDefaultOptionValues(options, finalContext, buildInfoProvider);
141141
AppState.getInstance().registerLifecycleObserver(options);
142+
143+
options.setLoggerApiFactory(new AndroidLoggerApiFactory());
142144
}
143145

144146
@TestOnly

sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
5757
scopes,
5858
this.options.getSessionTrackingIntervalMillis(),
5959
this.options.isEnableAutoSessionTracking(),
60-
this.options.isEnableAppLifecycleBreadcrumbs(),
61-
this.options.getLogs().isEnabled());
60+
this.options.isEnableAppLifecycleBreadcrumbs());
6261

6362
AppState.getInstance().addAppStateListener(watcher);
6463
}

sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import io.sentry.ISentryLifecycleToken;
66
import io.sentry.SentryLevel;
77
import io.sentry.Session;
8-
import io.sentry.logger.LoggerBatchProcessor;
98
import io.sentry.transport.CurrentDateProvider;
109
import io.sentry.transport.ICurrentDateProvider;
1110
import io.sentry.util.AutoClosableReentrantLock;
@@ -30,22 +29,18 @@ final class LifecycleWatcher implements AppState.AppStateListener {
3029
private final boolean enableSessionTracking;
3130
private final boolean enableAppLifecycleBreadcrumbs;
3231

33-
private final boolean enableLogFlushing;
34-
3532
private final @NotNull ICurrentDateProvider currentDateProvider;
3633

3734
LifecycleWatcher(
3835
final @NotNull IScopes scopes,
3936
final long sessionIntervalMillis,
4037
final boolean enableSessionTracking,
41-
final boolean enableAppLifecycleBreadcrumbs,
42-
final boolean enableLogFlushing) {
38+
final boolean enableAppLifecycleBreadcrumbs) {
4339
this(
4440
scopes,
4541
sessionIntervalMillis,
4642
enableSessionTracking,
4743
enableAppLifecycleBreadcrumbs,
48-
enableLogFlushing,
4944
CurrentDateProvider.getInstance());
5045
}
5146

@@ -54,12 +49,10 @@ final class LifecycleWatcher implements AppState.AppStateListener {
5449
final long sessionIntervalMillis,
5550
final boolean enableSessionTracking,
5651
final boolean enableAppLifecycleBreadcrumbs,
57-
final boolean enableLogFlushing,
5852
final @NotNull ICurrentDateProvider currentDateProvider) {
5953
this.sessionIntervalMillis = sessionIntervalMillis;
6054
this.enableSessionTracking = enableSessionTracking;
6155
this.enableAppLifecycleBreadcrumbs = enableAppLifecycleBreadcrumbs;
62-
this.enableLogFlushing = enableLogFlushing;
6356
this.scopes = scopes;
6457
this.currentDateProvider = currentDateProvider;
6558
}
@@ -108,29 +101,6 @@ public void onBackground() {
108101
scheduleEndSession();
109102

110103
addAppBreadcrumb("background");
111-
112-
if (enableLogFlushing) {
113-
try {
114-
scopes
115-
.getOptions()
116-
.getExecutorService()
117-
.submit(
118-
new Runnable() {
119-
@Override
120-
public void run() {
121-
scopes
122-
.getGlobalScope()
123-
.getClient()
124-
.flushLogs(LoggerBatchProcessor.FLUSH_AFTER_MS);
125-
}
126-
});
127-
} catch (Throwable t) {
128-
scopes
129-
.getOptions()
130-
.getLogger()
131-
.log(SentryLevel.ERROR, t, "Failed to submit log flush runnable");
132-
}
133-
}
134104
}
135105

136106
private void scheduleEndSession() {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package io.sentry.android.core
2+
3+
import androidx.test.ext.junit.runners.AndroidJUnit4
4+
import io.sentry.Scopes
5+
import kotlin.test.assertIs
6+
import org.junit.Test
7+
import org.junit.runner.RunWith
8+
import org.mockito.kotlin.mock
9+
10+
@RunWith(AndroidJUnit4::class)
11+
class AndroidLoggerApiFactoryTest {
12+
13+
@Test
14+
fun `factory creates AndroidLogger`() {
15+
val factory = AndroidLoggerApiFactory()
16+
val logger = factory.create(mock<Scopes>())
17+
assertIs<AndroidLoggerApi>(logger)
18+
}
19+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package io.sentry.android.core
2+
3+
import androidx.test.ext.junit.runners.AndroidJUnit4
4+
import io.sentry.Scopes
5+
import io.sentry.SentryClient
6+
import io.sentry.SentryOptions
7+
import io.sentry.test.ImmediateExecutorService
8+
import kotlin.test.assertTrue
9+
import org.junit.Before
10+
import org.junit.Test
11+
import org.junit.runner.RunWith
12+
import org.mockito.kotlin.any
13+
import org.mockito.kotlin.mock
14+
import org.mockito.kotlin.verify
15+
import org.mockito.kotlin.whenever
16+
17+
@RunWith(AndroidJUnit4::class)
18+
class AndroidLoggerApiTest {
19+
20+
@Before
21+
fun setup() {
22+
AppState.getInstance().resetInstance()
23+
}
24+
25+
@Test
26+
fun `AndroidLogger registers and unregisters app state listener`() {
27+
val scopes = mock<Scopes>()
28+
val logger = AndroidLoggerApi(scopes)
29+
assertTrue(AppState.getInstance().lifecycleObserver.listeners.isNotEmpty())
30+
31+
logger.close()
32+
assertTrue(AppState.getInstance().lifecycleObserver.listeners.isEmpty())
33+
}
34+
35+
@Test
36+
fun `AndroidLogger triggers flushing if app goes in background`() {
37+
val scopes = mock<Scopes>()
38+
39+
val client = mock<SentryClient>()
40+
whenever(scopes.client).thenReturn(client)
41+
42+
val options = SentryOptions()
43+
options.executorService = ImmediateExecutorService()
44+
whenever(scopes.options).thenReturn(options)
45+
46+
val logger = AndroidLoggerApi(scopes)
47+
logger.onBackground()
48+
49+
verify(client).flushLogs(any())
50+
}
51+
}

sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,4 +926,10 @@ class AndroidOptionsInitializerTest {
926926
fixture.initSut()
927927
assertIs<AndroidRuntimeManager>(fixture.sentryOptions.runtimeManager)
928928
}
929+
930+
@Test
931+
fun `AndroidLoggerApiFactory is set in the options`() {
932+
fixture.initSut()
933+
assertIs<AndroidLoggerApiFactory>(fixture.sentryOptions.loggerApiFactory)
934+
}
929935
}

sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,11 @@ class AppLifecycleIntegrationTest {
3434
}
3535

3636
@Test
37-
fun `When SessionTracking and AppLifecycle breadcrumbs and Logs are disabled, lifecycle watcher should not be started`() {
37+
fun `When SessionTracking and AppLifecycle breadcrumbs are disabled, lifecycle watcher should not be started`() {
3838
val sut = fixture.getSut()
3939
fixture.options.apply {
4040
isEnableAppLifecycleBreadcrumbs = false
4141
isEnableAutoSessionTracking = false
42-
logs.isEnabled = false
4342
}
4443

4544
sut.register(fixture.scopes, fixture.options)

sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ class LifecycleWatcherTest {
4444
sessionIntervalMillis: Long = 0L,
4545
enableAutoSessionTracking: Boolean = true,
4646
enableAppLifecycleBreadcrumbs: Boolean = true,
47-
enableLogFlushing: Boolean = true,
4847
session: Session? = null,
4948
): LifecycleWatcher {
5049
val argumentCaptor: ArgumentCaptor<ScopeCallback> =
@@ -67,7 +66,6 @@ class LifecycleWatcherTest {
6766
sessionIntervalMillis,
6867
enableAutoSessionTracking,
6968
enableAppLifecycleBreadcrumbs,
70-
enableLogFlushing,
7169
dateProvider,
7270
)
7371
}
@@ -305,27 +303,4 @@ class LifecycleWatcherTest {
305303
watcher.onBackground()
306304
verify(fixture.replayController, timeout(10000)).stop()
307305
}
308-
309-
@Test
310-
fun `flush logs when going in background`() {
311-
val watcher = fixture.getSUT(enableLogFlushing = true)
312-
313-
watcher.onForeground()
314-
watcher.onBackground()
315-
316-
watcher.onForeground()
317-
watcher.onBackground()
318-
319-
verify(fixture.client, times(2)).flushLogs(any())
320-
}
321-
322-
@Test
323-
fun `do not flush logs when going in background when logging is disabled`() {
324-
val watcher = fixture.getSUT(enableLogFlushing = false)
325-
326-
watcher.onForeground()
327-
watcher.onBackground()
328-
329-
verify(fixture.client, never()).flushLogs(any())
330-
}
331306
}

0 commit comments

Comments
 (0)