Skip to content

Commit

Permalink
Merge branch 'crashlytics/anrs' into td/anr
Browse files Browse the repository at this point in the history
  • Loading branch information
tejasd committed Jun 15, 2021
2 parents 8f1af2c + 1b56da8 commit ca67b3f
Show file tree
Hide file tree
Showing 19 changed files with 243 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.SimpleAnnotationValueVisitor8;

/**
* Generates an annotation implementation.
Expand Down Expand Up @@ -123,7 +125,21 @@ private static TypeSpec createBuilder(
FieldSpec.builder(field.type, field.name).addModifiers(Modifier.PRIVATE);
AnnotationValue value = annotationImpl.defaults.get(field);
if (value != null) {
fieldSpec.initializer("$L", value);
new SimpleAnnotationValueVisitor8<Void, Void>() {
@Override
public Void visitEnumConstant(VariableElement c, Void unused) {
// Using $L would print the simple name of the enum constant in JDK 17,
// see: https://github.com/square/javapoet/issues/845
fieldSpec.initializer("$T.$L", c.asType(), c.getSimpleName());
return null;
}

@Override
protected Void defaultAction(Object o, Void unused) {
fieldSpec.initializer("$L", value);
return null;
}
}.visit(value, null);
}
annotationBuilder.addMethod(
MethodSpec.methodBuilder(field.name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public final class AtMyAnnotation {

private String strVal = "default";

private MyAnnotation.MyEnum enumVal = com.example.MyAnnotation.MyEnum.VALUE1;
private MyAnnotation.MyEnum enumVal = MyAnnotation.MyEnum.VALUE1;

public AtMyAnnotation intVal(int intVal) {
this.intVal = intVal;
Expand Down Expand Up @@ -224,4 +224,4 @@ public String toString() {
return sb.toString();
}
}
}
}
4 changes: 2 additions & 2 deletions firebase-crashlytics-ndk/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
version=18.0.1
latestReleasedVersion=18.0.0
version=18.0.2
latestReleasedVersion=18.0.1
4 changes: 2 additions & 2 deletions firebase-crashlytics/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
version=18.0.1
latestReleasedVersion=18.0.0
version=18.0.2
latestReleasedVersion=18.0.1
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);
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 @@ -233,13 +233,16 @@ private CrashlyticsReport.Session.Event addLogsAndCustomKeysToEvent(

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

0 comments on commit ca67b3f

Please sign in to comment.