Skip to content

Commit 6dcf8f7

Browse files
authored
Clean up and clarify Crashlytics logging. (#2362)
* Clean up and clarify Crashlytics logging. Made some log messages clearer and easier to understand, also reduced noise by changing the log level on existing logs to: E: Unexpected/error state, not recoverable. Crashlytics will not function. W: Unexpected/error state, but recoverable. D: SDK lifecycle logs ("what" is happening). - Unix-style "no news is good news" logging, if something goes wrong, it will appear in W or E logs. V: Finer-grained logging ("how" things are happening). - "Success" logs, as well as logging of internally-stored values (e.g. current mapping file ID) are logged at this level. Also modified a few logs to *only* print in an error case.
1 parent d03a08e commit 6dcf8f7

File tree

20 files changed

+115
-100
lines changed

20 files changed

+115
-100
lines changed
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import java.util.UUID;
2525
import junit.framework.TestCase;
2626

27-
public class BreakpadControllerTest extends TestCase {
27+
public class CrashpadControllerTest extends TestCase {
2828

29-
private BreakpadController controller;
29+
private CrashpadController controller;
3030

3131
private Context context;
3232
private NativeApi mockNativeApi;
@@ -40,7 +40,7 @@ protected void setUp() throws Exception {
4040
context = ApplicationProvider.getApplicationContext();
4141
mockNativeApi = mock(NativeApi.class);
4242
mockFilesManager = mock(CrashFilesManager.class);
43-
controller = new BreakpadController(context, mockNativeApi, mockFilesManager);
43+
controller = new CrashpadController(context, mockNativeApi, mockFilesManager);
4444
}
4545

4646
@Override

firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/BreakpadController.java renamed to firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import java.io.OutputStreamWriter;
2828
import java.nio.charset.Charset;
2929

30-
class BreakpadController implements NativeComponentController {
30+
class CrashpadController implements NativeComponentController {
3131

3232
private static final Charset UTF_8 = Charset.forName("UTF-8");
3333
private static final String SESSION_METADATA_FILE = "session.json";
@@ -39,7 +39,7 @@ class BreakpadController implements NativeComponentController {
3939
private final NativeApi nativeApi;
4040
private final CrashFilesManager filesManager;
4141

42-
BreakpadController(Context context, NativeApi nativeApi, CrashFilesManager filesManager) {
42+
CrashpadController(Context context, NativeApi nativeApi, CrashFilesManager filesManager) {
4343
this.context = context;
4444
this.nativeApi = nativeApi;
4545
this.filesManager = filesManager;
@@ -55,7 +55,7 @@ public boolean initialize(String sessionId) {
5555
initSuccess = nativeApi.initialize(crashReportPath, context.getAssets());
5656
}
5757
} catch (IOException e) {
58-
Logger.getLogger().e("Error initializing CrashlyticsNdk", e);
58+
Logger.getLogger().e("Error initializing Crashlytics NDK", e);
5959
}
6060
return initSuccess;
6161
}
@@ -82,12 +82,14 @@ public SessionFiles getFilesForSession(String sessionId) {
8282
final File sessionFileDirectoryForMinidump = new File(sessionFileDirectory, "pending");
8383

8484
Logger.getLogger()
85-
.d("Minidump directory: " + sessionFileDirectoryForMinidump.getAbsolutePath());
85+
.v("Minidump directory: " + sessionFileDirectoryForMinidump.getAbsolutePath());
8686

8787
File minidump = getSingleFileWithExtension(sessionFileDirectoryForMinidump, ".dmp");
8888

8989
Logger.getLogger()
90-
.d("Minidump " + (minidump != null && minidump.exists() ? "exists" : "does not exist"));
90+
.v(
91+
"Minidump file "
92+
+ (minidump != null && minidump.exists() ? "exists" : "does not exist"));
9193

9294
final SessionFiles.Builder builder = new SessionFiles.Builder();
9395
if (sessionFileDirectory != null

firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/FirebaseCrashlyticsNdk.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static FirebaseCrashlyticsNdk create(@NonNull Context context) {
3131
final File rootDir = new File(context.getFilesDir(), FILES_PATH);
3232

3333
final NativeComponentController controller =
34-
new BreakpadController(
34+
new CrashpadController(
3535
context, new JniNativeApi(context), new NdkCrashFilesManager(rootDir));
3636
return new FirebaseCrashlyticsNdk(controller);
3737
}
@@ -50,8 +50,9 @@ public boolean hasCrashDataForSession(@NonNull String sessionId) {
5050
@Override
5151
public boolean openSession(String sessionId) {
5252
final boolean initSuccess = controller.initialize(sessionId);
53-
Logger.getLogger()
54-
.i("Crashlytics NDK initialization " + (initSuccess ? "successful" : "FAILED"));
53+
if (!initSuccess) {
54+
Logger.getLogger().w("Failed to initialize Crashlytics NDK for session " + sessionId);
55+
}
5556
return initSuccess;
5657
}
5758

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/network/DefaultSettingsSpiCallTest.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,13 @@ public void testHandleResponse_requestNotSuccessful() throws IOException {
186186

187187
assertNull(defaultSettingsSpiCall.handleResponse(mockHttpResponse));
188188
verify(mockHttpResponse, never()).body();
189-
verify(mockLogger, times(1)).e(eq("Failed to retrieve settings from " + TEST_URL));
189+
verify(mockLogger, times(1))
190+
.e(
191+
eq(
192+
"Settings request failed; (status: "
193+
+ HttpURLConnection.HTTP_INTERNAL_ERROR
194+
+ ") from "
195+
+ TEST_URL));
190196
}
191197

192198
public void testRequestWasSuccessful_successfulStatusCodes() {

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/CrashlyticsAnalyticsListener.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.firebase.analytics.connector.AnalyticsConnector.AnalyticsConnectorListener;
2121
import com.google.firebase.crashlytics.internal.Logger;
2222
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventReceiver;
23+
import java.util.Locale;
2324

2425
/**
2526
* Crashlytics listener for Firebase Analytics events. Processes incoming events and passes those
@@ -46,7 +47,10 @@ public void setBreadcrumbEventReceiver(@Nullable AnalyticsEventReceiver receiver
4647

4748
@Override
4849
public void onMessageTriggered(int id, @Nullable Bundle extras) {
49-
Logger.getLogger().d("Received Analytics message: " + id + " " + extras);
50+
Logger.getLogger()
51+
.v(
52+
String.format(
53+
Locale.US, "Analytics listener received message. ID: %d, Extras: %s", id, extras));
5054

5155
if (extras == null) {
5256
return;

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ public class FirebaseCrashlytics {
9292

9393
if (analyticsConnector != null) {
9494
// If FA is available, create a logger to log events from the Crashlytics origin.
95-
Logger.getLogger().d("Firebase Analytics is available.");
9695
final CrashlyticsOriginAnalyticsEventLogger directAnalyticsEventLogger =
9796
new CrashlyticsOriginAnalyticsEventLogger(analyticsConnector);
9897

@@ -107,7 +106,7 @@ public class FirebaseCrashlytics {
107106
subscribeToAnalyticsEvents(analyticsConnector, crashlyticsAnalyticsListener);
108107

109108
if (analyticsConnectorHandle != null) {
110-
Logger.getLogger().d("Firebase Analytics listener registered successfully.");
109+
Logger.getLogger().d("Registered Firebase Analytics listener.");
111110
// Create the event receiver which will supply breadcrumb events to Crashlytics
112111
final BreadcrumbAnalyticsEventReceiver breadcrumbReceiver =
113112
new BreadcrumbAnalyticsEventReceiver();
@@ -132,7 +131,8 @@ public class FirebaseCrashlytics {
132131
// Set the blocking analytics event logger for Crashlytics.
133132
analyticsEventLogger = blockingAnalyticsEventLogger;
134133
} else {
135-
Logger.getLogger().d("Firebase Analytics listener registration failed.");
134+
Logger.getLogger()
135+
.w("Could not register Firebase Analytics listener; a listener is already registered.");
136136
// FA is enabled, but the listener was not registered successfully.
137137
// We cannot listen for breadcrumbs.
138138
breadcrumbSource = new DisabledBreadcrumbSource();
@@ -142,7 +142,7 @@ public class FirebaseCrashlytics {
142142
}
143143
} else {
144144
// FA is entirely unavailable. We cannot listen for breadcrumbs or send events.
145-
Logger.getLogger().d("Firebase Analytics is unavailable.");
145+
Logger.getLogger().d("Firebase Analytics is not available.");
146146
breadcrumbSource = new DisabledBreadcrumbSource();
147147
analyticsEventLogger = new UnavailableAnalyticsEventLogger();
148148
}
@@ -170,11 +170,11 @@ public class FirebaseCrashlytics {
170170
appData =
171171
AppData.create(context, idManager, googleAppId, mappingFileId, unityVersionProvider);
172172
} catch (PackageManager.NameNotFoundException e) {
173-
Logger.getLogger().e("Could not retrieve app info, initialization failed.", e);
173+
Logger.getLogger().e("Error retrieving app package info.", e);
174174
return null;
175175
}
176176

177-
Logger.getLogger().d("Installer package name is: " + appData.installerPackageName);
177+
Logger.getLogger().v("Installer package name is: " + appData.installerPackageName);
178178

179179
final ExecutorService threadPoolExecutor =
180180
ExecutorUtils.buildSingleThreadExecutorService("com.google.firebase.crashlytics.startup");
@@ -282,7 +282,7 @@ public static FirebaseCrashlytics getInstance() {
282282
*/
283283
public void recordException(@NonNull Throwable throwable) {
284284
if (throwable == null) { // Users could call this with null despite the annotation.
285-
Logger.getLogger().w("Crashlytics is ignoring a request to log a null exception.");
285+
Logger.getLogger().w("A null value was passed to recordException. Ignoring.");
286286
return;
287287
}
288288
core.logException(throwable);

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/analytics/BlockingAnalyticsEventLogger.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,25 @@ public BlockingAnalyticsEventLogger(
5151
@Override
5252
public void logEvent(@NonNull String name, @Nullable Bundle params) {
5353
synchronized (latchLock) {
54-
Logger.getLogger().d("Logging Crashlytics event to Firebase");
54+
Logger.getLogger()
55+
.v("Logging event " + name + " to Firebase Analytics with params " + params);
5556
this.eventLatch = new CountDownLatch(1);
5657
this.callbackReceived = false;
5758

5859
baseAnalyticsEventLogger.logEvent(name, params);
5960

60-
Logger.getLogger().d("Awaiting app exception callback from FA...");
61+
Logger.getLogger().v("Awaiting app exception callback from Analytics...");
6162
try {
6263
if (eventLatch.await(timeout, timeUnit)) {
6364
callbackReceived = true;
64-
Logger.getLogger().d("App exception callback received from FA listener.");
65+
Logger.getLogger().v("App exception callback received from Analytics listener.");
6566
} else {
6667
Logger.getLogger()
67-
.d("Timeout exceeded while awaiting app exception callback from FA listener.");
68+
.w("Timeout exceeded while awaiting app exception callback from Analytics listener.");
6869
}
6970
} catch (InterruptedException ie) {
70-
Logger.getLogger().d("Interrupted while awaiting app exception callback from FA listener.");
71+
Logger.getLogger()
72+
.e("Interrupted while awaiting app exception callback from Analytics listener.");
7173
}
7274

7375
this.eventLatch = null;

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ static Architecture getValue() {
158158
String arch = Build.CPU_ABI;
159159

160160
if (TextUtils.isEmpty(arch)) {
161-
Logger.getLogger().d("Architecture#getValue()::Build.CPU_ABI returned null or empty");
161+
Logger.getLogger().v("Architecture#getValue()::Build.CPU_ABI returned null or empty");
162162
return UNKNOWN;
163163
}
164164

@@ -195,7 +195,7 @@ public static synchronized long getTotalRamInBytes() {
195195
// leave in this handling since we don't know for certain it isn't.
196196
bytes = convertMemInfoToBytes(result, "GB", BYTES_IN_A_GIGABYTE);
197197
} else {
198-
Logger.getLogger().d("Unexpected meminfo format while computing RAM: " + result);
198+
Logger.getLogger().w("Unexpected meminfo format while computing RAM: " + result);
199199
}
200200
} catch (NumberFormatException e) {
201201
Logger.getLogger().e("Unexpected meminfo format while computing RAM: " + result, e);
@@ -606,7 +606,7 @@ public static String resolveUnityEditorVersion(Context context) {
606606
final int id = CommonUtils.getResourcesIdentifier(context, UNITY_EDITOR_VERSION, "string");
607607
if (id != 0) {
608608
version = context.getResources().getString(id);
609-
Logger.getLogger().d("Unity Editor version is: " + version);
609+
Logger.getLogger().v("Unity Editor version is: " + version);
610610
}
611611
return version;
612612
}

0 commit comments

Comments
 (0)