Skip to content

Crashlytics Add Internal Keys feature for Unity Metadata #2671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,33 @@ public void testWriteKeys_readDifferentSession() {
assertEquals(0, readKeys.size());
}

// Ensures the Internal Keys and User Custom Keys are stored separately
public void testWriteKeys_readSeparateFromUser() {
final Map<String, String> keys =
new HashMap<String, String>() {
{
put(KEY_1, VALUE_1);
}
};

final Map<String, String> internalKeys =
new HashMap<String, String>() {
{
put(KEY_2, VALUE_2);
put(KEY_3, VALUE_3);
}
};

storeUnderTest.writeKeyData(SESSION_ID_1, keys);
storeUnderTest.writeKeyData(SESSION_ID_1, internalKeys, /*isInternal=*/ true);

final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
final Map<String, String> readInternalKeys = storeUnderTest.readKeyData(SESSION_ID_1, true);

assertEqualMaps(keys, readKeys);
assertEqualMaps(internalKeys, readInternalKeys);
}

public void testReadKeys_noStoredData() {
final Map<String, String> readKeys = storeUnderTest.readKeyData(SESSION_ID_1);
assertEquals(0, readKeys.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,13 @@ public void testNonFatalEvent_addsSortedKeysToEvent() {
ImmutableList.from(customAttribute1, customAttribute2);

when(reportMetadata.getCustomKeys()).thenReturn(attributes);
when(reportMetadata.getInternalKeys()).thenReturn(attributes);

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

verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
verify(mockEventAppBuilder).build();
verify(mockEventBuilder).setApp(mockEventApp);
verify(mockEventBuilder).build();
Expand Down Expand Up @@ -288,11 +290,13 @@ public void testFatalEvent_addsSortedKeysToEvent() {
ImmutableList.from(customAttribute1, customAttribute2);

when(reportMetadata.getCustomKeys()).thenReturn(attributes);
when(reportMetadata.getInternalKeys()).thenReturn(attributes);

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

verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes);
verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes);
verify(mockEventAppBuilder).build();
verify(mockEventBuilder).setApp(mockEventApp);
verify(mockEventBuilder).build();
Expand Down Expand Up @@ -461,6 +465,7 @@ private void mockEventInteractions() {
when(mockEvent.getApp()).thenReturn(mockEventApp);
when(mockEventApp.toBuilder()).thenReturn(mockEventAppBuilder);
when(mockEventAppBuilder.setCustomAttributes(any())).thenReturn(mockEventAppBuilder);
when(mockEventAppBuilder.setInternalKeys(any())).thenReturn(mockEventAppBuilder);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the necessary line to fix tests

when(mockEventAppBuilder.build()).thenReturn(mockEventApp);
when(dataCapture.captureEventData(
any(Throwable.class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,14 +434,28 @@ void setCustomKey(String key, String value) {
return;
}
}
cacheKeyData(userMetadata.getCustomKeys());
cacheKeyData(userMetadata.getCustomKeys(), false);
}

void setCustomKeys(Map<String, String> keysAndValues) {
// Write all the key/value pairs before doing anything computationally expensive.
userMetadata.setCustomKeys(keysAndValues);
// Once all the key/value pairs are added, update the cache.
cacheKeyData(userMetadata.getCustomKeys());
cacheKeyData(userMetadata.getCustomKeys(), false);
}

void setInternalKey(String key, String value) {
try {
userMetadata.setInternalKey(key, value);
} catch (IllegalArgumentException ex) {
if (context != null && CommonUtils.isAppDebuggable(context)) {
throw ex;
} else {
Logger.getLogger().e("Attempting to set custom attribute with null key, ignoring.");
return;
}
}
cacheKeyData(userMetadata.getInternalKeys(), true);
}

/**
Expand Down Expand Up @@ -475,13 +489,13 @@ public Void call() throws Exception {
* crash happens immediately after setting a value. If this becomes a problem, we can investigate
* writing synchronously, or potentially add an explicit user-facing API for synchronous writes.
*/
private void cacheKeyData(final Map<String, String> keyData) {
private void cacheKeyData(final Map<String, String> keyData, boolean isInternal) {
backgroundWorker.submit(
new Callable<Void>() {
@Override
public Void call() throws Exception {
final String currentSessionId = getCurrentSessionId();
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData);
new MetaDataStore(getFilesDir()).writeKeyData(currentSessionId, keyData, isInternal);
return null;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ public boolean onPreExecute(AppData appData, SettingsDataProvider settingsProvid
initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore);

final UserMetadata userMetadata = new UserMetadata();

final LogFileDirectoryProvider logFileDirectoryProvider =
new LogFileDirectoryProvider(fileStore);
final LogFileManager logFileManager = new LogFileManager(context, logFileDirectoryProvider);
Expand Down Expand Up @@ -347,6 +346,22 @@ public void setCustomKeys(Map<String, String> keysAndValues) {
controller.setCustomKeys(keysAndValues);
}

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

// endregion

// region Package-protected getters
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright 2021 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.common;

import androidx.annotation.NonNull;
import com.google.firebase.crashlytics.internal.Logger;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/** Handles any key/values for metadata. */
public class KeysMap {
private final Map<String, String> keys = new HashMap<>();
private int maxEntries;
private int maxEntryLength;

public KeysMap(int maxEntries, int maxEntryLength) {
this.maxEntries = maxEntries;
this.maxEntryLength = maxEntryLength;
}

@NonNull
public Map<String, String> getKeys() {
return Collections.unmodifiableMap(keys);
}

public void setKey(String key, String value) {
setSyncKeys(
new HashMap<String, String>() {
{
put(sanitizeKey(key), sanitizeAttribute(value));
}
});
}

public void setKeys(Map<String, String> keysAndValues) {
setSyncKeys(keysAndValues);
}

/** Gatekeeper function for access to attributes or internalKeys */
private synchronized void setSyncKeys(Map<String, String> keysAndValues) {
// We want all access to the keys hashmap to be locked so that there is no way to create
// a race condition and add more than maxEntries keys.

// Update any existing keys first, then add any additional keys
Map<String, String> currentKeys = new HashMap<String, String>();
Map<String, String> newKeys = new HashMap<String, String>();

// Split into current and new keys
for (Map.Entry<String, String> entry : keysAndValues.entrySet()) {
String key = sanitizeKey(entry.getKey());
String value = (entry.getValue() == null) ? "" : sanitizeAttribute(entry.getValue());
if (keys.containsKey(key)) {
currentKeys.put(key, value);
} else {
newKeys.put(key, value);
}
}

keys.putAll(currentKeys);

// Add new keys if there is space
if (keys.size() + newKeys.size() > maxEntries) {
int keySlotsLeft = maxEntries - keys.size();
Logger.getLogger().v("Exceeded maximum number of custom attributes (" + maxEntries + ").");
List<String> newKeyList = new ArrayList<>(newKeys.keySet());
newKeys.keySet().retainAll(newKeyList.subList(0, keySlotsLeft));
}
keys.putAll(newKeys);
}

/** Checks that the key is not null then sanitizes it. */
private String sanitizeKey(String key) {
if (key == null) {
throw new IllegalArgumentException("Custom attribute key must not be null.");
}
return sanitizeAttribute(key);
}

/** Trims the string and truncates it to maxEntryLength. */
public String sanitizeAttribute(String input) {
if (input != null) {
input = input.trim();
if (input.length() > maxEntryLength) {
input = input.substring(0, maxEntryLength);
}
}
return input;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class MetaDataStore {

private static final String USERDATA_SUFFIX = "user";
private static final String KEYDATA_SUFFIX = "keys";
private static final String INTERNAL_KEYDATA_SUFFIX = "internal-keys";
private static final String METADATA_EXT = ".meta";

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

public void writeKeyData(String sessionId, Map<String, String> keyData) {
final File f = getKeysFileForSession(sessionId);
writeKeyData(sessionId, keyData, false);
}

void writeKeyData(String sessionId, Map<String, String> keyData, boolean isInternal) {
final File f =
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
Writer writer = null;
try {
final String keyDataString = keysDataToJson(keyData);
Expand All @@ -100,7 +106,12 @@ public void writeKeyData(String sessionId, Map<String, String> keyData) {
}

public Map<String, String> readKeyData(String sessionId) {
final File f = getKeysFileForSession(sessionId);
return readKeyData(sessionId, false);
}

Map<String, String> readKeyData(String sessionId, boolean isInternal) {
final File f =
isInternal ? getInternalKeysFileForSession(sessionId) : getKeysFileForSession(sessionId);
if (!f.exists()) {
return Collections.emptyMap();
}
Expand All @@ -127,6 +138,11 @@ public File getKeysFileForSession(String sessionId) {
return new File(filesDir, sessionId + KEYDATA_SUFFIX + METADATA_EXT);
}

@NonNull
public File getInternalKeysFileForSession(String sessionId) {
return new File(filesDir, sessionId + INTERNAL_KEYDATA_SUFFIX + METADATA_EXT);
}

private static UserMetadata jsonToUserData(String json) throws JSONException {
final JSONObject dataObj = new JSONObject(json);
UserMetadata metadata = new UserMetadata();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,16 @@ private void persistEvent(

final List<CustomAttribute> sortedCustomAttributes =
getSortedCustomAttributes(reportMetadata.getCustomKeys());
final List<CustomAttribute> sortedInternalKeys =
getSortedCustomAttributes(reportMetadata.getInternalKeys());

if (!sortedCustomAttributes.isEmpty()) {
eventBuilder.setApp(
capturedEvent
.getApp()
.toBuilder()
.setCustomAttributes(ImmutableList.from(sortedCustomAttributes))
.setInternalKeys(ImmutableList.from(sortedInternalKeys))
.build());
}

Expand Down
Loading