Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 02fd92f8 authored by Garfield Tan's avatar Garfield Tan
Browse files

Refine metrics for file operation failure. 

* Report failures for each individual provider
* Report failures at sub-operations level
* Separate failures for external storage from internal storage

Note: It doesn't report failures when available bytes check fails or
moving cursor index fails. Delete failures also don't include failures
of cleanup after copy/move failures.

Test: Manual tests on file operations.
Bug: 33413461

Change-Id: I1713413f9498e8a264f8f54bffbfbad55f708b59
parent 35453d34

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);

                }