summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Arpan Kaphle <akaphle@google.com> 2023-03-08 19:24:44 +0000
committer Arpan Kaphle <akaphle@google.com> 2023-03-09 20:02:45 +0000
commit4f5d23740c76dcfcf855db8a2a9033bbb82fb700 (patch)
treecc17a453807c80a8f08bf060f73884e8d86ea55e
parent07937a009ae0a5f11c818a0bd2b0458e92aed8dc (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
-rw-r--r--services/credentials/java/com/android/server/credentials/ClearRequestSession.java6
-rw-r--r--services/credentials/java/com/android/server/credentials/CreateRequestSession.java6
-rw-r--r--services/credentials/java/com/android/server/credentials/CredentialManagerService.java36
-rw-r--r--services/credentials/java/com/android/server/credentials/GetRequestSession.java11
-rw-r--r--services/credentials/java/com/android/server/credentials/MetricUtilities.java20
-rw-r--r--services/credentials/java/com/android/server/credentials/RequestSession.java14
-rw-r--r--services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java15
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 ------ */