Skip to content

Commit

Permalink
Merge pull request #853 from Microsoft/feature/priority-database
Browse files Browse the repository at this point in the history
Add priority to database persistence
  • Loading branch information
jaeklim committed Oct 30, 2018
2 parents a35d53f + a3a1160 commit ca051cf
Show file tree
Hide file tree
Showing 12 changed files with 248 additions and 73 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import static com.microsoft.appcenter.Flags.PERSISTENCE_NORMAL;

@SuppressWarnings("unused")
@SmallTest
@RunWith(AndroidJUnit4.class)
Expand Down Expand Up @@ -49,7 +51,7 @@ public void missingLogSerializer() throws Persistence.PersistenceException {
//noinspection TryFinallyCanBeTryWithResources (try with resources statement is API >= 19)
try {
/* Generate a log and persist. */
persistence.putLog("exception", new MockLog());
persistence.putLog("exception", new MockLog(), PERSISTENCE_NORMAL);
} finally {
/* Close. */
//noinspection ThrowFromFinallyBlock
Expand Down
27 changes: 27 additions & 0 deletions sdk/appcenter/src/main/java/com/microsoft/appcenter/Flags.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.microsoft.appcenter;

/**
* Persistence and latency flags for telemetry.
*/
public final class Flags {

/**
* Mask for persistence within flags.
*/
public static final int PERSISTENCE_MASK = 0xFF;

/**
* An event can be lost due to low bandwidth or disk space constraints.
*/
public static final int PERSISTENCE_NORMAL = 0x01;

/**
* Used for events that should be prioritized over non-critical events.
*/
public static final int PERSISTENCE_CRITICAL = 0x02;

/**
* Default combination of flags.
*/
public static final int DEFAULT_FLAGS = PERSISTENCE_NORMAL;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.support.annotation.VisibleForTesting;

import com.microsoft.appcenter.CancellationException;
import com.microsoft.appcenter.Flags;
import com.microsoft.appcenter.http.HttpUtils;
import com.microsoft.appcenter.http.ServiceCallback;
import com.microsoft.appcenter.ingestion.AppCenterIngestion;
Expand Down Expand Up @@ -688,7 +689,8 @@ public synchronized void enqueue(@NonNull Log log, @NonNull final String groupNa
try {

/* Persist log. */
mPersistence.putLog(groupName, log);
// TODO introduce parameter for flags and extract persistence from mask.
mPersistence.putLog(groupName, log, Flags.PERSISTENCE_NORMAL);

/* Nothing more to do if the log is from a paused transmission target. */
Iterator<String> targetKeys = log.getTransmissionTargetTokens().iterator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import android.support.annotation.VisibleForTesting;

import com.microsoft.appcenter.Constants;
import com.microsoft.appcenter.Flags;
import com.microsoft.appcenter.ingestion.models.Log;
import com.microsoft.appcenter.ingestion.models.one.CommonSchemaLog;
import com.microsoft.appcenter.ingestion.models.one.PartAUtils;
Expand All @@ -36,6 +37,7 @@
import java.util.TreeMap;

import static com.microsoft.appcenter.AppCenter.LOG_TAG;
import static com.microsoft.appcenter.Flags.PERSISTENCE_NORMAL;

@SuppressWarnings("TryFinallyCanBeTryWithResources")
public class DatabasePersistence extends Persistence {
Expand All @@ -46,6 +48,12 @@ public class DatabasePersistence extends Persistence {
@VisibleForTesting
static final int VERSION_TYPE_API_KEY = 2;

/**
* Version of the schema that introduced target key field.
*/
@VisibleForTesting
static final int VERSION_TARGET_KEY = 3;

/**
* Name of group column in the table.
*/
Expand Down Expand Up @@ -76,11 +84,17 @@ public class DatabasePersistence extends Persistence {
@VisibleForTesting
static final String COLUMN_TARGET_KEY = "target_key";

/**
* Priority.
*/
@VisibleForTesting
static final String COLUMN_PRIORITY = "priority";

/**
* Table schema for Persistence.
*/
@VisibleForTesting
static final ContentValues SCHEMA = getContentValues("", "", "", "", "");
static final ContentValues SCHEMA = getContentValues("", "", "", "", "", 0);

/**
* Database name.
Expand All @@ -97,7 +111,7 @@ public class DatabasePersistence extends Persistence {
/**
* Current version of the schema.
*/
private static final int VERSION = 3;
private static final int VERSION = 4;

/**
* Size limit (in bytes) for a database row log payload.
Expand Down Expand Up @@ -174,7 +188,10 @@ public boolean onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
db.execSQL("ALTER TABLE " + TABLE + " ADD COLUMN `" + COLUMN_TARGET_TOKEN + "` TEXT");
db.execSQL("ALTER TABLE " + TABLE + " ADD COLUMN `" + COLUMN_DATA_TYPE + "` TEXT");
}
db.execSQL("ALTER TABLE " + TABLE + " ADD COLUMN `" + COLUMN_TARGET_KEY + "` TEXT");
if (oldVersion < VERSION_TARGET_KEY) {
db.execSQL("ALTER TABLE " + TABLE + " ADD COLUMN `" + COLUMN_TARGET_KEY + "` TEXT");
}
db.execSQL("ALTER TABLE " + TABLE + " ADD COLUMN `" + COLUMN_PRIORITY + "` INTEGER DEFAULT " + PERSISTENCE_NORMAL);
return true;
}
});
Expand All @@ -189,17 +206,19 @@ public boolean onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
*
* @param group The group of the storage for the log.
* @param logJ The JSON string for a log.
* @param targetToken target token if the log is common schema.
* @param targetKey project identifier part of the target token in clear text.
* @param targetToken The target token if the log is common schema.
* @param targetKey The project identifier part of the target token in clear text.
* @param priority The persistence priority.
* @return A {@link ContentValues} instance.
*/
private static ContentValues getContentValues(@Nullable String group, @Nullable String logJ, String targetToken, String type, String targetKey) {
private static ContentValues getContentValues(@Nullable String group, @Nullable String logJ, String targetToken, String type, String targetKey, int priority) {
ContentValues values = new ContentValues();
values.put(COLUMN_GROUP, group);
values.put(COLUMN_LOG, logJ);
values.put(COLUMN_TARGET_TOKEN, targetToken);
values.put(COLUMN_DATA_TYPE, type);
values.put(COLUMN_TARGET_KEY, targetKey);
values.put(COLUMN_PRIORITY, priority);
return values;
}

Expand All @@ -209,7 +228,7 @@ public boolean setMaxStorageSize(long maxStorageSizeInBytes) {
}

@Override
public long putLog(@NonNull String group, @NonNull Log log) throws PersistenceException {
public long putLog(@NonNull String group, @NonNull Log log, @IntRange(from = Flags.PERSISTENCE_NORMAL, to = Flags.PERSISTENCE_CRITICAL) int priority) throws PersistenceException {

/* Convert log to JSON string and put in the database. */
try {
Expand All @@ -230,7 +249,7 @@ public long putLog(@NonNull String group, @NonNull Log log) throws PersistenceEx
targetKey = null;
targetToken = null;
}
contentValues = getContentValues(group, isLargePayload ? null : payload, targetToken, log.getType(), targetKey);
contentValues = getContentValues(group, isLargePayload ? null : payload, targetToken, log.getType(), targetKey, priority);
long databaseId = mDatabaseManager.put(contentValues);
if (databaseId == -1) {
throw new PersistenceException("Failed to store a log to the Persistence database for log type " + log.getType() + ".");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ public abstract class Persistence implements Closeable {
/**
* Writes a log to the storage with the given {@code group}.
*
* @param group The group of the storage for the log.
* @param log The log to be placed in the storage.
* @param group The group of the storage for the log.
* @param log The log to be placed in the storage.
* @param priority The persistence priority.
* @return Log identifier from persistence after saving.
* @throws PersistenceException Exception will be thrown if Persistence cannot write a log to the storage.
*/
public abstract long putLog(@NonNull String group, @NonNull Log log) throws PersistenceException;
public abstract long putLog(@NonNull String group, @NonNull Log log, int priority) throws PersistenceException;

/**
* Deletes a log with the give ID from the {@code group}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,9 @@ public void idHelper() {
public void appCenterLog() {
new AppCenterLog();
}

@Test
public void flags() {
new Flags();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.content.Context;

import com.microsoft.appcenter.Flags;
import com.microsoft.appcenter.http.ServiceCallback;
import com.microsoft.appcenter.ingestion.Ingestion;
import com.microsoft.appcenter.ingestion.models.Log;
Expand Down Expand Up @@ -51,9 +52,9 @@ public void nullAppSecretProvided() throws Persistence.PersistenceException {
/* Check enqueue. */
Log log = mock(Log.class);
channel.enqueue(log, TEST_GROUP);
verify(persistence, never()).putLog(TEST_GROUP, log);
verify(persistence, never()).putLog(TEST_GROUP, log, Flags.PERSISTENCE_NORMAL);
channel.enqueue(mock(Log.class), "other");
verify(persistence, never()).putLog(anyString(), any(Log.class));
verify(persistence, never()).putLog(anyString(), any(Log.class), eq(Flags.PERSISTENCE_NORMAL));

/* Check clear. Even without app secret it works as it could be logs from previous process. */
channel.clear(TEST_GROUP);
Expand Down Expand Up @@ -128,7 +129,7 @@ public void startWithoutAppSecret() throws Persistence.PersistenceException {
verify(defaultIngestion, never()).sendAsync(anyString(), any(UUID.class), any(LogContainer.class), any(ServiceCallback.class));

/* Verify we didn't persist the log since AppCenter not started with app secret. */
verify(mockPersistence, never()).putLog(eq(appCenterGroup), any(Log.class));
verify(mockPersistence, never()).putLog(eq(appCenterGroup), any(Log.class), eq(Flags.PERSISTENCE_NORMAL));

/* Enqueuing 1 event from one collector group. */
channel.enqueue(mock(Log.class), oneCollectorGroup);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.content.Context;

import com.microsoft.appcenter.Flags;
import com.microsoft.appcenter.ingestion.AppCenterIngestion;
import com.microsoft.appcenter.ingestion.Ingestion;
import com.microsoft.appcenter.ingestion.models.Log;
Expand All @@ -20,6 +21,7 @@
import static org.mockito.Matchers.anyListOf;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.notNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -135,7 +137,7 @@ public void filter() throws Persistence.PersistenceException {
verify(listener1).shouldFilter(log);
verify(listener2).onPreparingLog(log, TEST_GROUP);
verify(listener2).shouldFilter(log);
verify(persistence, never()).putLog(TEST_GROUP, log);
verify(persistence, never()).putLog(eq(TEST_GROUP), eq(log), anyInt());
}

/* Given 1 log. */
Expand All @@ -155,7 +157,7 @@ public void filter() throws Persistence.PersistenceException {

/* Second listener skipped since first listener filtered out. */
verify(listener2, never()).shouldFilter(log);
verify(persistence, never()).putLog(TEST_GROUP, log);
verify(persistence, never()).putLog(eq(TEST_GROUP), eq(log), anyInt());
}

/* Given 1 log. */
Expand All @@ -173,7 +175,7 @@ public void filter() throws Persistence.PersistenceException {
verify(listener1).shouldFilter(log);
verify(listener2).onPreparingLog(log, TEST_GROUP);
verify(listener2).shouldFilter(log);
verify(persistence).putLog(TEST_GROUP, log);
verify(persistence).putLog(TEST_GROUP, log, Flags.PERSISTENCE_NORMAL);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.content.Context;

import com.microsoft.appcenter.Flags;
import com.microsoft.appcenter.http.ServiceCallback;
import com.microsoft.appcenter.ingestion.AppCenterIngestion;
import com.microsoft.appcenter.ingestion.OneCollectorIngestion;
Expand Down Expand Up @@ -59,7 +60,7 @@ public void pauseResumeGroup() throws Persistence.PersistenceException {

/* 50 logs are persisted but never being sent to Ingestion. */
assertEquals(50, channel.getCounter(TEST_GROUP));
verify(mockPersistence, times(50)).putLog(eq(TEST_GROUP), any(Log.class));
verify(mockPersistence, times(50)).putLog(eq(TEST_GROUP), any(Log.class), eq(Flags.PERSISTENCE_NORMAL));
verify(mockIngestion, never()).sendAsync(anyString(), any(UUID.class), any(LogContainer.class), any(ServiceCallback.class));
verify(mockPersistence, never()).deleteLogs(any(String.class), any(String.class));
verify(mockListener, never()).onBeforeSending(any(Log.class));
Expand Down Expand Up @@ -143,7 +144,7 @@ public void pauseResumeTargetToken() throws Persistence.PersistenceException {
channel.enqueue(log, TEST_GROUP);

/* Verify persisted but not incrementing and checking logs. */
verify(persistence).putLog(TEST_GROUP, log);
verify(persistence).putLog(TEST_GROUP, log, Flags.PERSISTENCE_NORMAL);
assertEquals(0, channel.getCounter(TEST_GROUP));
verify(persistence, never()).countLogs(TEST_GROUP);
verify(ingestion, never()).sendAsync(anyString(), any(UUID.class), any(LogContainer.class), any(ServiceCallback.class));
Expand Down Expand Up @@ -207,7 +208,7 @@ public void pauseGroupPauseTargetResumeGroupResumeTarget() throws Persistence.Pe
channel.enqueue(log, TEST_GROUP);

/* Verify persisted but not incrementing and checking logs. */
verify(persistence).putLog(TEST_GROUP, log);
verify(persistence).putLog(TEST_GROUP, log, Flags.PERSISTENCE_NORMAL);
assertEquals(0, channel.getCounter(TEST_GROUP));
verify(ingestion, never()).sendAsync(anyString(), any(UUID.class), any(LogContainer.class), any(ServiceCallback.class));

Expand Down
Loading

0 comments on commit ca051cf

Please sign in to comment.