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

Commit 016f98a4 authored by Arpan Kaphle's avatar Arpan Kaphle Committed by Arpan
Browse files

[25Q3] Default vals on exception + strange histograms 

This fixes the default values on exceptions bug, and also may contribute to a fix on the strange histograms in the candidate phase issue we've been seeing.

For the default values, we were missing a 'handover' method from candidate to final atom *when there were failures*. This means successful cases looked accurate, whereas failing cases had inaccuracies and default values.

This same reason was - only in failing cases - not fully transferring the provider data's 'responseCollective' (aka aggregate information). This, along with the recent Samsung bug finding on UIs, may be the root cause of our discrepent histogram issues.

Bug: 405199548
Bug: 402180648
Test: Build and StatsD
Flag: android.credentials.flags.metric_bugfixes_continued
Change-Id: I9cc798ae454630870160fc5bd2e5fdae87850288
parent c018324a

services/credentials/java/com/android/server/credentials/ClearRequestSession.java

+6 −0
Original line number Original line Diff line number Diff line
@@ -24,6 +24,7 @@ import android.credentials.ClearCredentialStateRequest; 
import android.credentials.CredentialManager;

import android.credentials.CredentialManager;

import android.credentials.CredentialProviderInfo;

import android.credentials.CredentialProviderInfo;

import android.credentials.IClearCredentialStateCallback;

import android.credentials.IClearCredentialStateCallback;

import android.credentials.flags.Flags;

import android.credentials.selection.ProviderData;

import android.credentials.selection.ProviderData;

import android.credentials.selection.RequestInfo;

import android.credentials.selection.RequestInfo;

import android.os.CancellationSignal;

import android.os.CancellationSignal;
@@ -140,6 +141,11 @@ public final class ClearRequestSession extends RequestSession<ClearCredentialSta 
            if (session.isProviderResponseSet()) {

            if (session.isProviderResponseSet()) {

                // If even one provider responded successfully, send back the response

                // If even one provider responded successfully, send back the response

                // TODO: Aggregate other exceptions

                // TODO: Aggregate other exceptions

                if (Flags.metricBugfixesContinued()) {

                    ComponentName componentName = session.getComponentName();

                    mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                            isPrimaryProviderViaProviderInfo(componentName));

                }

                respondToClientWithResponseAndFinish(null);

                respondToClientWithResponseAndFinish(null);

                return;

                return;

            }

            }

services/credentials/java/com/android/server/credentials/CreateRequestSession.java

+8 −0
Original line number Original line Diff line number Diff line
@@ -163,6 +163,10 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR 
    @Override

    @Override

    public void onFinalErrorReceived(ComponentName componentName, String errorType,

    public void onFinalErrorReceived(ComponentName componentName, String errorType,

            String message) {

            String message) {

        if (Flags.metricBugfixesContinued()) {

            mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                    isPrimaryProviderViaProviderInfo(componentName));

        }

        respondToClientWithErrorAndFinish(errorType, message);

        respondToClientWithErrorAndFinish(errorType, message);

    }

    }
@@ -199,6 +203,10 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR 
                getProviderDataAndInitiateUi();

                getProviderDataAndInitiateUi();

            } else {

            } else {

                String exception = CreateCredentialException.TYPE_NO_CREATE_OPTIONS;

                String exception = CreateCredentialException.TYPE_NO_CREATE_OPTIONS;

                if (Flags.metricBugfixesContinued()) {

                    mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                            isPrimaryProviderViaProviderInfo(componentName));

                }

                mRequestSessionMetric.collectFrameworkException(exception);

                mRequestSessionMetric.collectFrameworkException(exception);

                respondToClientWithErrorAndFinish(exception,

                respondToClientWithErrorAndFinish(exception,

                        "No create options available.");

                        "No create options available.");

services/credentials/java/com/android/server/credentials/GetRequestSession.java

+12 −0
Original line number Original line Diff line number Diff line
@@ -168,6 +168,10 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest, 
    @Override

    @Override

    public void onFinalErrorReceived(ComponentName componentName, String errorType,

    public void onFinalErrorReceived(ComponentName componentName, String errorType,

            String message) {

            String message) {

        if (Flags.metricBugfixesContinued()) {

            mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                    isPrimaryProviderViaProviderInfo(componentName));

        }

        respondToClientWithErrorAndFinish(errorType, message);

        respondToClientWithErrorAndFinish(errorType, message);

    }

    }
