Skip to content

Commit c816404

Browse files
authored
Crashlytics Add Internal Keys feature for Unity Metadata (#2671) (#2708)
1 parent 30ee760 commit c816404

File tree

10 files changed

+212
-70
lines changed

10 files changed

+212
-70
lines changed

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/MetaDataStoreTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,33 @@ public void testWriteKeys_readDifferentSession() {
222222
assertEquals(0, readKeys.size());
223223
}
224224

225+
// Ensures the Internal Keys and User Custom Keys are stored separately
226+
public void testWriteKeys_readSeparateFromUser() {
227+
final Map<String, String> keys =
228+
new HashMap<String, String>() {
229+
{
230+
put(KEY_1, VALUE_1);
231+
}
232+
};
233+
234+
final Map<String, String> internalKeys =
235+
new HashMap<String, String>() {
236+
{
237+
put(KEY_2, VALUE_2);
238+
put(KEY_3, VALUE_3);
239+
}
240+
};
241+
242+
storeUnderTest.writeKeyData(SESSION_ID_1, keys);
243+
storeUnderTest.writeKeyData(SESSION_ID_1, internalKeys, /*isInternal=*/ true);
244+
245+
final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
246+
final Map<String, String> readInternalKeys = storeUnderTest.readKeyData(SESSION_ID_1, true);
247+
248+
assertEqualMaps(keys, readKeys);
249+
assertEqualMaps(internalKeys, readInternalKeys);
250+
}
251+
225252
public void testReadKeys_noStoredData() {
226253
final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
227254
assertEquals(0, readKeys.size());

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,13 @@ public void testNonFatalEvent_addsSortedKeysToEvent() {
229229
ImmutableList.from(customAttribute1, customAttribute2);
230230

231231
when(reportMetadata.getCustomKeys()).thenReturn(attributes);
232+
when(reportMetadata.getInternalKeys()).thenReturn(attributes);
232233

233234
reportingCoordinator.onBeginSession(sessionId, timestamp);
234235
reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp);
235236

236237
verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
238+
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
237239
verify(mockEventAppBuilder).build();
238240
verify(mockEventBuilder).setApp(mockEventApp);
239241
verify(mockEventBuilder).build();
@@ -288,11 +290,13 @@ public void testFatalEvent_addsSortedKeysToEvent() {
288290
ImmutableList.from(customAttribute1, customAttribute2);
289291

290292
when(reportMetadata.getCustomKeys()).thenReturn(attributes);
293+
when(reportMetadata.getInternalKeys()).thenReturn(attributes);
291294

292295
reportingCoordinator.onBeginSession(sessionId, timestamp);
293296
reportingCoordinator.persistFatalEvent(mockException, mockThread, sessionId, timestamp);
294297

295298
verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
299+
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
296300
verify(mockEventAppBuilder).build();
297301
verify(mockEventBuilder).setApp(mockEventApp);
298302
verify(mockEventBuilder).build();
@@ -461,6 +465,7 @@ private void mockEventInteractions() {
461465
when(mockEvent.getApp()).thenReturn(mockEventApp);
462466
when(mockEventApp.toBuilder()).thenReturn(mockEventAppBuilder);
463467
when(mockEventAppBuilder.setCustomAttributes(any())).thenReturn(mockEventAppBuilder);
468+
when(mockEventAppBuilder.setInternalKeys(any())).thenReturn(mockEventAppBuilder);
464469
when(mockEventAppBuilder.build()).thenReturn(mockEventApp);
465470
when(dataCapture.captureEventData(
466471
any(Throwable.class),

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,14 +434,28 @@ void setCustomKey(String key, String value) {
434434
return;
435435
}
436436
}
437-
cacheKeyData(userMetadata.getCustomKeys());
437+
cacheKeyData(userMetadata.getCustomKeys(), false);
438438
}
439439

440440
void setCustomKeys(Map<String, String> keysAndValues) {
441441
// Write all the key/value pairs before doing anything computationally expensive.
442442
userMetadata.setCustomKeys(keysAndValues);
443443
// Once all the key/value pairs are added, update the cache.
444-
cacheKeyData(userMetadata.getCustomKeys());
444+
cacheKeyData(userMetadata.getCustomKeys(), false);
445+
}
446+
447+
void setInternalKey(String key, String value) {
448+
try {
449+
userMetadata.setInternalKey(key, value);
450+
} catch (IllegalArgumentException ex) {
451+
if (context != null && CommonUtils.isAppDebuggable(context)) {
452+
throw ex;
453+
} else {
454+
Logger.getLogger().e("Attempting to set custom attribute with null key, ignoring.");
455+
return;
456+
}
457+
}
458+
cacheKeyData(userMetadata.getInternalKeys(), true);
445459
}
446460

447461
/**
@@ -475,13 +489,13 @@ public Void call() throws Exception {
475489
* crash happens immediately after setting a value. If this becomes a problem, we can investigate
476490
* writing synchronously, or potentially add an explicit user-facing API for synchronous writes.
477491
*/
478-
private void cacheKeyData(final Map<String, String> keyData) {
492+
private void cacheKeyData(final Map<String, String> keyData, boolean isInternal) {
479493
backgroundWorker.submit(
480494
new Callable<Void>() {
481495
@Override
482496
public Void call() throws Exception {
483497
final String currentSessionId = getCurrentSessionId();
484-
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData);
498+
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData, isInternal);
485499
return null;
486500
}
487501
});

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ public boolean onPreExecute(AppData appData, SettingsDataProvider settingsProvid
131131
initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore);
132132

133133
final UserMetadata userMetadata = new UserMetadata();
134-
135134
final LogFileDirectoryProvider logFileDirectoryProvider =
136135
new LogFileDirectoryProvider(fileStore);
137136
final LogFileManager logFileManager = new LogFileManager(context, logFileDirectoryProvider);
@@ -347,6 +346,22 @@ public void setCustomKeys(Map<String, String> keysAndValues) {
347346
controller.setCustomKeys(keysAndValues);
348347
}
349348

349+
/**
350+
* Set a value to be associated with a given key for your crash data. The key/value pairs will be
351+
* reported with any crash that occurs in this session. A maximum of 64 key/value pairs can be
352+
* stored for any type. New keys added over that limit will be ignored. Keys and values are
353+
* trimmed ({@link String#trim()}), and keys or values that exceed 1024 characters will be
354+
* truncated.
355+
*
356+
* <p>IMPORTANT: This method is accessed via reflection and JNI. Do not change the type without
357+
* updating the SDKs that depend on it.
358+
*
359+
* @throws NullPointerException if key is null.
360+
*/
361+
public void setInternalKey(String key, String value) {
362+
controller.setInternalKey(key, value);
363+
}
364+
350365
// endregion
351366

352367
// region Package-protected getters
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// Copyright 2021 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.crashlytics.internal.common;
16+
17+
import androidx.annotation.NonNull;
18+
import com.google.firebase.crashlytics.internal.Logger;
19+
import java.util.ArrayList;
20+
import java.util.Collections;
21+
import java.util.HashMap;
22+
import java.util.List;
23+
import java.util.Map;
24+
25+
/** Handles any key/values for metadata. */
26+
public class KeysMap {
27+
private final Map<String, String> keys = new HashMap<>();
28+
private int maxEntries;
29+
private int maxEntryLength;
30+
31+
public KeysMap(int maxEntries, int maxEntryLength) {
32+
this.maxEntries = maxEntries;
33+
this.maxEntryLength = maxEntryLength;
34+
}
35+
36+
@NonNull
37+
public Map<String, String> getKeys() {
38+
return Collections.unmodifiableMap(keys);
39+
}
40+
41+
public void setKey(String key, String value) {
42+
setSyncKeys(
43+
new HashMap<String, String>() {
44+
{
45+
put(sanitizeKey(key), sanitizeAttribute(value));
46+
}
47+
});
48+
}
49+
50+
public void setKeys(Map<String, String> keysAndValues) {
51+
setSyncKeys(keysAndValues);
52+
}
53+
54+
/** Gatekeeper function for access to attributes or internalKeys */
55+
private synchronized void setSyncKeys(Map<String, String> keysAndValues) {
56+
// We want all access to the keys hashmap to be locked so that there is no way to create
57+
// a race condition and add more than maxEntries keys.
58+
59+
// Update any existing keys first, then add any additional keys
60+
Map<String, String> currentKeys = new HashMap<String, String>();
61+
Map<String, String> newKeys = new HashMap<String, String>();
62+
63+
// Split into current and new keys
64+
for (Map.Entry<String, String> entry : keysAndValues.entrySet()) {
65+
String key = sanitizeKey(entry.getKey());
66+
String value = (entry.getValue() == null) ? "" : sanitizeAttribute(entry.getValue());
67+
if (keys.containsKey(key)) {
68+
currentKeys.put(key, value);
69+
} else {
70+
newKeys.put(key, value);
71+
}
72+
}
73+
74+
keys.putAll(currentKeys);
75+
76+
// Add new keys if there is space
77+
if (keys.size() + newKeys.size() > maxEntries) {
78+
int keySlotsLeft = maxEntries - keys.size();
79+
Logger.getLogger().v("Exceeded maximum number of custom attributes (" + maxEntries + ").");
80+
List<String> newKeyList = new ArrayList<>(newKeys.keySet());
81+
newKeys.keySet().retainAll(newKeyList.subList(0, keySlotsLeft));
82+
}
83+
keys.putAll(newKeys);
84+
}
85+
86+
/** Checks that the key is not null then sanitizes it. */
87+
private String sanitizeKey(String key) {
88+
if (key == null) {
89+
throw new IllegalArgumentException("Custom attribute key must not be null.");
90+
}
91+
return sanitizeAttribute(key);
92+
}
93+
94+
/** Trims the string and truncates it to maxEntryLength. */
95+
public String sanitizeAttribute(String input) {
96+
if (input != null) {
97+
input = input.trim();
98+
if (input.length() > maxEntryLength) {
99+
input = input.substring(0, maxEntryLength);
100+
}
101+
}
102+
return input;
103+
}
104+
}

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class MetaDataStore {
4141

4242
private static final String USERDATA_SUFFIX = "user";
4343
private static final String KEYDATA_SUFFIX = "keys";
44+
private static final String INTERNAL_KEYDATA_SUFFIX = "internal-keys";
4445
private static final String METADATA_EXT = ".meta";
4546

4647
private static final String KEY_USER_ID = "userId";
@@ -85,7 +86,12 @@ public UserMetadata readUserData(String sessionId) {
8586
}
8687

8788
public void writeKeyData(String sessionId, Map<String, String> keyData) {
88-
final File f = getKeysFileForSession(sessionId);
89+
writeKeyData(sessionId, keyData, false);
90+
}
91+
92+
void writeKeyData(String sessionId, Map<String, String> keyData, boolean isInternal) {
93+
final File f =
94+
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
8995
Writer writer = null;
9096
try {
9197
final String keyDataString = keysDataToJson(keyData);
@@ -100,7 +106,12 @@ public void writeKeyData(String sessionId, Map<String, String> keyData) {
100106
}
101107

102108
public Map<String, String> readKeyData(String sessionId) {
103-
final File f = getKeysFileForSession(sessionId);
109+
return readKeyData(sessionId, false);
110+
}
111+
112+
Map<String, String> readKeyData(String sessionId, boolean isInternal) {
113+
final File f =
114+
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
104115
if (!f.exists()) {
105116
return Collections.emptyMap();
106117
}
@@ -127,6 +138,11 @@ public File getKeysFileForSession(String sessionId) {
127138
return new File(filesDir, sessionId + KEYDATA_SUFFIX + METADATA_EXT);
128139
}
129140

141+
@NonNull
142+
public File getInternalKeysFileForSession(String sessionId) {
143+
return new File(filesDir, sessionId + INTERNAL_KEYDATA_SUFFIX + METADATA_EXT);
144+
}
145+
130146
private static UserMetadata jsonToUserData(String json) throws JSONException {
131147
final JSONObject dataObj = new JSONObject(json);
132148
UserMetadata metadata = new UserMetadata();

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,16 @@ private void persistEvent(
220220

221221
final List<CustomAttribute> sortedCustomAttributes =
222222
getSortedCustomAttributes(reportMetadata.getCustomKeys());
223+
final List<CustomAttribute> sortedInternalKeys =
224+
getSortedCustomAttributes(reportMetadata.getInternalKeys());
223225

224226
if (!sortedCustomAttributes.isEmpty()) {
225227
eventBuilder.setApp(
226228
capturedEvent
227229
.getApp()
228230
.toBuilder()
229231
.setCustomAttributes(ImmutableList.from(sortedCustomAttributes))
232+
.setInternalKeys(ImmutableList.from(sortedInternalKeys))
230233
.build());
231234
}
232235

0 commit comments

Comments
 (0)