diff options
| author | 2023-03-08 19:24:44 +0000 | |
|---|---|---|
| committer | 2023-03-09 20:02:45 +0000 | |
| commit | 4f5d23740c76dcfcf855db8a2a9033bbb82fb700 (patch) | |
| tree | cc17a453807c80a8f08bf060f73884e8d86ea55e | |
| parent | 07937a009ae0a5f11c818a0bd2b0458e92aed8dc (diff) | |
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
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<ClearCredentialSta public ClearRequestSession(Context context, int userId, int callingUid, IClearCredentialStateCallback callback, ClearCredentialStateRequest request, - CallingAppInfo callingAppInfo, CancellationSignal cancellationSignal) { + CallingAppInfo callingAppInfo, CancellationSignal cancellationSignal, + long startedTimestamp) { super(context, userId, callingUid, request, callback, RequestInfo.TYPE_UNDEFINED, - callingAppInfo, cancellationSignal); + callingAppInfo, cancellationSignal, startedTimestamp); + setupInitialPhaseMetric(ApiName.CLEAR_CREDENTIAL.getMetricCode(), MetricUtilities.ZERO); } /** diff --git a/services/credentials/java/com/android/server/credentials/CreateRequestSession.java b/services/credentials/java/com/android/server/credentials/CreateRequestSession.java index 7e1780d05d75..47b8c7d30337 100644 --- a/services/credentials/java/com/android/server/credentials/CreateRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/CreateRequestSession.java @@ -53,9 +53,11 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR CreateCredentialRequest request, ICreateCredentialCallback callback, CallingAppInfo callingAppInfo, - CancellationSignal cancellationSignal) { + CancellationSignal cancellationSignal, + long startedTimestamp) { super(context, userId, callingUid, request, callback, RequestInfo.TYPE_CREATE, - callingAppInfo, cancellationSignal); + callingAppInfo, cancellationSignal, startedTimestamp); + setupInitialPhaseMetric(ApiName.CREATE_CREDENTIAL.getMetricCode(), MetricUtilities.UNIT); } /** diff --git a/services/credentials/java/com/android/server/credentials/CredentialManagerService.java b/services/credentials/java/com/android/server/credentials/CredentialManagerService.java index 6e998c4ef195..227ba12f62de 100644 --- a/services/credentials/java/com/android/server/credentials/CredentialManagerService.java +++ b/services/credentials/java/com/android/server/credentials/CredentialManagerService.java @@ -230,8 +230,10 @@ public final class CredentialManagerService if (hasPermission(android.Manifest.permission.LIST_ENABLED_CREDENTIAL_PROVIDERS)) { return; } - - throw new SecurityException("Caller is missing permission: QUERY_ALL_PACKAGES or LIST_ENABLED_CREDENTIAL_PROVIDERS"); + + throw new SecurityException( + "Caller is missing permission: QUERY_ALL_PACKAGES or " + + "LIST_ENABLED_CREDENTIAL_PROVIDERS"); } private boolean hasPermission(String permission) { @@ -408,6 +410,7 @@ public final class CredentialManagerService GetCredentialRequest request, IGetCredentialCallback callback, final String callingPackage) { + final long timestampBegan = System.nanoTime(); Log.i(TAG, "starting executeGetCredential with callingPackage: " + callingPackage); ICancellationSignal cancelTransport = CancellationSignal.createTransport(); @@ -429,7 +432,8 @@ public final class CredentialManagerService callback, request, constructCallingAppInfo(callingPackage, userId, request.getOrigin()), - CancellationSignal.fromTransport(cancelTransport)); + CancellationSignal.fromTransport(cancelTransport), + timestampBegan); processGetCredential(request, callback, session); return cancelTransport; @@ -508,6 +512,9 @@ public final class CredentialManagerService + e.getMessage()); } } + + finalizeAndEmitInitialPhaseMetric(session); + // TODO(b/271135048) - May still be worth emitting in the empty cases above. providerSessions.forEach(ProviderSession::invokeSession); } @@ -516,6 +523,7 @@ public final class CredentialManagerService CreateCredentialRequest request, ICreateCredentialCallback callback, String callingPackage) { + final long timestampBegan = System.nanoTime(); Log.i(TAG, "starting executeCreateCredential with callingPackage: " + callingPackage); ICancellationSignal cancelTransport = CancellationSignal.createTransport(); @@ -538,7 +546,8 @@ public final class CredentialManagerService request, callback, constructCallingAppInfo(callingPackage, userId, request.getOrigin()), - CancellationSignal.fromTransport(cancelTransport)); + CancellationSignal.fromTransport(cancelTransport), + timestampBegan); processCreateCredential(request, callback, session); return cancelTransport; @@ -566,10 +575,17 @@ public final class CredentialManagerService } } + finalizeAndEmitInitialPhaseMetric(session); // Iterate over all provider sessions and invoke the request providerSessions.forEach(ProviderSession::invokeSession); } + private void finalizeAndEmitInitialPhaseMetric(RequestSession session) { + var initMetric = session.mInitialPhaseMetric; + initMetric.setCredentialServiceBeginQueryTimeNanoseconds(System.nanoTime()); + MetricUtilities.logApiCalled(initMetric); + } + @Override public void setEnabledProviders( List<String> 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<ServiceInfo> getEnabledProviders() { Set<ServiceInfo> 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<GetCredentialRequest, private static final String TAG = "GetRequestSession"; public GetRequestSession(Context context, int userId, int callingUid, IGetCredentialCallback callback, GetCredentialRequest request, - CallingAppInfo callingAppInfo, CancellationSignal cancellationSignal) { + CallingAppInfo callingAppInfo, CancellationSignal cancellationSignal, + long startedTimestamp) { super(context, userId, callingUid, request, callback, RequestInfo.TYPE_GET, - callingAppInfo, cancellationSignal); + callingAppInfo, cancellationSignal, startedTimestamp); + int numTypes = (request.getCredentialOptions().stream() + .map(CredentialOption::getType).collect( + Collectors.toSet())).size(); // Dedupe type strings + setupInitialPhaseMetric(ApiName.GET_CREDENTIAL.getMetricCode(), numTypes); } /** diff --git a/services/credentials/java/com/android/server/credentials/MetricUtilities.java b/services/credentials/java/com/android/server/credentials/MetricUtilities.java index 255b2f80ff44..10ccd8bb6ccc 100644 --- a/services/credentials/java/com/android/server/credentials/MetricUtilities.java +++ b/services/credentials/java/com/android/server/credentials/MetricUtilities.java @@ -26,6 +26,7 @@ import com.android.server.credentials.metrics.ApiName; import com.android.server.credentials.metrics.ApiStatus; import com.android.server.credentials.metrics.CandidateProviderMetric; import com.android.server.credentials.metrics.ChosenProviderMetric; +import com.android.server.credentials.metrics.InitialPhaseMetric; import java.util.Map; @@ -39,7 +40,10 @@ public class MetricUtilities { public static final int DEFAULT_INT_32 = -1; public static final int[] DEFAULT_REPEATED_INT_32 = new int[0]; - + // Used for single count metric emits, such as singular amounts of various types + public static final int UNIT = 1; + // Used for zero count metric emits, such as zero amounts of various types + public static final int ZERO = 0; /** * This retrieves the uid of any package name, given a context and a component name for the @@ -155,4 +159,18 @@ public class MetricUtilities { } } + /** + * Handles the metric emit for the initial phase. + * + * @param initialPhaseMetric contains all the data for this emit + */ + protected static void logApiCalled(InitialPhaseMetric initialPhaseMetric) { + /* + FrameworkStatsLog.write(FrameworkStatsLog.INITIAL_PHASE, + .. session_id .. initialPhaseMetric.getSessionId(), + ... + TODO Immediately - Fill in asap now that the split atom is checked in. + */ + } + } diff --git a/services/credentials/java/com/android/server/credentials/RequestSession.java b/services/credentials/java/com/android/server/credentials/RequestSession.java index c1f35d0f8195..f9274079b2b8 100644 --- a/services/credentials/java/com/android/server/credentials/RequestSession.java +++ b/services/credentials/java/com/android/server/credentials/RequestSession.java @@ -38,6 +38,7 @@ import com.android.server.credentials.metrics.ApiName; import com.android.server.credentials.metrics.ApiStatus; import com.android.server.credentials.metrics.CandidateProviderMetric; import com.android.server.credentials.metrics.ChosenProviderMetric; +import com.android.server.credentials.metrics.InitialPhaseMetric; import java.util.ArrayList; import java.util.HashMap; @@ -75,7 +76,7 @@ abstract class RequestSession<T, U> implements CredentialManagerUi.CredentialMan protected final Map<String, ProviderSession> 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<T, U> 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<T, U> 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<T, U> implements CredentialManagerUi.CredentialMan protected abstract void launchUiWithProviderData(ArrayList<ProviderData> 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 ------ */ |