@@ -212,6 +216,10 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest, 
                getProviderDataAndInitiateUi();

                getProviderDataAndInitiateUi();

            } else {

            } else {

                String exception = GetCredentialException.TYPE_NO_CREDENTIAL;

                String exception = GetCredentialException.TYPE_NO_CREDENTIAL;

                if (Flags.metricBugfixesContinued()) {

                    mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                            isPrimaryProviderViaProviderInfo(componentName));

                }

                mRequestSessionMetric.collectFrameworkException(exception);

                mRequestSessionMetric.collectFrameworkException(exception);

                respondToClientWithErrorAndFinish(exception,

                respondToClientWithErrorAndFinish(exception,

                        "No credentials available");

                        "No credentials available");
@@ -235,6 +243,10 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest, 
        // Respond to client if all auth entries are empty and nothing else to show on the UI

        // Respond to client if all auth entries are empty and nothing else to show on the UI

        if (providerDataContainsEmptyAuthEntriesOnly()) {

        if (providerDataContainsEmptyAuthEntriesOnly()) {

            String exception = GetCredentialException.TYPE_NO_CREDENTIAL;

            String exception = GetCredentialException.TYPE_NO_CREDENTIAL;

            if (Flags.metricBugfixesContinued()) {

                mRequestSessionMetric.updateMetricsOnResponseReceived(mProviders, componentName,

                        isPrimaryProviderViaProviderInfo(componentName));

            }

            mRequestSessionMetric.collectFrameworkException(exception);

            mRequestSessionMetric.collectFrameworkException(exception);

            respondToClientWithErrorAndFinish(exception,

            respondToClientWithErrorAndFinish(exception,

                    "No credentials available");

                    "No credentials available");

services/credentials/java/com/android/server/credentials/RequestSession.java

+3 −0
Original line number Original line Diff line number Diff line
@@ -283,6 +283,9 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential 
            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*hasException=*/ false,

            mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*hasException=*/ false,

                    ProviderStatusForMetrics.FINAL_SUCCESS);

                    ProviderStatusForMetrics.FINAL_SUCCESS);

        }

        }

        if (Flags.metricBugfixesContinued()) {

            mRequestSessionMetric.captureMissingLogMetadata();

        }

    }

    }





    void cancelExistingPendingIntent() {

    void cancelExistingPendingIntent() {

services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java

+24 −0
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.credentials.metrics; 
import static com.android.server.credentials.MetricUtilities.DEFAULT_INT_32;

import static com.android.server.credentials.MetricUtilities.DEFAULT_INT_32;

import static com.android.server.credentials.MetricUtilities.DELTA_EXCEPTION_CUT;

import static com.android.server.credentials.MetricUtilities.DELTA_EXCEPTION_CUT;

import static com.android.server.credentials.MetricUtilities.DELTA_RESPONSES_CUT;

import static com.android.server.credentials.MetricUtilities.DELTA_RESPONSES_CUT;

import static com.android.server.credentials.MetricUtilities.UNIT;

import static com.android.server.credentials.MetricUtilities.generateMetricKey;

import static com.android.server.credentials.MetricUtilities.generateMetricKey;

import static com.android.server.credentials.MetricUtilities.logApiCalledAggregateCandidate;

import static com.android.server.credentials.MetricUtilities.logApiCalledAggregateCandidate;

import static com.android.server.credentials.MetricUtilities.logApiCalledAuthenticationMetric;

import static com.android.server.credentials.MetricUtilities.logApiCalledAuthenticationMetric;
@@ -337,6 +338,29 @@ public class RequestSessionMetric { 
        }

        }

    }

    }





    /**

     * In certain flows, such as exceptions where no provider is specified (e.g. user cancellations

     * or client cancellations), this can be used at the very final point to collect missing meta

     * data. Certain metadata, therefore, is undefined - specifically the provider. However, other

     * metadata, such as latency figures, can now be properly seen.

     */

    public void captureMissingLogMetadata() {

        try {

            if (mChosenProviderFinalPhaseMetric.getChosenUid() == DEFAULT_INT_32

                    && mChosenProviderFinalPhaseMetric.getQueryStartTimeNanoseconds() < UNIT) {

                mChosenProviderFinalPhaseMetric.setServiceBeganTimeNanoseconds(

                        mCandidateAggregateMetric.getServiceBeganTimeNanoseconds());

                mChosenProviderFinalPhaseMetric.setQueryStartTimeNanoseconds(

                        mCandidateAggregateMetric.getMinProviderTimestampNanoseconds());

                mChosenProviderFinalPhaseMetric.setQueryEndTimeNanoseconds(

                        mCandidateAggregateMetric.getMaxProviderTimestampNanoseconds());

                mChosenProviderFinalPhaseMetric.setFinalFinishTimeNanoseconds(System.nanoTime());

            }

        } catch (Exception e) {

            Slog.i(TAG, "Unexpected error during final phase missing metadata logging " + e);

        }

    }



    /**

    /**

     * Used to update metrics when a response is received in a RequestSession.

     * Used to update metrics when a response is received in a RequestSession.

     *

     *