From 71fff2694f16130ffba45fcfe86159dff2c6357f Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 20 Nov 2024 17:20:36 -0500 Subject: [PATCH 01/42] Initial implementation of recording a custom key value pair in recordException --- .../crashlytics/FirebaseCrashlytics.java | 18 +++++- .../common/CrashlyticsController.java | 9 ++- .../internal/common/CrashlyticsCore.java | 4 +- .../common/SessionReportingCoordinator.java | 55 ++++++++++++------- .../internal/metadata/EventMetadata.kt | 8 +++ 5 files changed, 69 insertions(+), 25 deletions(-) create mode 100644 firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index a585485d627..1275a4d8ff3 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -43,6 +43,7 @@ import com.google.firebase.remoteconfig.interop.FirebaseRemoteConfigInterop; import com.google.firebase.sessions.api.FirebaseSessionsDependencies; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutorService; /** @@ -202,11 +203,26 @@ public static FirebaseCrashlytics getInstance() { * @param throwable a {@link Throwable} to be recorded as a non-fatal event. */ public void recordException(@NonNull Throwable throwable) { + recordException(throwable, Map.of()); + } + + /** + * Records a non-fatal report to send to Crashlytics. + * + * @param throwable a {@link Throwable} to be recorded as a non-fatal event. + * @param userInfo a {@link Map} to add key value pairs to be recorded with the non fatal exception. + */ + public void recordException(@NonNull Throwable throwable, @NonNull Map userInfo) { if (throwable == null) { // Users could call this with null despite the annotation. Logger.getLogger().w("A null value was passed to recordException. Ignoring."); return; } - core.logException(throwable); + + if (userInfo == null) { // It's possible to set this to null even with the annotation. + userInfo = Map.of(); + } + + core.logException(throwable, userInfo); } /** diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index 7726d9e6924..9b5250ce064 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -35,6 +35,7 @@ import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsTasks; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; +import com.google.firebase.crashlytics.internal.metadata.EventMetadata; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -405,7 +406,10 @@ void writeToLog(final long timestamp, final String msg) { } /** Log a caught exception - write out Throwable as event section of protobuf */ - void writeNonFatalException(@NonNull final Thread thread, @NonNull final Throwable ex) { + void writeNonFatalException( + @NonNull final Thread thread, + @NonNull final Throwable ex, + @NonNull Map userInfo) { // Capture and close over the current time, so that we get the exact call time, // rather than the time at which the task executes. final long timestampMillis = System.currentTimeMillis(); @@ -417,7 +421,8 @@ void writeNonFatalException(@NonNull final Thread thread, @NonNull final Throwab Logger.getLogger().w("Tried to write a non-fatal exception while no session was open."); return; } - reportingCoordinator.persistNonFatalEvent(ex, thread, currentSessionId, timestampSeconds); + EventMetadata eventMetadata = new EventMetadata(currentSessionId, timestampSeconds, userInfo); + reportingCoordinator.persistNonFatalEvent(ex, thread, eventMetadata); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java index 8e451762642..eaccd9dd377 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java @@ -313,9 +313,9 @@ public static String getVersion() { * Throwable was thrown. The Throwable will always be processed on a background thread, so it is * safe to invoke this method from the main thread. */ - public void logException(@NonNull Throwable throwable) { + public void logException(@NonNull Throwable throwable, @NonNull Map userInfo) { crashlyticsWorkers.common.submit( - () -> controller.writeNonFatalException(Thread.currentThread(), throwable)); + () -> controller.writeNonFatalException(Thread.currentThread(), throwable, userInfo)); } /** diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 29e2301591a..7369fb5c175 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -25,6 +25,7 @@ import com.google.android.gms.tasks.Tasks; import com.google.firebase.crashlytics.internal.Logger; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; +import com.google.firebase.crashlytics.internal.metadata.EventMetadata; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -42,6 +43,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.SortedSet; @@ -124,13 +126,14 @@ public void onBeginSession(@NonNull String sessionId, long timestampSeconds) { public void persistFatalEvent( @NonNull Throwable event, @NonNull Thread thread, @NonNull String sessionId, long timestamp) { Logger.getLogger().v("Persisting fatal event for session " + sessionId); - persistEvent(event, thread, sessionId, EVENT_TYPE_CRASH, timestamp, true); + EventMetadata eventMetadata = new EventMetadata(sessionId, timestamp, Map.of()); + persistEvent(event, thread, EVENT_TYPE_CRASH, eventMetadata, true); } public void persistNonFatalEvent( - @NonNull Throwable event, @NonNull Thread thread, @NonNull String sessionId, long timestamp) { - Logger.getLogger().v("Persisting non-fatal event for session " + sessionId); - persistEvent(event, thread, sessionId, EVENT_TYPE_LOGGED, timestamp, false); + @NonNull Throwable event, @NonNull Thread thread, @NonNull EventMetadata eventMetadata) { + Logger.getLogger().v("Persisting non-fatal event for session " + eventMetadata.getSessionId()); + persistEvent(event, thread, EVENT_TYPE_LOGGED, eventMetadata, false); } @RequiresApi(api = Build.VERSION_CODES.R) @@ -253,23 +256,20 @@ private CrashlyticsReportWithSessionId ensureHasFid(CrashlyticsReportWithSession } private CrashlyticsReport.Session.Event addMetaDataToEvent( - CrashlyticsReport.Session.Event capturedEvent) { + CrashlyticsReport.Session.Event capturedEvent, Map eventCustomKeys) { CrashlyticsReport.Session.Event eventWithLogsAndCustomKeys = - addLogsAndCustomKeysToEvent(capturedEvent, logFileManager, reportMetadata); + addLogsCustomKeysAndEventKeysToEvent( + capturedEvent, logFileManager, reportMetadata, eventCustomKeys); CrashlyticsReport.Session.Event eventWithRollouts = addRolloutsStateToEvent(eventWithLogsAndCustomKeys, reportMetadata); return eventWithRollouts; } - private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent( - CrashlyticsReport.Session.Event capturedEvent) { - return addLogsAndCustomKeysToEvent(capturedEvent, logFileManager, reportMetadata); - } - - private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent( + private CrashlyticsReport.Session.Event addLogsCustomKeysAndEventKeysToEvent( CrashlyticsReport.Session.Event capturedEvent, LogFileManager logFileManager, - UserMetadata reportMetadata) { + UserMetadata reportMetadata, + Map eventKeys) { final CrashlyticsReport.Session.Event.Builder eventBuilder = capturedEvent.toBuilder(); final String content = logFileManager.getLogString(); @@ -283,12 +283,18 @@ private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent( // TODO: Put this back once support for reports endpoint is removed. // logFileManager.clearLog(); // Clear log to prepare for next event. + // Merges the app level custom keys, with the event specific custom keys. + Map customKeysMergedWithEventKeys = + new HashMap<>(reportMetadata.getCustomKeys()); + customKeysMergedWithEventKeys.putAll(eventKeys); + final List sortedCustomAttributes = - getSortedCustomAttributes(reportMetadata.getCustomKeys()); + getSortedCustomAttributes(customKeysMergedWithEventKeys); final List sortedInternalKeys = getSortedCustomAttributes(reportMetadata.getInternalKeys()); if (!sortedCustomAttributes.isEmpty() || !sortedInternalKeys.isEmpty()) { + // TODO(b/380072776): Change implementation to *not* override existing custom attributes. eventBuilder.setApp( capturedEvent.getApp().toBuilder() .setCustomAttributes(sortedCustomAttributes) @@ -299,6 +305,14 @@ private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent( return eventBuilder.build(); } + private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent( + CrashlyticsReport.Session.Event capturedEvent, + LogFileManager logFileManager, + UserMetadata reportMetadata) { + return addLogsCustomKeysAndEventKeysToEvent( + capturedEvent, logFileManager, reportMetadata, Map.of()); + } + private CrashlyticsReport.Session.Event addRolloutsStateToEvent( CrashlyticsReport.Session.Event capturedEvent, UserMetadata reportMetadata) { List reportRolloutAssignments = @@ -319,9 +333,8 @@ private CrashlyticsReport.Session.Event addRolloutsStateToEvent( private void persistEvent( @NonNull Throwable event, @NonNull Thread thread, - @NonNull String sessionId, @NonNull String eventType, - long timestamp, + @NonNull EventMetadata eventMetadata, boolean isFatal) { final boolean isHighPriority = eventType.equals(EVENT_TYPE_CRASH); @@ -331,23 +344,25 @@ private void persistEvent( event, thread, eventType, - timestamp, + eventMetadata.getTimestamp(), EVENT_THREAD_IMPORTANCE, MAX_CHAINED_EXCEPTION_DEPTH, isFatal); - CrashlyticsReport.Session.Event finallizedEvent = addMetaDataToEvent(capturedEvent); + CrashlyticsReport.Session.Event finallizedEvent = + addMetaDataToEvent(capturedEvent, eventMetadata.getUserInfo()); // Non-fatal, persistence write task we move to diskWriteWorker if (!isFatal) { crashlyticsWorkers.diskWrite.submit( () -> { Logger.getLogger().d("disk worker: log non-fatal event to persistence"); - reportPersistence.persistEvent(finallizedEvent, sessionId, isHighPriority); + reportPersistence.persistEvent( + finallizedEvent, eventMetadata.getSessionId(), isHighPriority); }); return; } - reportPersistence.persistEvent(finallizedEvent, sessionId, isHighPriority); + reportPersistence.persistEvent(finallizedEvent, eventMetadata.getSessionId(), isHighPriority); } private boolean onReportSendComplete(@NonNull Task task) { diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt new file mode 100644 index 00000000000..61c26f463c6 --- /dev/null +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -0,0 +1,8 @@ +package com.google.firebase.crashlytics.internal.metadata + +/** A class that represents information to attach to a specific event. */ +data class EventMetadata( + val sessionId: String, + val timestamp: Long, + val userInfo: Map = mapOf() +) From 981a51eeebb5d4ca3edbc3a096d58adea73ecc6a Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 09:36:38 -0500 Subject: [PATCH 02/42] Fix unit tests affected by EventMetadata --- .../common/CrashlyticsControllerTest.java | 8 +++++--- .../SessionReportingCoordinatorTest.java | 19 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java index 82cd14bcf74..9260a7f7950 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java @@ -44,6 +44,7 @@ import com.google.firebase.crashlytics.internal.NativeSessionFileProvider; import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; +import com.google.firebase.crashlytics.internal.metadata.EventMetadata; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -57,6 +58,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.TreeSet; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -216,14 +218,14 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal() when(mockSessionReportingCoordinator.listSortedOpenSessionIds()) .thenReturn(new TreeSet<>(Collections.singleton(sessionId))); - controller.writeNonFatalException(thread, nonFatal); + controller.writeNonFatalException(thread, nonFatal, Map.of()); crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(testSettingsProvider)); crashlyticsWorkers.common.await(); crashlyticsWorkers.diskWrite.await(); verify(mockSessionReportingCoordinator) - .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong()); + .persistNonFatalEvent(eq(nonFatal), eq(thread), any(EventMetadata.class)); } @SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo @@ -377,7 +379,7 @@ public void testLoggedExceptionsAfterCrashOk() { testSettingsProvider, Thread.currentThread(), new RuntimeException()); // This should not throw. - controller.writeNonFatalException(Thread.currentThread(), new RuntimeException()); + controller.writeNonFatalException(Thread.currentThread(), new RuntimeException(), Map.of()); } /** diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java index f1015447441..e04fb8f943e 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java @@ -35,6 +35,7 @@ import com.google.android.gms.tasks.Tasks; import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; +import com.google.firebase.crashlytics.internal.metadata.EventMetadata; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -138,7 +139,8 @@ public void testNonFatalEvent_persistsNormalPriorityEventWithoutAllThreadsForSes mockEventInteractions(); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); @@ -163,7 +165,8 @@ public void testNonFatalEvent_addsLogsToEvent() throws Exception { when(logFileManager.getLogString()).thenReturn(testLog); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); @@ -184,7 +187,8 @@ public void testNonFatalEvent_addsNoLogsToEventWhenNoneAvailable() throws Except when(logFileManager.getLogString()).thenReturn(null); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); @@ -261,7 +265,8 @@ public void testNonFatalEvent_addsSortedKeysToEvent() throws Exception { when(reportMetadata.getInternalKeys()).thenReturn(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); @@ -286,7 +291,8 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() throws Except when(reportMetadata.getCustomKeys()).thenReturn(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); @@ -309,7 +315,8 @@ public void testNonFatalEvent_addRolloutsEvent() throws Exception { when(reportMetadata.getRolloutsState()).thenReturn(rolloutsState); reportingCoordinator.onBeginSession(sessionId, timestamp); - reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); crashlyticsWorkers.diskWrite.await(); From 81fd3fa7f1d217c470112ee6c8fefa31ae7a446d Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 09:40:12 -0500 Subject: [PATCH 03/42] Add copyright header to EventMetadata --- .../crashlytics/internal/metadata/EventMetadata.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index 61c26f463c6..1ddd1f0316a 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -1,3 +1,17 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package com.google.firebase.crashlytics.internal.metadata /** A class that represents information to attach to a specific event. */ From 2e392549e1ae8d53aaba0dc3f86d1005dccd65fe Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 10:01:53 -0500 Subject: [PATCH 04/42] Update api.txt file --- firebase-crashlytics/api.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/api.txt b/firebase-crashlytics/api.txt index 3fb22e31b6e..c7102d05e75 100644 --- a/firebase-crashlytics/api.txt +++ b/firebase-crashlytics/api.txt @@ -20,10 +20,11 @@ package com.google.firebase.crashlytics { method public void deleteUnsentReports(); method public boolean didCrashOnPreviousExecution(); method @NonNull public static com.google.firebase.crashlytics.FirebaseCrashlytics getInstance(); + method public boolean isCrashlyticsCollectionEnabled(); method public void log(@NonNull String); method public void recordException(@NonNull Throwable); + method public void recordException(@NonNull Throwable, @NonNull java.util.Map); method public void sendUnsentReports(); - method public boolean isCrashlyticsCollectionEnabled(); method public void setCrashlyticsCollectionEnabled(boolean); method public void setCrashlyticsCollectionEnabled(@Nullable Boolean); method public void setCustomKey(@NonNull String, boolean); From f6196e2bf4a1c9ae76657f9647f259d39b80c645 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 10:13:19 -0500 Subject: [PATCH 05/42] Add a JvmOverload in EventMetadata to use the default value for userInfo --- .../internal/common/SessionReportingCoordinator.java | 2 +- .../firebase/crashlytics/internal/metadata/EventMetadata.kt | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 7369fb5c175..d067f8a0455 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -126,7 +126,7 @@ public void onBeginSession(@NonNull String sessionId, long timestampSeconds) { public void persistFatalEvent( @NonNull Throwable event, @NonNull Thread thread, @NonNull String sessionId, long timestamp) { Logger.getLogger().v("Persisting fatal event for session " + sessionId); - EventMetadata eventMetadata = new EventMetadata(sessionId, timestamp, Map.of()); + EventMetadata eventMetadata = new EventMetadata(sessionId, timestamp); persistEvent(event, thread, EVENT_TYPE_CRASH, eventMetadata, true); } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index 1ddd1f0316a..a97c2dc3fd8 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -15,7 +15,9 @@ package com.google.firebase.crashlytics.internal.metadata /** A class that represents information to attach to a specific event. */ -data class EventMetadata( +data class EventMetadata +@JvmOverloads +constructor( val sessionId: String, val timestamp: Long, val userInfo: Map = mapOf() From 38a4a261634ed76a5486e13764f9195d3d094271 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 10:57:11 -0500 Subject: [PATCH 06/42] Add a unit test to verify the behavior of userInfo keys --- .../SessionReportingCoordinatorTest.java | 101 ++++++++++++++++-- 1 file changed, 95 insertions(+), 6 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java index e04fb8f943e..33bfe75f7c6 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java @@ -140,7 +140,7 @@ public void testNonFatalEvent_persistsNormalPriorityEventWithoutAllThreadsForSes reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); @@ -166,7 +166,7 @@ public void testNonFatalEvent_addsLogsToEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); @@ -188,7 +188,7 @@ public void testNonFatalEvent_addsNoLogsToEventWhenNoneAvailable() throws Except reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); @@ -266,7 +266,7 @@ public void testNonFatalEvent_addsSortedKeysToEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); @@ -292,7 +292,7 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() throws Except reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); @@ -303,6 +303,95 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() throws Except verify(logFileManager, never()).clearLog(); } + @Test + public void testNonFatalEvent_addsUserInfoKeysToEventWhenAvailable() throws Exception { + final long timestamp = System.currentTimeMillis(); + + mockEventInteractions(); + + final String sessionId = "testSessionId"; + + final Map attributes = Collections.emptyMap(); + when(reportMetadata.getCustomKeys()).thenReturn(attributes); + + final String testKey1 = "testKey1"; + final String testValue1 = "testValue1"; + + final Map userInfo = new HashMap<>(); + userInfo.put(testKey1, testValue1); + + final CustomAttribute customAttribute1 = + CustomAttribute.builder().setKey(testKey1).setValue(testValue1).build(); + + final List expectedCustomAttributes = new ArrayList<>(); + expectedCustomAttributes.add(customAttribute1); + + reportingCoordinator.onBeginSession(sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, userInfo)); + + crashlyticsWorkers.diskWrite.await(); + + verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes); + verify(mockEventAppBuilder).build(); + verify(mockEventBuilder).setApp(mockEventApp); + verify(mockEventBuilder).build(); + verify(logFileManager, never()).clearLog(); + } + + @Test + public void testNonFatalEvent_mergesUserInfoKeysWithCustomKeys() throws Exception { + final long timestamp = System.currentTimeMillis(); + + mockEventInteractions(); + + final String sessionId = "testSessionId"; + + final String testKey1 = "testKey1"; + final String testValue1 = "testValue1"; + + final String testKey2 = "testKey2"; + final String testValue2 = "testValue2"; + + final Map attributes = new HashMap<>(); + attributes.put(testKey1, testValue1); + attributes.put(testKey2, testValue2); + + when(reportMetadata.getCustomKeys()).thenReturn(attributes); + + final String testValue1UserInfo = "testValue1"; + final String testKey3 = "testKey3"; + final String testValue3 = "testValue3"; + + final Map userInfo = new HashMap<>(); + userInfo.put(testKey1, testValue1UserInfo); + userInfo.put(testKey3, testValue3); + + final CustomAttribute customAttribute1 = + CustomAttribute.builder().setKey(testKey1).setValue(testValue1UserInfo).build(); + final CustomAttribute customAttribute2 = + CustomAttribute.builder().setKey(testKey2).setValue(testValue2).build(); + final CustomAttribute customAttribute3 = + CustomAttribute.builder().setKey(testKey3).setValue(testValue3).build(); + + final List expectedCustomAttributes = new ArrayList<>(); + expectedCustomAttributes.add(customAttribute1); + expectedCustomAttributes.add(customAttribute2); + expectedCustomAttributes.add(customAttribute3); + + reportingCoordinator.onBeginSession(sessionId, timestamp); + reportingCoordinator.persistNonFatalEvent( + mockException, mockThread, new EventMetadata(sessionId, timestamp, userInfo)); + + crashlyticsWorkers.diskWrite.await(); + + verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes); + verify(mockEventAppBuilder).build(); + verify(mockEventBuilder).setApp(mockEventApp); + verify(mockEventBuilder).build(); + verify(logFileManager, never()).clearLog(); + } + @Test public void testNonFatalEvent_addRolloutsEvent() throws Exception { long timestamp = System.currentTimeMillis(); @@ -316,7 +405,7 @@ public void testNonFatalEvent_addRolloutsEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( - mockException, mockThread, new EventMetadata(sessionId, timestamp, Map.of())); + mockException, mockThread, new EventMetadata(sessionId, timestamp)); crashlyticsWorkers.diskWrite.await(); From 7c422bb80da1c2f3f208aed7b7ed7296fbb5499c Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 11:03:25 -0500 Subject: [PATCH 07/42] Update documentation of EventMetadata --- .../crashlytics/internal/metadata/EventMetadata.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index a97c2dc3fd8..97dd7f116b3 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -14,7 +14,13 @@ package com.google.firebase.crashlytics.internal.metadata -/** A class that represents information to attach to a specific event. */ +/** + * A class that represents information to attach to a specific event. + * + * @property sessionId the sessionId to attach to the event. + * @property timestamp the timestamp to attach to the event. + * @property userInfo a [Map] of key value pairs to attach to the event. + */ data class EventMetadata @JvmOverloads constructor( From 9f99b4412e0cbc3b7a72e3f3a28b69ce2171157b Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 11:11:17 -0500 Subject: [PATCH 08/42] Remove TODO --- .../crashlytics/internal/common/SessionReportingCoordinator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index d067f8a0455..7514cf4c902 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -294,7 +294,6 @@ private CrashlyticsReport.Session.Event addLogsCustomKeysAndEventKeysToEvent( getSortedCustomAttributes(reportMetadata.getInternalKeys()); if (!sortedCustomAttributes.isEmpty() || !sortedInternalKeys.isEmpty()) { - // TODO(b/380072776): Change implementation to *not* override existing custom attributes. eventBuilder.setApp( capturedEvent.getApp().toBuilder() .setCustomAttributes(sortedCustomAttributes) From bb3aae2bd4780cb089d1522b653f55b468b28c51 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 11:39:30 -0500 Subject: [PATCH 09/42] Additional documentation for EventMetadata --- .../firebase/crashlytics/internal/metadata/EventMetadata.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index 97dd7f116b3..1bca65a26f8 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -19,7 +19,8 @@ package com.google.firebase.crashlytics.internal.metadata * * @property sessionId the sessionId to attach to the event. * @property timestamp the timestamp to attach to the event. - * @property userInfo a [Map] of key value pairs to attach to the event. + * @property userInfo a [Map] of key value pairs to attach to the event, in addition + * to the global custom keys. */ data class EventMetadata @JvmOverloads From c02234e865ab6a199bc501d2a1afee6b8cb5ba59 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 21 Nov 2024 11:39:46 -0500 Subject: [PATCH 10/42] Add details to CHANGELOG --- firebase-crashlytics/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index ce71357e836..6c358e13a74 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased - +* [feature] Added an overload to the `recordException` API to allow attaching additional custom + keys to the non fatal report. # 19.2.1 * [changed] Updated protobuf dependency to `3.25.5` to fix From 5fdeade8c675d715033d3987853d552fcde8e6a3 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 25 Nov 2024 16:38:44 -0500 Subject: [PATCH 11/42] Refactor the merging of event and global keys into a method, and add TODOs --- .../common/SessionReportingCoordinator.java | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 7514cf4c902..3be2552c032 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -283,13 +283,12 @@ private CrashlyticsReport.Session.Event addLogsCustomKeysAndEventKeysToEvent( // TODO: Put this back once support for reports endpoint is removed. // logFileManager.clearLog(); // Clear log to prepare for next event. - // Merges the app level custom keys, with the event specific custom keys. - Map customKeysMergedWithEventKeys = - new HashMap<>(reportMetadata.getCustomKeys()); - customKeysMergedWithEventKeys.putAll(eventKeys); + // Merges the app level custom keys and the event specific custom keys. + Map customKeysForEvent = + mergeEventKeysIntoCustomKeys(reportMetadata.getCustomKeys(), eventKeys); final List sortedCustomAttributes = - getSortedCustomAttributes(customKeysMergedWithEventKeys); + getSortedCustomAttributes(customKeysForEvent); final List sortedInternalKeys = getSortedCustomAttributes(reportMetadata.getInternalKeys()); @@ -382,6 +381,17 @@ private boolean onReportSendComplete(@NonNull Task mergeEventKeysIntoCustomKeys( + Map globalCustomKeys, Map eventKeys) { + Map result = new HashMap<>(globalCustomKeys); + result.putAll(eventKeys); + + // TODO: Add 64 key restriction. + // TODO: Add 1024 character restriction. + return result; + } + @NonNull private static List getSortedCustomAttributes( @NonNull Map attributes) { From dfaf14dfc9fb4b6b5a2398e44ed08adb1ab9ff93 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 25 Nov 2024 17:19:58 -0500 Subject: [PATCH 12/42] Add a TODO --- .../google/firebase/crashlytics/internal/metadata/KeysMap.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java index 620c7dca9d9..c3286010621 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java @@ -19,6 +19,7 @@ import com.google.firebase.crashlytics.internal.common.CommonUtils; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; /** Handles any key/values for metadata. */ @@ -54,6 +55,7 @@ public synchronized boolean setKey(String key, String value) { keys.put(sanitizedKey, value == null ? "" : santitizedAttribute); return true; } + // TODO: Explore using a LinkedHashMap to overwrite keys in this case. Logger.getLogger() .w( "Ignored entry \"" From 8a3b6e9066e9165cdcc25ec681c1755f8e575104 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 10:26:29 -0500 Subject: [PATCH 13/42] Revert CHANGELOG --- firebase-crashlytics/CHANGELOG.md | 61 ------------------------------- 1 file changed, 61 deletions(-) diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 6c358e13a74..e954b6dc972 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,71 +1,10 @@ # Unreleased -* [feature] Added an overload to the `recordException` API to allow attaching additional custom - keys to the non fatal report. -# 19.2.1 -* [changed] Updated protobuf dependency to `3.25.5` to fix - [CVE-2024-7254](https://nvd.nist.gov/vuln/detail/CVE-2024-7254). - - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-crashlytics` library. The Kotlin extensions library has no additional -updates. - -# 19.2.0 -* [fixed] Improved data consistency for rapid user actions. -* [fixed] Fixed exception propagation in the case of no default uncaught exception handler. -* [changed] Internal changes to improve startup time. -* [changed] Internal changes to the way background tasks are scheduled. -* [changed] Migrated SDK to use standard Firebase executors. - -# 19.1.0 -* [feature] Added the `isCrashlyticsCollectionEnabled` API to check if Crashlytics collection is - enabled. - (GitHub [#5919](https://github.com/firebase/firebase-android-sdk/issues/5919){: .external}) -* [fixed] Ensure that on-demand fatal events are never processed on the main thread. - (GitHub [#4345](https://github.com/firebase/firebase-android-sdk/issues/4345){: .external}) -* [changed] Internal changes to the way session IDs are generated. - - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-crashlytics` library. The Kotlin extensions library has no additional -updates. - -# 19.0.3 -* [changed] Update the internal file system to handle long file names. - - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-crashlytics` library. The Kotlin extensions library has no additional -updates. - -# 19.0.2 -* [changed] Changing caught exception type to fail safely on any exception type. - - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-crashlytics` library. The Kotlin extensions library has no additional -updates. - -# 19.0.1 -* [changed] Improve cold initialization time. -* [fixed] Fixed version compatibility issues with other Firebase libraries. - - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-crashlytics` library. The Kotlin extensions library has no additional -updates. # 19.0.0 * [fixed] Force validation or rotation of FIDs. * [fixed] Added keep rule for shrinkage of Crashlytics build resources in strict mode. - ## Kotlin The Kotlin extensions library transitively includes the updated `firebase-crashlytics` library. The Kotlin extensions library has no additional From 19fce3ef941d765cbeaca651eb0896f97986d7b8 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 10:39:16 -0500 Subject: [PATCH 14/42] Fix typo --- .../firebase/crashlytics/internal/metadata/KeysMap.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java index c3286010621..2008ce202e5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java @@ -48,11 +48,11 @@ public synchronized boolean setKey(String key, String value) { String sanitizedKey = sanitizeKey(key); // The entry can be added if we're under the size limit or we're updating an existing entry if (keys.size() < maxEntries || keys.containsKey(sanitizedKey)) { - String santitizedAttribute = sanitizeString(value, maxEntryLength); - if (CommonUtils.nullSafeEquals(keys.get(sanitizedKey), santitizedAttribute)) { + String sanitizedAttribute = sanitizeString(value, maxEntryLength); + if (CommonUtils.nullSafeEquals(keys.get(sanitizedKey), sanitizedAttribute)) { return false; } - keys.put(sanitizedKey, value == null ? "" : santitizedAttribute); + keys.put(sanitizedKey, value == null ? "" : sanitizedAttribute); return true; } // TODO: Explore using a LinkedHashMap to overwrite keys in this case. From 92e133a5603eef3364667b6ff5b494b74f2a5799 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 10:43:41 -0500 Subject: [PATCH 15/42] Fix CHANGELOG --- firebase-crashlytics/CHANGELOG.md | 60 +++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 96bf504a18f..28e901ab661 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,10 +1,70 @@ # Unreleased * [fixed] Execute failure listener outside the main thread [#6535] +# 19.2.1 +* [changed] Updated protobuf dependency to `3.25.5` to fix + [CVE-2024-7254](https://nvd.nist.gov/vuln/detail/CVE-2024-7254). + + +## Kotlin +The Kotlin extensions library transitively includes the updated +`firebase-crashlytics` library. The Kotlin extensions library has no additional +updates. + +# 19.2.0 +* [fixed] Improved data consistency for rapid user actions. +* [fixed] Fixed exception propagation in the case of no default uncaught exception handler. +* [changed] Internal changes to improve startup time. +* [changed] Internal changes to the way background tasks are scheduled. +* [changed] Migrated SDK to use standard Firebase executors. + +# 19.1.0 +* [feature] Added the `isCrashlyticsCollectionEnabled` API to check if Crashlytics collection is + enabled. + (GitHub [#5919](https://github.com/firebase/firebase-android-sdk/issues/5919){: .external}) +* [fixed] Ensure that on-demand fatal events are never processed on the main thread. + (GitHub [#4345](https://github.com/firebase/firebase-android-sdk/issues/4345){: .external}) +* [changed] Internal changes to the way session IDs are generated. + + +## Kotlin +The Kotlin extensions library transitively includes the updated +`firebase-crashlytics` library. The Kotlin extensions library has no additional +updates. + +# 19.0.3 +* [changed] Update the internal file system to handle long file names. + + +## Kotlin +The Kotlin extensions library transitively includes the updated +`firebase-crashlytics` library. The Kotlin extensions library has no additional +updates. + +# 19.0.2 +* [changed] Changing caught exception type to fail safely on any exception type. + + +## Kotlin +The Kotlin extensions library transitively includes the updated +`firebase-crashlytics` library. The Kotlin extensions library has no additional +updates. + +# 19.0.1 +* [changed] Improve cold initialization time. +* [fixed] Fixed version compatibility issues with other Firebase libraries. + + +## Kotlin +The Kotlin extensions library transitively includes the updated +`firebase-crashlytics` library. The Kotlin extensions library has no additional +updates. + # 19.0.0 * [fixed] Force validation or rotation of FIDs. * [fixed] Added keep rule for shrinkage of Crashlytics build resources in strict mode. + ## Kotlin The Kotlin extensions library transitively includes the updated `firebase-crashlytics` library. The Kotlin extensions library has no additional From 4d58ecd2c77a1423d81c3e95d2b43f421a172050 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 13:24:34 -0500 Subject: [PATCH 16/42] unused import --- .../google/firebase/crashlytics/internal/metadata/KeysMap.java | 1 - 1 file changed, 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java index 2008ce202e5..e0aff943cff 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java @@ -19,7 +19,6 @@ import com.google.firebase.crashlytics.internal.common.CommonUtils; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.Map; /** Handles any key/values for metadata. */ From f4654080b97afdb19438192814a18ce0e5facf86 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 13:24:49 -0500 Subject: [PATCH 17/42] Refactor the method to merge event and global keys --- .../common/SessionReportingCoordinator.java | 18 +---------- .../internal/metadata/UserMetadata.java | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 3be2552c032..cdc837e5651 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -43,7 +43,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.SortedSet; @@ -283,12 +282,8 @@ private CrashlyticsReport.Session.Event addLogsCustomKeysAndEventKeysToEvent( // TODO: Put this back once support for reports endpoint is removed. // logFileManager.clearLog(); // Clear log to prepare for next event. - // Merges the app level custom keys and the event specific custom keys. - Map customKeysForEvent = - mergeEventKeysIntoCustomKeys(reportMetadata.getCustomKeys(), eventKeys); - final List sortedCustomAttributes = - getSortedCustomAttributes(customKeysForEvent); + getSortedCustomAttributes(reportMetadata.getCustomKeys(eventKeys)); final List sortedInternalKeys = getSortedCustomAttributes(reportMetadata.getInternalKeys()); @@ -381,17 +376,6 @@ private boolean onReportSendComplete(@NonNull Task mergeEventKeysIntoCustomKeys( - Map globalCustomKeys, Map eventKeys) { - Map result = new HashMap<>(globalCustomKeys); - result.putAll(eventKeys); - - // TODO: Add 64 key restriction. - // TODO: Add 1024 character restriction. - return result; - } - @NonNull private static List getSortedCustomAttributes( @NonNull Map attributes) { diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 36ca7fda8a5..c5cb9b6366f 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -17,10 +17,12 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.firebase.crashlytics.internal.Logger; import com.google.firebase.crashlytics.internal.common.CommonUtils; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; import com.google.firebase.crashlytics.internal.persistence.FileStore; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicMarkableReference; @@ -135,7 +137,37 @@ public void setUserId(String identifier) { crashlyticsWorkers.diskWrite.submit(this::serializeUserDataIfNeeded); } + /** Creates a {Map} with all the custom keys to attach to the event. + * + * @param eventKeys a {Map} representing event specific keys. + * + * @return the {Map} respecting the custom key constraints. + */ + public Map getCustomKeys(Map eventKeys) { + Map globalKeys = customKeys.getKeys(); + HashMap result = new HashMap<>(globalKeys); + for (Map.Entry entry : eventKeys.entrySet()) { + if (result.size() < MAX_ATTRIBUTES) { + String key = KeysMap.sanitizeString(entry.getKey(), MAX_ATTRIBUTE_SIZE); + String value = KeysMap.sanitizeString(entry.getValue(), MAX_ATTRIBUTE_SIZE); + result.put(key, value); + } + + // TODO: Explore using a LinkedHashMap to overwrite keys in this case. + long ignoredEventKeysCount = eventKeys.size() - (MAX_ATTRIBUTES - globalKeys.size()); + Logger.getLogger() + .w( + "Ignored " + + ignoredEventKeysCount + + " event specific keys. Maximum key count is " + + MAX_ATTRIBUTES); + break; + } + return result; + } + /** + * * @return defensive copy of the custom keys. */ public Map getCustomKeys() { From d0d7c53b1b27b7cffb96c424684c438206024f75 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 13:25:35 -0500 Subject: [PATCH 18/42] return unmodifiable map --- .../firebase/crashlytics/internal/metadata/UserMetadata.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index c5cb9b6366f..94d69dc414c 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -22,6 +22,8 @@ import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; import com.google.firebase.crashlytics.internal.persistence.FileStore; + +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -163,7 +165,7 @@ public Map getCustomKeys(Map eventKeys) { + MAX_ATTRIBUTES); break; } - return result; + return Collections.unmodifiableMap(result); } /** From 4e4f1bf09548478378c7c9afc2e8296faddfa896 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 13:44:07 -0500 Subject: [PATCH 19/42] Fix accidental new lines --- .../firebase/crashlytics/internal/metadata/UserMetadata.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 94d69dc414c..a06770067f4 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -22,7 +22,6 @@ import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; import com.google.firebase.crashlytics.internal.persistence.FileStore; - import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -169,7 +168,6 @@ public Map getCustomKeys(Map eventKeys) { } /** - * * @return defensive copy of the custom keys. */ public Map getCustomKeys() { From b0a3bfc1c7eb5a3583ed3acef1ada6c44d7a6b20 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 13:52:11 -0500 Subject: [PATCH 20/42] Add missing continue --- .../firebase/crashlytics/internal/metadata/UserMetadata.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index a06770067f4..d111ae070a5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -152,6 +152,8 @@ public Map getCustomKeys(Map eventKeys) { String key = KeysMap.sanitizeString(entry.getKey(), MAX_ATTRIBUTE_SIZE); String value = KeysMap.sanitizeString(entry.getValue(), MAX_ATTRIBUTE_SIZE); result.put(key, value); + + continue; } // TODO: Explore using a LinkedHashMap to overwrite keys in this case. From abd31d3f4398ae286dedef8813712700ea20a8ea Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 16:12:48 -0500 Subject: [PATCH 21/42] Update unit tests --- .../SessionReportingCoordinatorTest.java | 84 +++++++++---------- .../internal/metadata/UserMetadata.java | 10 +-- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java index 33bfe75f7c6..d7cdeafdf72 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java @@ -34,13 +34,16 @@ import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.concurrent.TestOnlyExecutors; +import com.google.firebase.crashlytics.internal.CrashlyticsTestCase; import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.EventMetadata; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; +import com.google.firebase.crashlytics.internal.metadata.RolloutAssignment; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport.CustomAttribute; import com.google.firebase.crashlytics.internal.persistence.CrashlyticsReportPersistence; +import com.google.firebase.crashlytics.internal.persistence.FileStore; import com.google.firebase.crashlytics.internal.send.DataTransportCrashlyticsReportSender; import java.io.File; import java.util.ArrayList; @@ -55,13 +58,14 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -public class SessionReportingCoordinatorTest { +public class SessionReportingCoordinatorTest extends CrashlyticsTestCase { + + private static String TEST_SESSION_ID = "testSession1"; @Mock private CrashlyticsReportDataCapture dataCapture; @Mock private CrashlyticsReportPersistence reportPersistence; @Mock private DataTransportCrashlyticsReportSender reportSender; @Mock private LogFileManager logFileManager; - @Mock private UserMetadata reportMetadata; @Mock private IdManager idManager; @Mock private CrashlyticsReport mockReport; @Mock private CrashlyticsReport.Session.Event mockEvent; @@ -71,6 +75,7 @@ public class SessionReportingCoordinatorTest { @Mock private Exception mockException; @Mock private Thread mockThread; + private UserMetadata reportMetadata; private SessionReportingCoordinator reportingCoordinator; private AutoCloseable mocks; @@ -81,6 +86,9 @@ public class SessionReportingCoordinatorTest { public void setUp() { mocks = MockitoAnnotations.openMocks(this); + FileStore testFileStore = new FileStore(getContext()); + reportMetadata = new UserMetadata(TEST_SESSION_ID, testFileStore, crashlyticsWorkers); + reportingCoordinator = new SessionReportingCoordinator( dataCapture, @@ -261,8 +269,7 @@ public void testNonFatalEvent_addsSortedKeysToEvent() throws Exception { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - when(reportMetadata.getCustomKeys()).thenReturn(attributes); - when(reportMetadata.getInternalKeys()).thenReturn(attributes); + addKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( @@ -286,10 +293,6 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() throws Except final String sessionId = "testSessionId"; - final Map attributes = Collections.emptyMap(); - - when(reportMetadata.getCustomKeys()).thenReturn(attributes); - reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( mockException, mockThread, new EventMetadata(sessionId, timestamp)); @@ -311,9 +314,6 @@ public void testNonFatalEvent_addsUserInfoKeysToEventWhenAvailable() throws Exce final String sessionId = "testSessionId"; - final Map attributes = Collections.emptyMap(); - when(reportMetadata.getCustomKeys()).thenReturn(attributes); - final String testKey1 = "testKey1"; final String testValue1 = "testValue1"; @@ -357,7 +357,7 @@ public void testNonFatalEvent_mergesUserInfoKeysWithCustomKeys() throws Exceptio attributes.put(testKey1, testValue1); attributes.put(testKey2, testValue2); - when(reportMetadata.getCustomKeys()).thenReturn(attributes); + addKeysToUserMetadata(attributes); final String testValue1UserInfo = "testValue1"; final String testKey3 = "testKey3"; @@ -374,10 +374,8 @@ public void testNonFatalEvent_mergesUserInfoKeysWithCustomKeys() throws Exceptio final CustomAttribute customAttribute3 = CustomAttribute.builder().setKey(testKey3).setValue(testValue3).build(); - final List expectedCustomAttributes = new ArrayList<>(); - expectedCustomAttributes.add(customAttribute1); - expectedCustomAttributes.add(customAttribute2); - expectedCustomAttributes.add(customAttribute3); + final List expectedCustomAttributes = + List.of(customAttribute1, customAttribute2, customAttribute3); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( @@ -398,10 +396,10 @@ public void testNonFatalEvent_addRolloutsEvent() throws Exception { String sessionId = "testSessionId"; mockEventInteractions(); - List rolloutsState = - new ArrayList(); - rolloutsState.add(mockRolloutAssignment()); - when(reportMetadata.getRolloutsState()).thenReturn(rolloutsState); + List rolloutsState = new ArrayList<>(); + rolloutsState.add(fakeRolloutAssignment()); + reportMetadata.updateRolloutsState(rolloutsState); + crashlyticsWorkers.diskWrite.await(); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( @@ -412,14 +410,13 @@ public void testNonFatalEvent_addRolloutsEvent() throws Exception { verify(mockEventAppBuilder, never()).setCustomAttributes(anyList()); verify(mockEventAppBuilder, never()).build(); verify(mockEventBuilder, never()).setApp(mockEventApp); - verify(reportMetadata).getRolloutsState(); // first build for custom keys // second build for rollouts verify(mockEventBuilder, times(2)).build(); } @Test - public void testFatalEvent_addsSortedCustomKeysToEvent() { + public void testFatalEvent_addsSortedCustomKeysToEvent() throws Exception { final long timestamp = System.currentTimeMillis(); mockEventInteractions(); @@ -444,7 +441,7 @@ public void testFatalEvent_addsSortedCustomKeysToEvent() { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - when(reportMetadata.getCustomKeys()).thenReturn(attributes); + addKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -457,7 +454,7 @@ public void testFatalEvent_addsSortedCustomKeysToEvent() { } @Test - public void testFatalEvent_addsSortedInternalKeysToEvent() { + public void testFatalEvent_addsSortedInternalKeysToEvent() throws Exception { final long timestamp = System.currentTimeMillis(); mockEventInteractions(); @@ -482,7 +479,7 @@ public void testFatalEvent_addsSortedInternalKeysToEvent() { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - when(reportMetadata.getInternalKeys()).thenReturn(attributes); + addKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -502,10 +499,6 @@ public void testFatalEvent_addsNoKeysToEventWhenNoneAvailable() { final String sessionId = "testSessionId"; - final Map attributes = Collections.emptyMap(); - - when(reportMetadata.getCustomKeys()).thenReturn(attributes); - reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -517,15 +510,16 @@ public void testFatalEvent_addsNoKeysToEventWhenNoneAvailable() { } @Test - public void testFatalEvent_addRolloutsToEvent() { + public void testFatalEvent_addRolloutsToEvent() throws Exception { long timestamp = System.currentTimeMillis(); String sessionId = "testSessionId"; mockEventInteractions(); - List rolloutsState = - new ArrayList(); - rolloutsState.add(mockRolloutAssignment()); - when(reportMetadata.getRolloutsState()).thenReturn(rolloutsState); + List rolloutsState = new ArrayList<>(); + rolloutsState.add(fakeRolloutAssignment()); + + reportMetadata.updateRolloutsState(rolloutsState); + crashlyticsWorkers.diskWrite.await(); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -533,7 +527,6 @@ public void testFatalEvent_addRolloutsToEvent() { verify(mockEventAppBuilder, never()).setCustomAttributes(anyList()); verify(mockEventAppBuilder, never()).build(); verify(mockEventBuilder, never()).setApp(mockEventApp); - verify(reportMetadata).getRolloutsState(); // first build for custom keys // second build for rollouts verify(mockEventBuilder, times(2)).build(); @@ -618,6 +611,14 @@ public void testRemoveAllReports_deletesPersistedReports() { verify(reportPersistence).deleteAllReports(); } + private void addKeysToUserMetadata(Map customKeys) throws Exception { + reportMetadata.setCustomKeys(customKeys); + for (Map.Entry entry : customKeys.entrySet()) { + reportMetadata.setInternalKey(entry.getKey(), entry.getValue()); + } + crashlyticsWorkers.diskWrite.await(); + } + private void mockEventInteractions() { when(mockEvent.toBuilder()).thenReturn(mockEventBuilder); when(mockEventBuilder.build()).thenReturn(mockEvent); @@ -652,16 +653,7 @@ private static CrashlyticsReportWithSessionId mockReportWithSessionId(String ses mockReport(sessionId), sessionId, new File("fake")); } - private static CrashlyticsReport.Session.Event.RolloutAssignment mockRolloutAssignment() { - return CrashlyticsReport.Session.Event.RolloutAssignment.builder() - .setTemplateVersion(2) - .setParameterKey("my_feature") - .setParameterValue("false") - .setRolloutVariant( - CrashlyticsReport.Session.Event.RolloutAssignment.RolloutVariant.builder() - .setRolloutId("rollout_1") - .setVariantId("enabled") - .build()) - .build(); + private static RolloutAssignment fakeRolloutAssignment() { + return RolloutAssignment.create("rollout_1", "my_feature", "false", "enabled", 2); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index d111ae070a5..f4e1f7bf263 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -148,15 +148,13 @@ public Map getCustomKeys(Map eventKeys) { Map globalKeys = customKeys.getKeys(); HashMap result = new HashMap<>(globalKeys); for (Map.Entry entry : eventKeys.entrySet()) { - if (result.size() < MAX_ATTRIBUTES) { - String key = KeysMap.sanitizeString(entry.getKey(), MAX_ATTRIBUTE_SIZE); - String value = KeysMap.sanitizeString(entry.getValue(), MAX_ATTRIBUTE_SIZE); - result.put(key, value); - + String sanitizedKey = KeysMap.sanitizeString(entry.getKey(), MAX_ATTRIBUTE_SIZE); + if (result.size() < MAX_ATTRIBUTES || result.containsKey(sanitizedKey)) { + String sanitizedValue = KeysMap.sanitizeString(entry.getValue(), MAX_ATTRIBUTE_SIZE); + result.put(sanitizedKey, sanitizedValue); continue; } - // TODO: Explore using a LinkedHashMap to overwrite keys in this case. long ignoredEventKeysCount = eventKeys.size() - (MAX_ATTRIBUTES - globalKeys.size()); Logger.getLogger() .w( From 16648c095b7a3d1fbfea3fd6985c79a54ea85d31 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 17:17:19 -0500 Subject: [PATCH 22/42] Explicitly differentiate between internal and external keys in the SessionReportingCoordinatorTest --- .../SessionReportingCoordinatorTest.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java index d7cdeafdf72..0f5cf09233b 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java @@ -60,7 +60,7 @@ public class SessionReportingCoordinatorTest extends CrashlyticsTestCase { - private static String TEST_SESSION_ID = "testSession1"; + private static final String TEST_SESSION_ID = "testSessionId"; @Mock private CrashlyticsReportDataCapture dataCapture; @Mock private CrashlyticsReportPersistence reportPersistence; @@ -269,7 +269,8 @@ public void testNonFatalEvent_addsSortedKeysToEvent() throws Exception { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - addKeysToUserMetadata(attributes); + addCustomKeysToUserMetadata(attributes); + addInternalKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent( @@ -357,7 +358,7 @@ public void testNonFatalEvent_mergesUserInfoKeysWithCustomKeys() throws Exceptio attributes.put(testKey1, testValue1); attributes.put(testKey2, testValue2); - addKeysToUserMetadata(attributes); + addCustomKeysToUserMetadata(attributes); final String testValue1UserInfo = "testValue1"; final String testKey3 = "testKey3"; @@ -441,7 +442,7 @@ public void testFatalEvent_addsSortedCustomKeysToEvent() throws Exception { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - addKeysToUserMetadata(attributes); + addCustomKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -479,7 +480,7 @@ public void testFatalEvent_addsSortedInternalKeysToEvent() throws Exception { expectedCustomAttributes.add(customAttribute1); expectedCustomAttributes.add(customAttribute2); - addKeysToUserMetadata(attributes); + addInternalKeysToUserMetadata(attributes); reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp); @@ -611,7 +612,7 @@ public void testRemoveAllReports_deletesPersistedReports() { verify(reportPersistence).deleteAllReports(); } - private void addKeysToUserMetadata(Map customKeys) throws Exception { + private void addCustomKeysToUserMetadata(Map customKeys) throws Exception { reportMetadata.setCustomKeys(customKeys); for (Map.Entry entry : customKeys.entrySet()) { reportMetadata.setInternalKey(entry.getKey(), entry.getValue()); @@ -619,6 +620,13 @@ private void addKeysToUserMetadata(Map customKeys) throws Except crashlyticsWorkers.diskWrite.await(); } + private void addInternalKeysToUserMetadata(Map internalKeys) throws Exception { + for (Map.Entry entry : internalKeys.entrySet()) { + reportMetadata.setInternalKey(entry.getKey(), entry.getValue()); + } + crashlyticsWorkers.diskWrite.await(); + } + private void mockEventInteractions() { when(mockEvent.toBuilder()).thenReturn(mockEventBuilder); when(mockEventBuilder.build()).thenReturn(mockEvent); From 2c4942fbaf53e5f693dcf933a70744564b5c551b Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 26 Nov 2024 18:13:34 -0500 Subject: [PATCH 23/42] Change back behavior in empty event keys, and add additional unit tests --- .../internal/common/CrashlyticsCoreTest.java | 118 ++++++++++++++++++ .../internal/metadata/UserMetadata.java | 4 + 2 files changed, 122 insertions(+) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java index 6bd0924f3d0..497192b5348 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java @@ -159,6 +159,124 @@ public void testCustomAttributes() throws Exception { assertEquals(longValue, metadata.getCustomKeys().get(key1)); } + @Test + public void testCustomAttributes_retrievedWithEmptyEventKeys() throws Exception { + UserMetadata metadata = crashlyticsCore.getController().getUserMetadata(); + + assertTrue(metadata.getCustomKeys(Map.of()).isEmpty()); + + final String id = "id012345"; + crashlyticsCore.setUserId(id); + crashlyticsWorkers.common.await(); + assertEquals(id, metadata.getUserId()); + + final StringBuffer idBuffer = new StringBuffer(id); + while (idBuffer.length() < UserMetadata.MAX_ATTRIBUTE_SIZE) { + idBuffer.append("0"); + } + final String longId = idBuffer.toString(); + final String superLongId = longId + "more chars"; + + crashlyticsCore.setUserId(superLongId); + crashlyticsWorkers.common.await(); + assertEquals(longId, metadata.getUserId()); + + final String key1 = "key1"; + final String value1 = "value1"; + crashlyticsCore.setCustomKey(key1, value1); + crashlyticsWorkers.common.await(); + assertEquals(value1, metadata.getCustomKeys(Map.of()).get(key1)); + + // Adding an existing key with the same value should return false + assertFalse(metadata.setCustomKey(key1, value1)); + assertTrue(metadata.setCustomKey(key1, "someOtherValue")); + assertTrue(metadata.setCustomKey(key1, value1)); + assertFalse(metadata.setCustomKey(key1, value1)); + + final String longValue = longId.replaceAll("0", "x"); + final String superLongValue = longValue + "some more chars"; + + // test truncation of custom keys and attributes + crashlyticsCore.setCustomKey(superLongId, superLongValue); + crashlyticsWorkers.common.await(); + assertNull(metadata.getCustomKeys(Map.of()).get(superLongId)); + assertEquals(longValue, metadata.getCustomKeys().get(longId)); + + // test the max number of attributes. We've already set 2. + for (int i = 2; i < UserMetadata.MAX_ATTRIBUTES; ++i) { + final String key = "key" + i; + final String value = "value" + i; + crashlyticsCore.setCustomKey(key, value); + crashlyticsWorkers.common.await(); + assertEquals(value, metadata.getCustomKeys(Map.of()).get(key)); + } + // should be full now, extra key, value pairs will be dropped. + final String key = "new key"; + crashlyticsCore.setCustomKey(key, "some value"); + crashlyticsWorkers.common.await(); + assertFalse(metadata.getCustomKeys(Map.of()).containsKey(key)); + + // should be able to update existing keys + crashlyticsCore.setCustomKey(key1, longValue); + crashlyticsWorkers.common.await(); + assertEquals(longValue, metadata.getCustomKeys(Map.of()).get(key1)); + + // when we set a key to null, it should still exist with an empty value + crashlyticsCore.setCustomKey(key1, null); + crashlyticsWorkers.common.await(); + assertEquals("", metadata.getCustomKeys(Map.of()).get(key1)); + + // keys and values are trimmed. + crashlyticsCore.setCustomKey(" " + key1 + " ", " " + longValue + " "); + crashlyticsWorkers.common.await(); + assertTrue(metadata.getCustomKeys(Map.of()).containsKey(key1)); + assertEquals(longValue, metadata.getCustomKeys(Map.of()).get(key1)); + } + + @Test + public void testCustomKeysMergedWithEventKeys() throws Exception { + UserMetadata metadata = crashlyticsCore.getController().getUserMetadata(); + + Map keysAndValues = new HashMap<>(); + keysAndValues.put("1", "value"); + keysAndValues.put("2", "value"); + keysAndValues.put("3", "value"); + + metadata.setCustomKeys(keysAndValues); + crashlyticsWorkers.common.await(); + + Map eventKeysAndValues = new HashMap<>(); + eventKeysAndValues.put("4", "eventValue"); + eventKeysAndValues.put("5", "eventValue"); + + // Tests reading custom keys with event keys. + assertEquals(keysAndValues.size(), metadata.getCustomKeys().size()); + assertEquals(keysAndValues.size(), metadata.getCustomKeys(Map.of()).size()); + assertEquals( + keysAndValues.size() + eventKeysAndValues.size(), + metadata.getCustomKeys(eventKeysAndValues).size()); + + // Tests event keys don't add to custom keys in future reads. + assertEquals(keysAndValues.size(), metadata.getCustomKeys().size()); + assertEquals(keysAndValues.size(), metadata.getCustomKeys(Map.of()).size()); + + // Tests additional event keys. + eventKeysAndValues.put("6", "eventValue"); + eventKeysAndValues.put("7", "eventValue"); + assertEquals( + keysAndValues.size() + eventKeysAndValues.size(), + metadata.getCustomKeys(eventKeysAndValues).size()); + + // Tests overriding custom key with event keys. + keysAndValues.put("7", "value"); + metadata.setCustomKeys(keysAndValues); + crashlyticsWorkers.common.await(); + + assertEquals("value", metadata.getCustomKeys().get("7")); + assertEquals("value", metadata.getCustomKeys(Map.of()).get("7")); + assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("7")); + } + @Test public void testBulkCustomKeys() throws Exception { final double DELTA = 1e-15; diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index f4e1f7bf263..cc53278cd54 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -145,6 +145,10 @@ public void setUserId(String identifier) { * @return the {Map} respecting the custom key constraints. */ public Map getCustomKeys(Map eventKeys) { + // In case of empty event keys, preserve existing behavior. + if (eventKeys.isEmpty()) return customKeys.getKeys(); + + // Otherwise merge the event keys with custom keys as appropriate. Map globalKeys = customKeys.getKeys(); HashMap result = new HashMap<>(globalKeys); for (Map.Entry entry : eventKeys.entrySet()) { From 9cd5814d11ffcf6e194552b56e742a7717058b18 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 10:18:16 -0500 Subject: [PATCH 24/42] Add extra tests and fix a bug in adding event keys --- .../internal/common/CrashlyticsCoreTest.java | 15 +++++++++++++++ .../internal/metadata/UserMetadata.java | 10 ++++------ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java index 497192b5348..6080ef89283 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java @@ -275,6 +275,21 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { assertEquals("value", metadata.getCustomKeys().get("7")); assertEquals("value", metadata.getCustomKeys(Map.of()).get("7")); assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("7")); + + // Test the event key behavior when the count of custom keys is max. + for (int i = keysAndValues.size(); i < UserMetadata.MAX_ATTRIBUTES; ++i) { + final String key = "key" + i; + final String value = "value" + i; + metadata.setCustomKey(key, value); + crashlyticsWorkers.common.await(); + assertEquals(value, metadata.getCustomKeys().get(key)); + } + + assertEquals(UserMetadata.MAX_ATTRIBUTES, metadata.getCustomKeys().size()); + + assertEquals("value", metadata.getCustomKeys().get("7")); + assertEquals("value", metadata.getCustomKeys(Map.of()).get("7")); + assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("7")); } @Test diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index cc53278cd54..b01fea63fa3 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -159,14 +159,12 @@ public Map getCustomKeys(Map eventKeys) { continue; } - long ignoredEventKeysCount = eventKeys.size() - (MAX_ATTRIBUTES - globalKeys.size()); Logger.getLogger() .w( - "Ignored " - + ignoredEventKeysCount - + " event specific keys. Maximum key count is " - + MAX_ATTRIBUTES); - break; + "Ignored entry \"" + + entry.getKey() + + "\" when adding event specific keys. Maximum allowable: " + + MAX_ATTRIBUTE_SIZE); } return Collections.unmodifiableMap(result); } From ca6f434f0d7c9dc7024705f89694f71e682813ec Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 10:31:51 -0500 Subject: [PATCH 25/42] additional unit test changes --- .../internal/common/CrashlyticsCoreTest.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java index 6080ef89283..f1323d354ce 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java @@ -238,16 +238,16 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { UserMetadata metadata = crashlyticsCore.getController().getUserMetadata(); Map keysAndValues = new HashMap<>(); - keysAndValues.put("1", "value"); - keysAndValues.put("2", "value"); - keysAndValues.put("3", "value"); + keysAndValues.put("customKey1", "value"); + keysAndValues.put("customKey2", "value"); + keysAndValues.put("customKey3", "value"); metadata.setCustomKeys(keysAndValues); crashlyticsWorkers.common.await(); Map eventKeysAndValues = new HashMap<>(); - eventKeysAndValues.put("4", "eventValue"); - eventKeysAndValues.put("5", "eventValue"); + eventKeysAndValues.put("eventKey1", "eventValue"); + eventKeysAndValues.put("eventKey2", "eventValue"); // Tests reading custom keys with event keys. assertEquals(keysAndValues.size(), metadata.getCustomKeys().size()); @@ -261,20 +261,20 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { assertEquals(keysAndValues.size(), metadata.getCustomKeys(Map.of()).size()); // Tests additional event keys. - eventKeysAndValues.put("6", "eventValue"); - eventKeysAndValues.put("7", "eventValue"); + eventKeysAndValues.put("eventKey3", "eventValue"); + eventKeysAndValues.put("eventKey4", "eventValue"); assertEquals( keysAndValues.size() + eventKeysAndValues.size(), metadata.getCustomKeys(eventKeysAndValues).size()); // Tests overriding custom key with event keys. - keysAndValues.put("7", "value"); + keysAndValues.put("eventKey1", "value"); metadata.setCustomKeys(keysAndValues); crashlyticsWorkers.common.await(); - assertEquals("value", metadata.getCustomKeys().get("7")); - assertEquals("value", metadata.getCustomKeys(Map.of()).get("7")); - assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("7")); + assertEquals("value", metadata.getCustomKeys().get("eventKey1")); + assertEquals("value", metadata.getCustomKeys(Map.of()).get("eventKey1")); + assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("eventKey1")); // Test the event key behavior when the count of custom keys is max. for (int i = keysAndValues.size(); i < UserMetadata.MAX_ATTRIBUTES; ++i) { @@ -287,9 +287,13 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { assertEquals(UserMetadata.MAX_ATTRIBUTES, metadata.getCustomKeys().size()); - assertEquals("value", metadata.getCustomKeys().get("7")); - assertEquals("value", metadata.getCustomKeys(Map.of()).get("7")); - assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("7")); + // Tests event keys override global custom keys when the key exists. + assertEquals("value", metadata.getCustomKeys().get("eventKey1")); + assertEquals("value", metadata.getCustomKeys(Map.of()).get("eventKey1")); + assertEquals("eventValue", metadata.getCustomKeys(eventKeysAndValues).get("eventKey1")); + + // Test when event keys *don't* override global custom keys. + assertNull(metadata.getCustomKeys(eventKeysAndValues).get("eventKey2")); } @Test From 12142f5496fd82f030103a4e0e50f7224f948a6c Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 10:34:38 -0500 Subject: [PATCH 26/42] additional unit test changes --- .../crashlytics/internal/common/CrashlyticsCoreTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java index f1323d354ce..56ac7f825c5 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java @@ -242,7 +242,7 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { keysAndValues.put("customKey2", "value"); keysAndValues.put("customKey3", "value"); - metadata.setCustomKeys(keysAndValues); + crashlyticsCore.setCustomKeys(keysAndValues); crashlyticsWorkers.common.await(); Map eventKeysAndValues = new HashMap<>(); @@ -269,7 +269,7 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { // Tests overriding custom key with event keys. keysAndValues.put("eventKey1", "value"); - metadata.setCustomKeys(keysAndValues); + crashlyticsCore.setCustomKeys(keysAndValues); crashlyticsWorkers.common.await(); assertEquals("value", metadata.getCustomKeys().get("eventKey1")); @@ -280,7 +280,7 @@ public void testCustomKeysMergedWithEventKeys() throws Exception { for (int i = keysAndValues.size(); i < UserMetadata.MAX_ATTRIBUTES; ++i) { final String key = "key" + i; final String value = "value" + i; - metadata.setCustomKey(key, value); + crashlyticsCore.setCustomKey(key, value); crashlyticsWorkers.common.await(); assertEquals(value, metadata.getCustomKeys().get(key)); } From d13605e3c7be91c1df83cc8376b862a3f1fe0d63 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 10:37:34 -0500 Subject: [PATCH 27/42] Rename userInfo in EventMetadata to additionalCustomKeys --- .../internal/common/SessionReportingCoordinator.java | 2 +- .../firebase/crashlytics/internal/metadata/EventMetadata.kt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index cdc837e5651..5abc282d556 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -342,7 +342,7 @@ private void persistEvent( MAX_CHAINED_EXCEPTION_DEPTH, isFatal); CrashlyticsReport.Session.Event finallizedEvent = - addMetaDataToEvent(capturedEvent, eventMetadata.getUserInfo()); + addMetaDataToEvent(capturedEvent, eventMetadata.getAdditionalCustomKeys()); // Non-fatal, persistence write task we move to diskWriteWorker if (!isFatal) { diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index 1bca65a26f8..9011075910e 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -19,13 +19,13 @@ package com.google.firebase.crashlytics.internal.metadata * * @property sessionId the sessionId to attach to the event. * @property timestamp the timestamp to attach to the event. - * @property userInfo a [Map] of key value pairs to attach to the event, in addition - * to the global custom keys. + * @property additionalCustomKeys a [Map] of key value pairs to attach to the event, + * in addition to the global custom keys. */ data class EventMetadata @JvmOverloads constructor( val sessionId: String, val timestamp: Long, - val userInfo: Map = mapOf() + val additionalCustomKeys: Map = mapOf() ) From c3cf46be74c86022d231dcdf55ee130313ea0110 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 10:59:08 -0500 Subject: [PATCH 28/42] Update javadoc --- .../crashlytics/FirebaseCrashlytics.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index d53993fcbaf..6a0d2e9dc6d 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -211,20 +211,28 @@ public void recordException(@NonNull Throwable throwable) { /** * Records a non-fatal report to send to Crashlytics. * + *

Combined with app level custom keys, the event is restricted to a maximum of 64 key/value + * pairs. New keys beyond that limit are ignored. Keys or values that exceed 1024 characters are + * truncated. + * + *

The values of event keys override the values of app level custom keys if they're identical. + * * @param throwable a {@link Throwable} to be recorded as a non-fatal event. - * @param userInfo a {@link Map} to add key value pairs to be recorded with the non fatal exception. + * @param customKeys a {@link Map} containing key value pairs to be associated + * with the non fatal exception, in addition to the global custom keys. */ - public void recordException(@NonNull Throwable throwable, @NonNull Map userInfo) { + public void recordException( + @NonNull Throwable throwable, @NonNull Map customKeys) { if (throwable == null) { // Users could call this with null despite the annotation. Logger.getLogger().w("A null value was passed to recordException. Ignoring."); return; } - if (userInfo == null) { // It's possible to set this to null even with the annotation. - userInfo = Map.of(); + if (customKeys == null) { // It's possible to set this to null even with the annotation. + customKeys = Map.of(); } - core.logException(throwable, userInfo); + core.logException(throwable, customKeys); } /** From 4471d2347961ea290e659c98860b60cd71522716 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 11:00:04 -0500 Subject: [PATCH 29/42] Update comment --- .../com/google/firebase/crashlytics/FirebaseCrashlytics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index 6a0d2e9dc6d..62a3dea1be2 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -228,7 +228,7 @@ public void recordException( return; } - if (customKeys == null) { // It's possible to set this to null even with the annotation. + if (customKeys == null) { // Users could call this with null despite the annotation. customKeys = Map.of(); } From bbfc5e8f3df50d77200eb10d302719ccb77e7e4e Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 11:11:53 -0500 Subject: [PATCH 30/42] more javadoc updates --- .../crashlytics/internal/metadata/UserMetadata.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index b01fea63fa3..930fc1ccd9b 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -138,11 +138,14 @@ public void setUserId(String identifier) { crashlyticsWorkers.diskWrite.submit(this::serializeUserDataIfNeeded); } - /** Creates a {Map} with all the custom keys to attach to the event. - * - * @param eventKeys a {Map} representing event specific keys. + /** + * Returns a {@link Map} containing all the custom keys to attach to the event. + * It overrides the values of app level custom keys with the values of event level custom keys if + * they're identical, and event keys or values that exceed 1024 characters are truncated. + * Combined with app level custom keys, the map is restricted to 64 key value pairs. * - * @return the {Map} respecting the custom key constraints. + * @param eventKeys a {@link Map} representing event specific keys. + * @return a {@link Map} containing all the custom keys to attach to the event. */ public Map getCustomKeys(Map eventKeys) { // In case of empty event keys, preserve existing behavior. From dcc33e692a9e13022c9dbec3eceed8fe91988c16 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 11:13:12 -0500 Subject: [PATCH 31/42] Rename userInfo to eventKeys where applicable --- .../crashlytics/internal/common/CrashlyticsController.java | 5 +++-- .../crashlytics/internal/common/CrashlyticsCore.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index 9b5250ce064..b55a26678d4 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -409,7 +409,7 @@ void writeToLog(final long timestamp, final String msg) { void writeNonFatalException( @NonNull final Thread thread, @NonNull final Throwable ex, - @NonNull Map userInfo) { + @NonNull Map eventKeys) { // Capture and close over the current time, so that we get the exact call time, // rather than the time at which the task executes. final long timestampMillis = System.currentTimeMillis(); @@ -421,7 +421,8 @@ void writeNonFatalException( Logger.getLogger().w("Tried to write a non-fatal exception while no session was open."); return; } - EventMetadata eventMetadata = new EventMetadata(currentSessionId, timestampSeconds, userInfo); + EventMetadata eventMetadata = + new EventMetadata(currentSessionId, timestampSeconds, eventKeys); reportingCoordinator.persistNonFatalEvent(ex, thread, eventMetadata); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java index eaccd9dd377..e5083758f66 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java @@ -313,9 +313,9 @@ public static String getVersion() { * Throwable was thrown. The Throwable will always be processed on a background thread, so it is * safe to invoke this method from the main thread. */ - public void logException(@NonNull Throwable throwable, @NonNull Map userInfo) { + public void logException(@NonNull Throwable throwable, @NonNull Map eventKeys) { crashlyticsWorkers.common.submit( - () -> controller.writeNonFatalException(Thread.currentThread(), throwable, userInfo)); + () -> controller.writeNonFatalException(Thread.currentThread(), throwable, eventKeys)); } /** From af2ddb1e7925d695db604de18a8a164a4b751961 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 11:14:33 -0500 Subject: [PATCH 32/42] update doc --- .../com/google/firebase/crashlytics/FirebaseCrashlytics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index 62a3dea1be2..a72ba9117d6 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -219,7 +219,7 @@ public void recordException(@NonNull Throwable throwable) { * * @param throwable a {@link Throwable} to be recorded as a non-fatal event. * @param customKeys a {@link Map} containing key value pairs to be associated - * with the non fatal exception, in addition to the global custom keys. + * with the non fatal exception, in addition to the app level custom keys. */ public void recordException( @NonNull Throwable throwable, @NonNull Map customKeys) { From c88649ed34c75e383d4fdc470f7da6bfc92a61e0 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 11:46:34 -0500 Subject: [PATCH 33/42] Remove LinkedHashMap TODO. --- .../google/firebase/crashlytics/internal/metadata/KeysMap.java | 1 - 1 file changed, 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java index e0aff943cff..5d688b5e134 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/KeysMap.java @@ -54,7 +54,6 @@ public synchronized boolean setKey(String key, String value) { keys.put(sanitizedKey, value == null ? "" : sanitizedAttribute); return true; } - // TODO: Explore using a LinkedHashMap to overwrite keys in this case. Logger.getLogger() .w( "Ignored entry \"" From b384a7c5a48b489e2a7539c82392aac39f19f1cf Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 16:09:29 -0500 Subject: [PATCH 34/42] Update the public API --- .../firebase/crashlytics/FirebaseCrashlytics.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index a72ba9117d6..1f147f5a0e4 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -205,7 +205,7 @@ public static FirebaseCrashlytics getInstance() { * @param throwable a {@link Throwable} to be recorded as a non-fatal event. */ public void recordException(@NonNull Throwable throwable) { - recordException(throwable, Map.of()); + core.logException(throwable, Map.of()); } /** @@ -218,21 +218,17 @@ public void recordException(@NonNull Throwable throwable) { *

The values of event keys override the values of app level custom keys if they're identical. * * @param throwable a {@link Throwable} to be recorded as a non-fatal event. - * @param customKeys a {@link Map} containing key value pairs to be associated - * with the non fatal exception, in addition to the app level custom keys. + * @param keysAndValues A dictionary of keys and the values to associate with the non fatal + * exception, in addition to the app level custom keys. */ public void recordException( - @NonNull Throwable throwable, @NonNull Map customKeys) { + @NonNull Throwable throwable, @NonNull CustomKeysAndValues keysAndValues) { if (throwable == null) { // Users could call this with null despite the annotation. Logger.getLogger().w("A null value was passed to recordException. Ignoring."); return; } - if (customKeys == null) { // Users could call this with null despite the annotation. - customKeys = Map.of(); - } - - core.logException(throwable, customKeys); + core.logException(throwable, keysAndValues.keysAndValues); } /** From 658f8700cf62dba77dcaae51f0d20b9801b5b299 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 16:13:37 -0500 Subject: [PATCH 35/42] Restore null throwable loggingin the original recordException --- .../com/google/firebase/crashlytics/FirebaseCrashlytics.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index 1f147f5a0e4..46f992aaf12 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -205,6 +205,11 @@ public static FirebaseCrashlytics getInstance() { * @param throwable a {@link Throwable} to be recorded as a non-fatal event. */ public void recordException(@NonNull Throwable throwable) { + if (throwable == null) { // Users could call this with null despite the annotation. + Logger.getLogger().w("A null value was passed to recordException. Ignoring."); + return; + } + core.logException(throwable, Map.of()); } From 6144c29affd01785230db665760c9b442bc3491a Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 27 Nov 2024 16:40:57 -0500 Subject: [PATCH 36/42] Update api.txt --- firebase-crashlytics/api.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/api.txt b/firebase-crashlytics/api.txt index c7102d05e75..c8fa0079300 100644 --- a/firebase-crashlytics/api.txt +++ b/firebase-crashlytics/api.txt @@ -23,7 +23,7 @@ package com.google.firebase.crashlytics { method public boolean isCrashlyticsCollectionEnabled(); method public void log(@NonNull String); method public void recordException(@NonNull Throwable); - method public void recordException(@NonNull Throwable, @NonNull java.util.Map); + method public void recordException(@NonNull Throwable, @NonNull com.google.firebase.crashlytics.CustomKeysAndValues); method public void sendUnsentReports(); method public void setCrashlyticsCollectionEnabled(boolean); method public void setCrashlyticsCollectionEnabled(@Nullable Boolean); From c2852093367363adc3d6dffcfbda70059b039bf2 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 3 Dec 2024 10:22:31 -0500 Subject: [PATCH 37/42] Fix brackets --- .../firebase/crashlytics/internal/metadata/UserMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 930fc1ccd9b..2ca698900e3 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -149,7 +149,7 @@ public void setUserId(String identifier) { */ public Map getCustomKeys(Map eventKeys) { // In case of empty event keys, preserve existing behavior. - if (eventKeys.isEmpty()) return customKeys.getKeys(); + if (eventKeys.isEmpty()) { return customKeys.getKeys(); } // Otherwise merge the event keys with custom keys as appropriate. Map globalKeys = customKeys.getKeys(); From 72ff0c898158f295d3c3167b7c42e6ef4275ec4a Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 3 Dec 2024 10:44:39 -0500 Subject: [PATCH 38/42] Update logging --- .../internal/metadata/UserMetadata.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 2ca698900e3..6133c232c6a 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -149,26 +149,33 @@ public void setUserId(String identifier) { */ public Map getCustomKeys(Map eventKeys) { // In case of empty event keys, preserve existing behavior. - if (eventKeys.isEmpty()) { return customKeys.getKeys(); } + if (eventKeys.isEmpty()) { + return customKeys.getKeys(); + } // Otherwise merge the event keys with custom keys as appropriate. Map globalKeys = customKeys.getKeys(); HashMap result = new HashMap<>(globalKeys); + int eventKeysOverLimit = 0; for (Map.Entry entry : eventKeys.entrySet()) { String sanitizedKey = KeysMap.sanitizeString(entry.getKey(), MAX_ATTRIBUTE_SIZE); if (result.size() < MAX_ATTRIBUTES || result.containsKey(sanitizedKey)) { String sanitizedValue = KeysMap.sanitizeString(entry.getValue(), MAX_ATTRIBUTE_SIZE); result.put(sanitizedKey, sanitizedValue); - continue; + } else { + eventKeysOverLimit++; } + } + if (eventKeysOverLimit > 0) { Logger.getLogger() .w( - "Ignored entry \"" - + entry.getKey() - + "\" when adding event specific keys. Maximum allowable: " + "Ignored" + + eventKeysOverLimit + + " keys when adding event specific keys. Maximum allowable: " + MAX_ATTRIBUTE_SIZE); } + return Collections.unmodifiableMap(result); } From d53c1f51011fd489570377025daacd63e805d166 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 3 Dec 2024 11:03:34 -0500 Subject: [PATCH 39/42] Chnage visibility of EventMetadata --- .../firebase/crashlytics/internal/metadata/EventMetadata.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt index 9011075910e..fef5899cef5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/EventMetadata.kt @@ -22,7 +22,7 @@ package com.google.firebase.crashlytics.internal.metadata * @property additionalCustomKeys a [Map] of key value pairs to attach to the event, * in addition to the global custom keys. */ -data class EventMetadata +internal data class EventMetadata @JvmOverloads constructor( val sessionId: String, From c94e370b48f799f3c443dc5629b52674ee1ea5a2 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 3 Dec 2024 11:21:29 -0500 Subject: [PATCH 40/42] Fix space in log --- .../firebase/crashlytics/internal/metadata/UserMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 6133c232c6a..ec7403078d7 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -170,7 +170,7 @@ public Map getCustomKeys(Map eventKeys) { if (eventKeysOverLimit > 0) { Logger.getLogger() .w( - "Ignored" + "Ignored " + eventKeysOverLimit + " keys when adding event specific keys. Maximum allowable: " + MAX_ATTRIBUTE_SIZE); From 3d3d1d462e6677cb2bbf4e572add674fd5792d33 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 5 Dec 2024 14:30:45 -0500 Subject: [PATCH 41/42] Add changelog --- firebase-crashlytics/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 5b57a9581e7..78dd9da2f3e 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased - +* [feature] Added an overload for `recordException` that allows setting custom keys limited + to the non fatal [#3551] # 19.3.0 * [fixed] Fixed inefficiency in the Kotlin `FirebaseCrashlytics.setCustomKeys` extension. From 79417ac8c6ac7e25f8e7a5081bae12b89def13b6 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Thu, 5 Dec 2024 15:34:19 -0500 Subject: [PATCH 42/42] Reword changelog --- firebase-crashlytics/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 78dd9da2f3e..4d526585b44 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased -* [feature] Added an overload for `recordException` that allows setting custom keys limited - to the non fatal [#3551] +* [feature] Added an overload for `recordException` that allows logging additional custom + keys to the non fatal event [#3551] # 19.3.0 * [fixed] Fixed inefficiency in the Kotlin `FirebaseCrashlytics.setCustomKeys` extension.