From 4f5d23740c76dcfcf855db8a2a9033bbb82fb700 Mon Sep 17 00:00:00 2001 From: Arpan Kaphle Date: Wed, 8 Mar 2023 19:24:44 +0000 Subject: Placing InitialPhase Emit in Proper Locations Note, some details, such as query start, can be optimized in future versions. For example, we could use min(provider_start_1, provider_start_2, ....) = query_start, and emit it. Alternatievely, we could do this in the backend. For now, I've gone and made it a general time that is un-emitted but exists as a utility locally. We may choose to emit it in future versions as more granular backend queries are developed. Bug: 270403549 Test: To be chained in, builds for now Change-Id: I6041bd6fd2d62c7ea501ccb3094a886dd9aa2758 --- .../server/credentials/ClearRequestSession.java | 6 ++-- .../server/credentials/CreateRequestSession.java | 6 ++-- .../credentials/CredentialManagerService.java | 36 ++++++++++++++++++---- .../server/credentials/GetRequestSession.java | 11 +++++-- .../server/credentials/MetricUtilities.java | 20 +++++++++++- .../android/server/credentials/RequestSession.java | 14 +++++++-- .../credentials/metrics/InitialPhaseMetric.java | 15 ++------- 7 files changed, 81 insertions(+), 27 deletions(-) diff --git a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java index e84f0cc17d05..94260e20c4e7 100644 --- a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java @@ -46,9 +46,11 @@ public final class ClearRequestSession extends RequestSession providers, int userId, ISetEnabledProvidersCallback callback) { @@ -654,6 +670,7 @@ public final class CredentialManagerService } MetricUtilities.logApiCalled(ApiName.IS_ENABLED_CREDENTIAL_PROVIDER_SERVICE, ApiStatus.SUCCESS, callingUid); + // TODO(b/271135048) - Update asap to use the new logging types return true; } } @@ -683,12 +700,15 @@ public final class CredentialManagerService mContext, userId, providerFilter, getEnabledProviders()); } + @SuppressWarnings("GuardedBy") // ErrorProne requires service.mLock which is the same + // this.mLock private Set getEnabledProviders() { Set enabledProviders = new HashSet<>(); synchronized (mLock) { runForUser( (service) -> { - enabledProviders.add(service.getCredentialProviderInfo().getServiceInfo()); + enabledProviders.add( + service.getCredentialProviderInfo().getServiceInfo()); }); } return enabledProviders; @@ -699,6 +719,7 @@ public final class CredentialManagerService ClearCredentialStateRequest request, IClearCredentialStateCallback callback, String callingPackage) { + final long timestampBegan = System.nanoTime(); Log.i(TAG, "starting clearCredentialState with callingPackage: " + callingPackage); final int userId = UserHandle.getCallingUserId(); int callingUid = Binder.getCallingUid(); @@ -716,7 +737,8 @@ public final class CredentialManagerService callback, request, constructCallingAppInfo(callingPackage, userId, null), - CancellationSignal.fromTransport(cancelTransport)); + CancellationSignal.fromTransport(cancelTransport), + timestampBegan); // Initiate all provider sessions // TODO: Determine if provider needs to have clear capability in their manifest @@ -735,6 +757,8 @@ public final class CredentialManagerService } } + finalizeAndEmitInitialPhaseMetric(session); + // Iterate over all provider sessions and invoke the request providerSessions.forEach(ProviderSession::invokeSession); return cancelTransport; diff --git a/services/credentials/java/com/android/server/credentials/GetRequestSession.java b/services/credentials/java/com/android/server/credentials/GetRequestSession.java index f59b32c667da..8e90c091da69 100644 --- a/services/credentials/java/com/android/server/credentials/GetRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/GetRequestSession.java @@ -19,6 +19,7 @@ package com.android.server.credentials; import android.annotation.Nullable; import android.content.ComponentName; import android.content.Context; +import android.credentials.CredentialOption; import android.credentials.CredentialProviderInfo; import android.credentials.GetCredentialException; import android.credentials.GetCredentialRequest; @@ -36,6 +37,7 @@ import com.android.server.credentials.metrics.ApiStatus; import com.android.server.credentials.metrics.ProviderStatusForMetrics; import java.util.ArrayList; +import java.util.stream.Collectors; /** * Central session for a single getCredentials request. This class listens to the @@ -47,9 +49,14 @@ public class GetRequestSession extends RequestSession implements CredentialManagerUi.CredentialMan protected final Map mProviders = new HashMap<>(); protected ChosenProviderMetric mChosenProviderMetric = new ChosenProviderMetric(); - //TODO improve design to allow grouped metrics per request + protected InitialPhaseMetric mInitialPhaseMetric = new InitialPhaseMetric(); protected final String mHybridService; @NonNull @@ -96,7 +97,7 @@ abstract class RequestSession implements CredentialManagerUi.CredentialMan @UserIdInt int userId, int callingUid, @NonNull T clientRequest, U clientCallback, @NonNull String requestType, CallingAppInfo callingAppInfo, - CancellationSignal cancellationSignal) { + CancellationSignal cancellationSignal, long timestampStarted) { mContext = context; mUserId = userId; mCallingUid = callingUid; @@ -111,6 +112,9 @@ abstract class RequestSession implements CredentialManagerUi.CredentialMan mUserId, this); mHybridService = context.getResources().getString( R.string.config_defaultCredentialManagerHybridService); + mInitialPhaseMetric.setCredentialServiceStartedTimeNanoseconds(timestampStarted); + mInitialPhaseMetric.setSessionId(mRequestId.hashCode()); + mInitialPhaseMetric.setCallerUid(mCallingUid); } public abstract ProviderSession initiateProviderSession(CredentialProviderInfo providerInfo, @@ -118,6 +122,12 @@ abstract class RequestSession implements CredentialManagerUi.CredentialMan protected abstract void launchUiWithProviderData(ArrayList providerDataList); + // Sets up the initial metric collector for use across all request session impls + protected void setupInitialPhaseMetric(int metricCode, int requestClassType) { + this.mInitialPhaseMetric.setApiName(metricCode); + this.mInitialPhaseMetric.setCountRequestClassType(requestClassType); + } + // UI callbacks @Override // from CredentialManagerUiCallbacks diff --git a/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java index 5f062b05df67..31c6f6fb6a03 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java @@ -25,6 +25,8 @@ package com.android.server.credentials.metrics; */ public class InitialPhaseMetric { private static final String TAG = "PreCandidateMetric"; + // A sequence id to order united emits, due to split, this will statically always be 1 + public static final int SEQUENCE_ID = 1; // The api being called, default set to unknown private int mApiName = ApiName.UNKNOWN.getMetricCode(); @@ -32,8 +34,6 @@ public class InitialPhaseMetric { private int mCallerUid = -1; // The session id to unite multiple atom emits, default to -1 private long mSessionId = -1; - // A sequence id to order united emits, default to -1 - private int mSequenceId = -1; private int mCountRequestClassType = -1; // Raw timestamps in nanoseconds, *the only* one logged as such (i.e. 64 bits) since it is a @@ -50,7 +50,7 @@ public class InitialPhaseMetric { /* ---------- Latencies ---------- */ - /* -- Direct Latencies -- */ + /* -- Direct Latency Utility -- */ public int getServiceStartToQueryLatencyMicroseconds() { return (int) ((this.mCredentialServiceStartedTimeNanoseconds @@ -108,15 +108,6 @@ public class InitialPhaseMetric { return mSessionId; } - /* ------ SequenceId ------ */ - - public void setSequenceId(int sequenceId) { - mSequenceId = sequenceId; - } - - public int getSequenceId() { - return mSequenceId; - } /* ------ Count Request Class Types ------ */ -- cgit v1.2.3-59-g8ed1b