Merge "Refine metrics for file operation failure." (829b4711) · Commits · e / os / android_packages_apps_DocumentsUI

src/com/android/documentsui/Metrics.java

+87 −0

Original line number	Diff line number	Diff line
		@@ -17,17 +17,21 @@
		package com.android.documentsui;

		import static android.os.Environment.STANDARD_DIRECTORIES;
		import static com.android.documentsui.DocumentsApplication.acquireUnstableProviderOrThrow;
		import static com.android.documentsui.base.Shared.DEBUG;

		import android.annotation.IntDef;
		import android.annotation.Nullable;
		import android.annotation.StringDef;
		import android.app.Activity;
		import android.content.ContentProviderClient;
		import android.content.Context;
		import android.content.Intent;
		import android.content.pm.ResolveInfo;
		import android.net.Uri;
		import android.os.RemoteException;
		import android.provider.DocumentsContract;
		import android.provider.DocumentsContract.Path;
		import android.provider.DocumentsProvider;
		import android.util.Log;

		@@ -37,6 +41,7 @@ import com.android.documentsui.base.RootInfo;
		import com.android.documentsui.base.State;
		import com.android.documentsui.base.State.ActionType;
		import com.android.documentsui.files.LauncherActivity;
		import com.android.documentsui.roots.RootsAccess;
		import com.android.documentsui.services.FileOperationService;
		import com.android.documentsui.services.FileOperationService.OpType;
		import com.android.internal.logging.MetricsLogger;
		@@ -68,6 +73,14 @@ public final class Metrics {
		private static final String COUNT_CREATE_AT_LOCATION = "docsui_create_at_location";
		private static final String COUNT_OPEN_AT_LOCATION = "docsui_open_at_location";
		private static final String COUNT_GET_CONTENT_AT_LOCATION = "docsui_get_content_at_location";
		private static final String COUNT_MEDIA_FILEOP_FAILURE = "docsui_media_fileop_failure";
		private static final String COUNT_DOWNLOADS_FILEOP_FAILURE = "docsui_downloads_fileop_failure";
		private static final String COUNT_INTERNAL_STORAGE_FILEOP_FAILURE
		= "docsui_internal_storage_fileop_failure";
		private static final String COUNT_EXTERNAL_STORAGE_FILEOP_FAILURE
		= "docsui_external_storage_fileop_failure";
		private static final String COUNT_MTP_FILEOP_FAILURE = "docsui_mtp_fileop_failure";
		private static final String COUNT_OTHER_FILEOP_FAILURE = "docsui_other_fileop_failure";

		// Indices for bucketing roots in the roots histogram. "Other" is the catch-all index for any
		// root that is not explicitly recognized by the Metrics code (see {@link
		@@ -210,6 +223,33 @@ public final class Metrics {
		@Retention(RetentionPolicy.SOURCE)
		public @interface Provider {}

		// Codes representing different types of sub-fileops. These are used for bucketing fileop
		// failures in COUNT_*_FILEOP_FAILURE.
		public static final int SUBFILEOP_QUERY_DOCUMENT = 1;
		public static final int SUBFILEOP_QUERY_CHILDREN = 2;
		public static final int SUBFILEOP_OPEN_FILE = 3;
		public static final int SUBFILEOP_READ_FILE = 4;
		public static final int SUBFILEOP_CREATE_DOCUMENT = 5;
		public static final int SUBFILEOP_WRITE_FILE = 6;
		public static final int SUBFILEOP_DELETE_DOCUMENT = 7;
		public static final int SUBFILEOP_OBTAIN_STREAM_TYPE = 8;
		public static final int SUBFILEOP_QUICK_MOVE = 9;
		public static final int SUBFILEOP_QUICK_COPY = 10;

		@IntDef(flag = false, value = {
		SUBFILEOP_QUERY_DOCUMENT,
		SUBFILEOP_QUERY_CHILDREN,
		SUBFILEOP_OPEN_FILE,
		SUBFILEOP_READ_FILE,
		SUBFILEOP_CREATE_DOCUMENT,
		SUBFILEOP_WRITE_FILE,
		SUBFILEOP_DELETE_DOCUMENT,
		SUBFILEOP_OBTAIN_STREAM_TYPE,
		SUBFILEOP_QUICK_MOVE,
		SUBFILEOP_QUICK_COPY
		})
		@Retention(RetentionPolicy.SOURCE)
		public @interface SubFileOp {}

		// Codes representing different user actions. These are used for bucketing stats in the
		// COUNT_USER_ACTION histogram.
		@@ -494,6 +534,28 @@ public final class Metrics {
		}
		}

		public static void logFileOperationFailure(
		Context context, @SubFileOp int subFileOp, Uri docUri) {
		final String authority = docUri.getAuthority();
		switch (authority) {
		case Providers.AUTHORITY_MEDIA:
		logHistogram(context, COUNT_MEDIA_FILEOP_FAILURE, subFileOp);
		break;
		case Providers.AUTHORITY_STORAGE:
		logStorageFileOperationFailure(context, subFileOp, docUri);
		break;
		case Providers.AUTHORITY_DOWNLOADS:
		logHistogram(context, COUNT_DOWNLOADS_FILEOP_FAILURE, subFileOp);
		break;
		case Providers.AUTHORITY_MTP:
		logHistogram(context, COUNT_MTP_FILEOP_FAILURE, subFileOp);
		break;
		default:
		logHistogram(context, COUNT_OTHER_FILEOP_FAILURE, subFileOp);
		break;
		}
		}

		/**
		* Logs create directory operation error. We do not differentiate between internal and external
		* locations, all create directory errors are logged under COUNT_FILEOP_SYSTEM. Call this when a
		@@ -661,6 +723,31 @@ public final class Metrics {
		logHistogram(context, COUNT_USER_ACTION, userAction);
		}

		private static void logStorageFileOperationFailure(
		Context context, @SubFileOp int subFileOp, Uri docUri) {
		assert(Providers.AUTHORITY_STORAGE.equals(docUri.getAuthority()));

		boolean isInternal;
		try (ContentProviderClient client = acquireUnstableProviderOrThrow(
		context.getContentResolver(), Providers.AUTHORITY_STORAGE)) {
		final Path path = DocumentsContract.findDocumentPath(client, docUri);

		final RootsAccess roots = DocumentsApplication.getRootsCache(context);
		final RootInfo root = roots.getRootOneshot(
		Providers.AUTHORITY_STORAGE, path.getRootId());
		isInternal = !root.supportsEject();
		} catch (RemoteException \| RuntimeException e) {
		Log.e(TAG, "Failed to obtain its root info. Log the metrics as internal.", e);
		// It's not very likely to have an external storage so log it as internal.
		isInternal = true;
		}

		final String histogram = isInternal
		? COUNT_INTERNAL_STORAGE_FILEOP_FAILURE
		: COUNT_EXTERNAL_STORAGE_FILEOP_FAILURE;
		logHistogram(context, histogram, subFileOp);
		}

		/**
		* Internal method for making a MetricsLogger.count call. Increments the given counter by 1.
		*

src/com/android/documentsui/services/CopyJob.java

+30 −0

Original line number	Diff line number	Diff line
		@@ -351,6 +351,8 @@ class CopyJob extends ResolvedResourcesJob {
		} catch (RemoteException \| RuntimeException e) {
		Log.e(TAG, "Provider side copy failed for: " + src.derivedUri
		+ " due to an exception.", e);
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_QUICK_COPY, src.derivedUri);
		}

		// If optimized copy fails, then fallback to byte-by-byte copy.
		@@ -374,6 +376,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		streamTypes = getContentResolver().getStreamTypes(src.derivedUri, "/");
		} catch (RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OBTAIN_STREAM_TYPE, src.derivedUri);
		throw new ResourceException(
		"Failed to obtain streamable types for %s due to an exception.",
		src.derivedUri, e);
		@@ -385,6 +389,8 @@ class CopyJob extends ResolvedResourcesJob {
		dstDisplayName = src.displayName +
		(extension != null ? "." + extension : src.displayName);
		} else {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OBTAIN_STREAM_TYPE, src.derivedUri);
		throw new ResourceException("Cannot copy virtual file %s. No streamable formats "
		+ "available.", src.derivedUri);
		}
		@@ -400,12 +406,16 @@ class CopyJob extends ResolvedResourcesJob {
		dstUri = DocumentsContract.createDocument(
		getClient(dest), dest.derivedUri, dstMimeType, dstDisplayName);
		} catch (RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_CREATE_DOCUMENT, dest.derivedUri);
		throw new ResourceException(
		"Couldn't create destination document " + dstDisplayName + " in directory %s "
		+ "due to an exception.", dest.derivedUri, e);
		}
		if (dstUri == null) {
		// If this is a directory, the entire subdir will not be copied over.
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_CREATE_DOCUMENT, dest.derivedUri);
		throw new ResourceException(
		"Couldn't create destination document " + dstDisplayName + " in directory %s.",
		dest.derivedUri);
		@@ -415,6 +425,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		dstInfo = DocumentInfo.fromUri(getContentResolver(), dstUri);
		} catch (FileNotFoundException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_QUERY_DOCUMENT, dstUri);
		throw new ResourceException("Could not load DocumentInfo for newly created file %s.",
		dstUri);
		}
		@@ -453,6 +465,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		cursor = getClient(srcDir).query(queryUri, queryColumns, null, null, null);
		} catch (RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_QUERY_CHILDREN, srcDir.derivedUri);
		throw new ResourceException("Failed to query children of %s due to an exception.",
		srcDir.derivedUri, e);
		}
		@@ -511,6 +525,8 @@ class CopyJob extends ResolvedResourcesJob {
		srcFileAsAsset = getClient(src).openTypedAssetFileDescriptor(
		src.derivedUri, mimeType, null, canceller);
		} catch (FileNotFoundException \| RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OPEN_FILE, src.derivedUri);
		throw new ResourceException("Failed to open a file as asset for %s due to an "
		+ "exception.", src.derivedUri, e);
		}
		@@ -518,6 +534,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		in = new AssetFileDescriptor.AutoCloseInputStream(srcFileAsAsset);
		} catch (IOException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OPEN_FILE, src.derivedUri);
		throw new ResourceException("Failed to open a file input stream for %s due "
		+ "an exception.", src.derivedUri, e);
		}
		@@ -525,6 +543,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		srcFile = getClient(src).openFile(src.derivedUri, "r", canceller);
		} catch (FileNotFoundException \| RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OPEN_FILE, src.derivedUri);
		throw new ResourceException(
		"Failed to open a file for %s due to an exception.", src.derivedUri, e);
		}
		@@ -534,6 +554,8 @@ class CopyJob extends ResolvedResourcesJob {
		try {
		dstFile = getClient(dest).openFile(dest.derivedUri, "w", canceller);
		} catch (FileNotFoundException \| RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_OPEN_FILE, dest.derivedUri);
		throw new ResourceException("Failed to open the destination file %s for writing "
		+ "due to an exception.", dest.derivedUri, e);
		}
		@@ -541,16 +563,20 @@ class CopyJob extends ResolvedResourcesJob {

		byte[] buffer = new byte[32 * 1024];
		int len;
		boolean reading = true;
		try {
		while ((len = in.read(buffer)) != -1) {
		if (isCanceled()) {
		if (DEBUG) Log.d(TAG, "Canceled copy mid-copy of: " + src.derivedUri);
		return;
		}
		reading = false;
		out.write(buffer, 0, len);
		makeCopyProgress(len);
		reading = true;
		}

		reading = false;
		// Need to invoke Os#fsync to ensure the file is written to the storage device.
		try {
		Os.fsync(dstFile.getFileDescriptor());
		@@ -566,6 +592,10 @@ class CopyJob extends ResolvedResourcesJob {
		IoUtils.close(dstFile.getFileDescriptor());
		srcFile.checkError();
		} catch (IOException e) {
		Metrics.logFileOperationFailure(
		appContext,
		reading ? Metrics.SUBFILEOP_READ_FILE : Metrics.SUBFILEOP_WRITE_FILE,
		reading ? src.derivedUri: dest.derivedUri);
		throw new ResourceException(
		"Failed to copy bytes from %s to %s due to an IO exception.",
		src.derivedUri, dest.derivedUri, e);

src/com/android/documentsui/services/DeleteJob.java

+2 −0

Original line number	Diff line number	Diff line
		@@ -113,6 +113,8 @@ final class DeleteJob extends ResolvedResourcesJob {
		try {
		deleteDocument(doc, parentDoc);
		} catch (ResourceException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_DELETE_DOCUMENT, doc.derivedUri);
		Log.e(TAG, "Failed to delete document @ " + doc.derivedUri, e);
		onFileFailed(doc);
		}

src/com/android/documentsui/services/MoveJob.java

+3 −0

Original line number	Diff line number	Diff line
		@@ -29,6 +29,7 @@ import android.provider.DocumentsContract;
		import android.provider.DocumentsContract.Document;
		import android.util.Log;

		import com.android.documentsui.Metrics;
		import com.android.documentsui.R;
		import com.android.documentsui.base.DocumentInfo;
		import com.android.documentsui.base.DocumentStack;
		@@ -149,6 +150,8 @@ final class MoveJob extends CopyJob {
		return;
		}
		} catch (RemoteException \| RuntimeException e) {
		Metrics.logFileOperationFailure(
		appContext, Metrics.SUBFILEOP_QUICK_MOVE, src.derivedUri);
		Log.e(TAG, "Provider side move failed for: " + src.derivedUri
		+ " due to an exception: ", e);
		}