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

Commit 5180136a authored by Arpan Kaphle's avatar Arpan Kaphle Committed by Android (Google) Code Review
Browse files

Merge changes from topic "OrderOfLoggingCredMan" into main

* changes:
  Fix Order of Logging
  Add Flag for Metrics Deduplication Effort
parents 8a8d6e1d 52d6bcb8
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -144,3 +144,13 @@ flag {
        purpose: PURPOSE_BUGFIX
    }
}

flag {
    namespace: "credential_manager"
    name: "fix_metric_duplication_emits"
    description: "Fixes duplicate emits in the original metric emit system."
    bug: "362994633"
    metadata {
        purpose: PURPOSE_BUGFIX
    }
}
 No newline at end of file
+61 −11
Original line number Diff line number Diff line
@@ -258,12 +258,33 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
        if (propagateCancellation) {
            mProviders.values().forEach(ProviderSession::cancelProviderRemoteSession);
        }
        mRequestSessionMetric.logApiCalledAtFinish(apiStatus);
        mRequestSessionStatus = RequestSessionStatus.COMPLETE;
        if (Flags.fixMetricDuplicationEmits()) {
            logTrackOneCandidatesAndPrepareFinalPhaseLogs(apiStatus);
        }
        mRequestSessionMetric.logApiCalledAtFinish(apiStatus);
        mProviders.clear();
        clearRequestSessionLocked();
    }

    /**
     * Ensures all logging done in final phase methods only occur within the 'finishSession'.
     */
    private void logTrackOneCandidatesAndPrepareFinalPhaseLogs(int apiStatus) {
        mRequestSessionMetric.logCandidateAggregateMetrics(mProviders);
        if (isRespondingWithError(apiStatus)) {
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                    /*hasException=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
        } else if (isRespondingWithUserCanceledError(apiStatus)) {
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                    /*hasException=*/false, ProviderStatusForMetrics.FINAL_FAILURE
            );
        } else if (isRespondingWithSuccess(apiStatus)) {
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*hasException=*/ false,
                    ProviderStatusForMetrics.FINAL_SUCCESS);
        }
    }

    void cancelExistingPendingIntent() {
        if (mPendingIntent != null) {
            try {
@@ -343,9 +364,11 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
     * @param response the response associated with the API call that just completed
     */
    protected void respondToClientWithResponseAndFinish(V response) {
        if (!Flags.fixMetricDuplicationEmits()) {
            mRequestSessionMetric.logCandidateAggregateMetrics(mProviders);
        mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*has_exception=*/ false,
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*hasException=*/ false,
                    ProviderStatusForMetrics.FINAL_SUCCESS);
        }
        if (mRequestSessionStatus == RequestSessionStatus.COMPLETE) {
            Slog.w(TAG, "Request has already been completed. This is strange.");
            return;
@@ -360,8 +383,10 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
            finishSession(/*propagateCancellation=*/false,
                    ApiStatus.SUCCESS.getMetricCode());
        } catch (RemoteException e) {
            if (!Flags.fixMetricDuplicationEmits()) {
                mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                    /*has_exception=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
                        /*hasException=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
            }
            Slog.e(TAG, "Issue while responding to client with a response : " + e);
            finishSession(/*propagateCancellation=*/false, ApiStatus.FAILURE.getMetricCode());
        }
@@ -374,9 +399,11 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
     * @param errorMsg  the error message given back in the flow
     */
    protected void respondToClientWithErrorAndFinish(String errorType, String errorMsg) {
        if (!Flags.fixMetricDuplicationEmits()) {
            mRequestSessionMetric.logCandidateAggregateMetrics(mProviders);
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                /*has_exception=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
                    /*hasException=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
        }
        if (mRequestSessionStatus == RequestSessionStatus.COMPLETE) {
            Slog.w(TAG, "Request has already been completed. This is strange.");
            return;
@@ -385,7 +412,6 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
            finishSession(/*propagateCancellation=*/true, ApiStatus.CLIENT_CANCELED.getMetricCode());
            return;
        }

        try {
            invokeClientCallbackError(errorType, errorMsg);
        } catch (RemoteException e) {
@@ -393,7 +419,9 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
        }
        boolean isUserCanceled = errorType.contains(MetricUtilities.USER_CANCELED_SUBSTRING);
        if (isUserCanceled) {
            mRequestSessionMetric.setHasExceptionFinalPhase(/* has_exception */ false);
            if (!Flags.fixMetricDuplicationEmits()) {
                mRequestSessionMetric.setHasExceptionFinalPhase(/* hasException */ false);
            }
            finishSession(/*propagateCancellation=*/false,
                    ApiStatus.USER_CANCELED.getMetricCode());
        } else {
@@ -421,4 +449,26 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
            finishSession(isUiWaitingForData(), ApiStatus.CLIENT_CANCELED.getMetricCode());
        }
    }

    /**
     * This captures the final state of the apiStatus as presented in 'finishSession'.
     */
    private boolean isRespondingWithError(int apiStatus) {
        return apiStatus == ApiStatus.FAILURE.getMetricCode()
                || apiStatus == ApiStatus.CLIENT_CANCELED.getMetricCode();
    }

    /**
     * A unique failure case, where we do not set the exception bit to be true.
     */
    private boolean isRespondingWithUserCanceledError(int apiStatus) {
        return apiStatus == ApiStatus.USER_CANCELED.getMetricCode();
    }

    /**
     * This captures the final state of the apiStatus as presented in 'finishSession'.
     */
    private boolean isRespondingWithSuccess(int apiStatus) {
        return apiStatus == ApiStatus.SUCCESS.getMetricCode();
    }
}