Skip to content

Clean up and clarify Crashlytics logging. #2362

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import java.util.UUID;
import junit.framework.TestCase;

public class BreakpadControllerTest extends TestCase {
public class CrashpadControllerTest extends TestCase {

private BreakpadController controller;
private CrashpadController controller;

private Context context;
private NativeApi mockNativeApi;
Expand All @@ -40,7 +40,7 @@ protected void setUp() throws Exception {
context = ApplicationProvider.getApplicationContext();
mockNativeApi = mock(NativeApi.class);
mockFilesManager = mock(CrashFilesManager.class);
controller = new BreakpadController(context, mockNativeApi, mockFilesManager);
controller = new CrashpadController(context, mockNativeApi, mockFilesManager);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;

class BreakpadController implements NativeComponentController {
class CrashpadController implements NativeComponentController {

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

BreakpadController(Context context, NativeApi nativeApi, CrashFilesManager filesManager) {
CrashpadController(Context context, NativeApi nativeApi, CrashFilesManager filesManager) {
this.context = context;
this.nativeApi = nativeApi;
this.filesManager = filesManager;
Expand All @@ -55,7 +55,7 @@ public boolean initialize(String sessionId) {
initSuccess = nativeApi.initialize(crashReportPath, context.getAssets());
}
} catch (IOException e) {
Logger.getLogger().e("Error initializing CrashlyticsNdk", e);
Logger.getLogger().e("Error initializing Crashlytics NDK", e);
}
return initSuccess;
}
Expand All @@ -82,12 +82,14 @@ public SessionFiles getFilesForSession(String sessionId) {
final File sessionFileDirectoryForMinidump = new File(sessionFileDirectory, "pending");

Logger.getLogger()
.d("Minidump directory: " + sessionFileDirectoryForMinidump.getAbsolutePath());
.v("Minidump directory: " + sessionFileDirectoryForMinidump.getAbsolutePath());

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

Logger.getLogger()
.d("Minidump " + (minidump != null && minidump.exists() ? "exists" : "does not exist"));
.v(
"Minidump file "
+ (minidump != null && minidump.exists() ? "exists" : "does not exist"));

final SessionFiles.Builder builder = new SessionFiles.Builder();
if (sessionFileDirectory != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static FirebaseCrashlyticsNdk create(@NonNull Context context) {
final File rootDir = new File(context.getFilesDir(), FILES_PATH);

final NativeComponentController controller =
new BreakpadController(
new CrashpadController(
context, new JniNativeApi(context), new NdkCrashFilesManager(rootDir));
return new FirebaseCrashlyticsNdk(controller);
}
Expand All @@ -50,8 +50,9 @@ public boolean hasCrashDataForSession(@NonNull String sessionId) {
@Override
public boolean openSession(String sessionId) {
final boolean initSuccess = controller.initialize(sessionId);
Logger.getLogger()
.i("Crashlytics NDK initialization " + (initSuccess ? "successful" : "FAILED"));
if (!initSuccess) {
Logger.getLogger().w("Failed to initialize Crashlytics NDK for session " + sessionId);
}
return initSuccess;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,13 @@ public void testHandleResponse_requestNotSuccessful() throws IOException {

assertNull(defaultSettingsSpiCall.handleResponse(mockHttpResponse));
verify(mockHttpResponse, never()).body();
verify(mockLogger, times(1)).e(eq("Failed to retrieve settings from " + TEST_URL));
verify(mockLogger, times(1))
.e(
eq(
"Settings request failed; (status: "
+ HttpURLConnection.HTTP_INTERNAL_ERROR
+ ") from "
+ TEST_URL));
}

public void testRequestWasSuccessful_successfulStatusCodes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.firebase.analytics.connector.AnalyticsConnector.AnalyticsConnectorListener;
import com.google.firebase.crashlytics.internal.Logger;
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventReceiver;
import java.util.Locale;

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

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

if (extras == null) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public class FirebaseCrashlytics {

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

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

if (analyticsConnectorHandle != null) {
Logger.getLogger().d("Firebase Analytics listener registered successfully.");
Logger.getLogger().d("Registered Firebase Analytics listener.");
// Create the event receiver which will supply breadcrumb events to Crashlytics
final BreadcrumbAnalyticsEventReceiver breadcrumbReceiver =
new BreadcrumbAnalyticsEventReceiver();
Expand All @@ -132,7 +131,8 @@ public class FirebaseCrashlytics {
// Set the blocking analytics event logger for Crashlytics.
analyticsEventLogger = blockingAnalyticsEventLogger;
} else {
Logger.getLogger().d("Firebase Analytics listener registration failed.");
Logger.getLogger()
.w("Could not register Firebase Analytics listener; a listener is already registered.");
// FA is enabled, but the listener was not registered successfully.
// We cannot listen for breadcrumbs.
breadcrumbSource = new DisabledBreadcrumbSource();
Expand All @@ -142,7 +142,7 @@ public class FirebaseCrashlytics {
}
} else {
// FA is entirely unavailable. We cannot listen for breadcrumbs or send events.
Logger.getLogger().d("Firebase Analytics is unavailable.");
Logger.getLogger().d("Firebase Analytics is not available.");
breadcrumbSource = new DisabledBreadcrumbSource();
analyticsEventLogger = new UnavailableAnalyticsEventLogger();
}
Expand Down Expand Up @@ -170,11 +170,11 @@ public class FirebaseCrashlytics {
appData =
AppData.create(context, idManager, googleAppId, mappingFileId, unityVersionProvider);
} catch (PackageManager.NameNotFoundException e) {
Logger.getLogger().e("Could not retrieve app info, initialization failed.", e);
Logger.getLogger().e("Error retrieving app package info.", e);
return null;
}

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

final ExecutorService threadPoolExecutor =
ExecutorUtils.buildSingleThreadExecutorService("com.google.firebase.crashlytics.startup");
Expand Down Expand Up @@ -282,7 +282,7 @@ public static FirebaseCrashlytics getInstance() {
*/
public void recordException(@NonNull Throwable throwable) {
if (throwable == null) { // Users could call this with null despite the annotation.
Logger.getLogger().w("Crashlytics is ignoring a request to log a null exception.");
Logger.getLogger().w("A null value was passed to recordException. Ignoring.");
return;
}
core.logException(throwable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,25 @@ public BlockingAnalyticsEventLogger(
@Override
public void logEvent(@NonNull String name, @Nullable Bundle params) {
synchronized (latchLock) {
Logger.getLogger().d("Logging Crashlytics event to Firebase");
Logger.getLogger()
.v("Logging event " + name + " to Firebase Analytics with params " + params);
this.eventLatch = new CountDownLatch(1);
this.callbackReceived = false;

baseAnalyticsEventLogger.logEvent(name, params);

Logger.getLogger().d("Awaiting app exception callback from FA...");
Logger.getLogger().v("Awaiting app exception callback from Analytics...");
try {
if (eventLatch.await(timeout, timeUnit)) {
callbackReceived = true;
Logger.getLogger().d("App exception callback received from FA listener.");
Logger.getLogger().v("App exception callback received from Analytics listener.");
} else {
Logger.getLogger()
.d("Timeout exceeded while awaiting app exception callback from FA listener.");
.w("Timeout exceeded while awaiting app exception callback from Analytics listener.");
}
} catch (InterruptedException ie) {
Logger.getLogger().d("Interrupted while awaiting app exception callback from FA listener.");
Logger.getLogger()
.e("Interrupted while awaiting app exception callback from Analytics listener.");
}

this.eventLatch = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ static Architecture getValue() {
String arch = Build.CPU_ABI;

if (TextUtils.isEmpty(arch)) {
Logger.getLogger().d("Architecture#getValue()::Build.CPU_ABI returned null or empty");
Logger.getLogger().v("Architecture#getValue()::Build.CPU_ABI returned null or empty");
return UNKNOWN;
}

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