From b5ce61c19ff16e7937d8b82fe367dbf042832311 Mon Sep 17 00:00:00 2001 From: Arpan Kaphle Date: Tue, 25 Apr 2023 20:30:51 +0000 Subject: [PATCH] Optimizing Log Messages This aims to remove redundant messages, and converts logs to Slogs. It considers the go/greenlog message. Bug: 278591970 Bug: 270568738 Test: Build Test (Log statements only) Change-Id: I4a175501cb2cb85891c31c9f31b31cbd17bcd870 --- .../credentials/ClearRequestSession.java | 8 ++-- .../credentials/CreateRequestSession.java | 8 ++-- .../credentials/CredentialManagerService.java | 19 +++++----- .../CredentialManagerServiceImpl.java | 12 +++--- .../server/credentials/GetRequestSession.java | 8 ++-- .../server/credentials/MetricUtilities.java | 6 +-- .../credentials/PrepareGetRequestSession.java | 2 +- .../credentials/ProviderClearSession.java | 2 +- .../credentials/ProviderCreateSession.java | 18 ++++----- .../credentials/ProviderGetSession.java | 37 +++++++++---------- .../ProviderRegistryGetSession.java | 10 ++--- .../credentials/RemoteCredentialService.java | 4 +- .../server/credentials/RequestSession.java | 4 +- .../server/credentials/metrics/ApiName.java | 4 +- .../metrics/CandidateBrowsingPhaseMetric.java | 2 - .../metrics/CandidatePhaseMetric.java | 4 +- .../ChosenProviderFinalPhaseMetric.java | 4 +- .../server/credentials/metrics/EntryEnum.java | 4 +- .../metrics/InitialPhaseMetric.java | 8 ---- .../metrics/ProviderSessionMetric.java | 9 ++--- .../metrics/RequestSessionMetric.java | 32 ++++++++-------- .../metrics/shared/ResponseCollective.java | 13 ------- 22 files changed, 96 insertions(+), 122 deletions(-) diff --git a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java index 19a0c5e8adcb1..04ecd6ebd2d1d 100644 --- a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java @@ -67,7 +67,7 @@ public final class ClearRequestSession extends RequestSession requestOptions) { if (!requestOptions.isEmpty() && !isServiceCapableLocked(requestOptions)) { - Slog.d(TAG, "Service does not have the required capabilities"); + Slog.i(TAG, "Service does not have the required capabilities"); return null; } if (mInfo == null) { - Slog.w(TAG, "in initiateProviderSessionForRequest in CredManServiceImpl, " + Slog.w(TAG, "Initiating provider session for request " + "but mInfo is null. This shouldn't happen"); return null; } diff --git a/services/credentials/java/com/android/server/credentials/GetRequestSession.java b/services/credentials/java/com/android/server/credentials/GetRequestSession.java index 0271727249b12..15034104b5e0f 100644 --- a/services/credentials/java/com/android/server/credentials/GetRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/GetRequestSession.java @@ -72,7 +72,7 @@ public class GetRequestSession extends RequestSession filteredOptions = new ArrayList<>(); for (CredentialOption option : clientRequest.getCredentialOptions()) { if (providerCapabilities.contains(option.getType()) && isProviderAllowed(option, info.getComponentName()) && checkSystemProviderRequirement(option, info.isSystemProvider())) { - Slog.d(TAG, "Option of type: " + option.getType() + " meets all filtering" + Slog.i(TAG, "Option of type: " + option.getType() + " meets all filtering" + "conditions"); filteredOptions.add(option); } @@ -163,14 +163,14 @@ public final class ProviderGetSession extends ProviderSession implements CredentialManagerUi.Credential } protected void finishSession(boolean propagateCancellation) { - Slog.d(TAG, "finishing session with propagateCancellation " + propagateCancellation); + Slog.i(TAG, "finishing session with propagateCancellation " + propagateCancellation); if (propagateCancellation) { mProviders.values().forEach(ProviderSession::cancelProviderRemoteSession); } @@ -265,7 +265,7 @@ abstract class RequestSession implements CredentialManagerUi.Credential @NonNull protected ArrayList getProviderDataForUi() { - Slog.d(TAG, "In getProviderDataAndInitiateUi providers size: " + mProviders.size()); + Slog.i(TAG, "For ui, provider data size: " + mProviders.size()); ArrayList providerDataList = new ArrayList<>(); mRequestSessionMetric.logCandidatePhaseMetrics(mProviders); diff --git a/services/credentials/java/com/android/server/credentials/metrics/ApiName.java b/services/credentials/java/com/android/server/credentials/metrics/ApiName.java index b99f28d07f75f..1930a4859e87e 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/ApiName.java +++ b/services/credentials/java/com/android/server/credentials/metrics/ApiName.java @@ -27,7 +27,7 @@ import static com.android.internal.util.FrameworkStatsLog.CREDENTIAL_MANAGER_INI import static com.android.internal.util.FrameworkStatsLog.CREDENTIAL_MANAGER_INITIAL_PHASE_REPORTED__API_NAME__API_NAME_UNKNOWN; import android.credentials.ui.RequestInfo; -import android.util.Log; +import android.util.Slog; import java.util.AbstractMap; import java.util.Map; @@ -79,7 +79,7 @@ CREDENTIAL_MANAGER_INITIAL_PHASE_REPORTED__API_NAME__API_NAME_IS_ENABLED_CREDENT */ public static int getMetricCodeFromRequestInfo(String stringKey) { if (!sRequestInfoToMetric.containsKey(stringKey)) { - Log.w(TAG, "Attempted to use an unsupported string key request info"); + Slog.i(TAG, "Attempted to use an unsupported string key request info"); return UNKNOWN.mInnerMetricCode; } return sRequestInfoToMetric.get(stringKey); diff --git a/services/credentials/java/com/android/server/credentials/metrics/CandidateBrowsingPhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/CandidateBrowsingPhaseMetric.java index 0e1e03897bf18..07af6549411ea 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/CandidateBrowsingPhaseMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/CandidateBrowsingPhaseMetric.java @@ -27,8 +27,6 @@ package com.android.server.credentials.metrics; * though collection will begin in the candidate phase when the user begins browsing options. */ public class CandidateBrowsingPhaseMetric { - - private static final String TAG = "CandidateBrowsingPhaseMetric"; // The session id associated with the API Call this candidate provider is a part of, default -1 private int mSessionId = -1; // The EntryEnum that was pressed, defaults to -1 diff --git a/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java index e3e91cc54b29c..3ea9b1ce86f8e 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/CandidatePhaseMetric.java @@ -16,7 +16,7 @@ package com.android.server.credentials.metrics; -import android.util.Log; +import android.util.Slog; import com.android.server.credentials.MetricUtilities; import com.android.server.credentials.metrics.shared.ResponseCollective; @@ -112,7 +112,7 @@ public class CandidatePhaseMetric { */ public int getTimestampFromReferenceStartMicroseconds(long specificTimestamp) { if (specificTimestamp < mServiceBeganTimeNanoseconds) { - Log.i(TAG, "The timestamp is before service started, falling back to default int"); + Slog.i(TAG, "The timestamp is before service started, falling back to default int"); return MetricUtilities.DEFAULT_INT_32; } return (int) ((specificTimestamp diff --git a/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java b/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java index 64d33e6fdd766..93a82906aa503 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/ChosenProviderFinalPhaseMetric.java @@ -16,7 +16,7 @@ package com.android.server.credentials.metrics; -import android.util.Log; +import android.util.Slog; import com.android.server.credentials.MetricUtilities; import com.android.server.credentials.metrics.shared.ResponseCollective; @@ -216,7 +216,7 @@ public class ChosenProviderFinalPhaseMetric { */ public int getTimestampFromReferenceStartMicroseconds(long specificTimestamp) { if (specificTimestamp < mServiceBeganTimeNanoseconds) { - Log.i(TAG, "The timestamp is before service started, falling back to default int"); + Slog.i(TAG, "The timestamp is before service started, falling back to default int"); return MetricUtilities.DEFAULT_INT_32; } return (int) ((specificTimestamp diff --git a/services/credentials/java/com/android/server/credentials/metrics/EntryEnum.java b/services/credentials/java/com/android/server/credentials/metrics/EntryEnum.java index b9125ddf11452..530f01cbdfc56 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/EntryEnum.java +++ b/services/credentials/java/com/android/server/credentials/metrics/EntryEnum.java @@ -26,7 +26,7 @@ import static com.android.server.credentials.ProviderGetSession.AUTHENTICATION_A import static com.android.server.credentials.ProviderGetSession.CREDENTIAL_ENTRY_KEY; import static com.android.server.credentials.ProviderGetSession.REMOTE_ENTRY_KEY; -import android.util.Log; +import android.util.Slog; import java.util.AbstractMap; import java.util.Map; @@ -77,7 +77,7 @@ public enum EntryEnum { */ public static int getMetricCodeFromString(String stringKey) { if (!sKeyToEntryCode.containsKey(stringKey)) { - Log.w(TAG, "Attempted to use an unsupported string key entry type"); + Slog.i(TAG, "Attempted to use an unsupported string key entry type"); return UNKNOWN.mInnerMetricCode; } return sKeyToEntryCode.get(stringKey); 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 3f10109b52d54..060e56ce965b3 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/InitialPhaseMetric.java @@ -16,8 +16,6 @@ package com.android.server.credentials.metrics; -import android.util.Log; - import java.util.LinkedHashMap; import java.util.Map; @@ -142,9 +140,6 @@ public class InitialPhaseMetric { * @return a string array for deduped classtypes */ public String[] getUniqueRequestStrings() { - if (mRequestCounts.isEmpty()) { - Log.w(TAG, "There are no unique string request types collected"); - } String[] result = new String[mRequestCounts.keySet().size()]; mRequestCounts.keySet().toArray(result); return result; @@ -155,9 +150,6 @@ public class InitialPhaseMetric { * @return a string array for deduped classtype counts */ public int[] getUniqueRequestCounts() { - if (mRequestCounts.isEmpty()) { - Log.w(TAG, "There are no unique string request type counts collected"); - } return mRequestCounts.values().stream().mapToInt(Integer::intValue).toArray(); } } diff --git a/services/credentials/java/com/android/server/credentials/metrics/ProviderSessionMetric.java b/services/credentials/java/com/android/server/credentials/metrics/ProviderSessionMetric.java index 7b671469dc8c7..e618f3b356519 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/ProviderSessionMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/ProviderSessionMetric.java @@ -72,7 +72,7 @@ public class ProviderSessionMetric { try { mCandidatePhasePerProviderMetric.setFrameworkException(exceptionType); } catch (Exception e) { - Slog.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during candidate exception metric logging: " + e); } } @@ -101,7 +101,7 @@ public class ProviderSessionMetric { .getMetricCode()); } } catch (Exception e) { - Slog.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during candidate update metric logging: " + e); } } @@ -122,7 +122,7 @@ public class ProviderSessionMetric { initMetric.getCredentialServiceStartedTimeNanoseconds()); mCandidatePhasePerProviderMetric.setStartQueryTimeNanoseconds(System.nanoTime()); } catch (Exception e) { - Slog.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during candidate setup metric logging: " + e); } } @@ -144,9 +144,8 @@ public class ProviderSessionMetric { } else { Slog.i(TAG, "Your response type is unsupported for metric logging"); } - } catch (Exception e) { - Slog.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during candidate entry metric logging: " + e); } } diff --git a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java index b999b301d5cff..4624e0b3701a7 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java @@ -24,7 +24,7 @@ import static com.android.server.credentials.MetricUtilities.logApiCalledFinalPh import android.credentials.GetCredentialRequest; import android.credentials.ui.UserSelectionDialogResult; import android.os.IBinder; -import android.util.Log; +import android.util.Slog; import com.android.server.credentials.ProviderSession; @@ -90,7 +90,7 @@ public class RequestSessionMetric { mInitialPhaseMetric.setCallerUid(mCallingUid); mInitialPhaseMetric.setApiName(metricCode); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting initial metrics: " + e); } } @@ -103,7 +103,7 @@ public class RequestSessionMetric { try { mChosenProviderFinalPhaseMetric.setUiReturned(uiReturned); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting ui end time metric: " + e); } } @@ -116,7 +116,7 @@ public class RequestSessionMetric { try { mChosenProviderFinalPhaseMetric.setUiCallStartTimeNanoseconds(uiCallStartTime); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting ui start metric: " + e); } } @@ -132,7 +132,7 @@ public class RequestSessionMetric { mChosenProviderFinalPhaseMetric.setUiReturned(uiReturned); mChosenProviderFinalPhaseMetric.setUiCallEndTimeNanoseconds(uiEndTimestamp); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting ui response metric: " + e); } } @@ -146,7 +146,7 @@ public class RequestSessionMetric { try { mChosenProviderFinalPhaseMetric.setChosenProviderStatus(status); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error setting chosen provider status metric: " + e); } } @@ -159,7 +159,7 @@ public class RequestSessionMetric { try { mInitialPhaseMetric.setOriginSpecified(origin); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting create flow metric: " + e); } } @@ -175,7 +175,7 @@ public class RequestSessionMetric { uniqueRequestCounts.put(optionKey, uniqueRequestCounts.get(optionKey) + 1); }); } catch (Exception e) { - Log.w(TAG, "Unexpected error during get request metric logging: " + e); + Slog.i(TAG, "Unexpected error during get request metric logging: " + e); } return uniqueRequestCounts; } @@ -190,7 +190,7 @@ public class RequestSessionMetric { mInitialPhaseMetric.setOriginSpecified(request.getOrigin() != null); mInitialPhaseMetric.setRequestCounts(getRequestCountMap(request)); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting get flow metric: " + e); } } @@ -213,7 +213,7 @@ public class RequestSessionMetric { browsingPhaseMetric.setProviderUid(selectedProviderPhaseMetric.getCandidateUid()); mCandidateBrowsingPhaseMetric.add(browsingPhaseMetric); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error collecting browsing metric: " + e); } } @@ -226,7 +226,7 @@ public class RequestSessionMetric { try { mChosenProviderFinalPhaseMetric.setHasException(exceptionBitFinalPhase); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error setting final exception metric: " + e); } } @@ -244,7 +244,7 @@ public class RequestSessionMetric { mChosenProviderFinalPhaseMetric.setChosenProviderStatus( finalStatus.getMetricCode()); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during metric logging: " + e); } } @@ -276,7 +276,7 @@ public class RequestSessionMetric { candidatePhaseMetric.getResponseCollective()); mChosenProviderFinalPhaseMetric.setFinalFinishTimeNanoseconds(System.nanoTime()); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during metric candidate to final transfer: " + e); } } @@ -299,7 +299,7 @@ public class RequestSessionMetric { /* apiStatus */ ApiStatus.FAILURE.getMetricCode()); } } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during final metric failure emit: " + e); } } @@ -313,7 +313,7 @@ public class RequestSessionMetric { try { logApiCalledCandidatePhase(providers, ++mSequenceCounter, mInitialPhaseMetric); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during candidate metric emit: " + e); } } @@ -328,7 +328,7 @@ public class RequestSessionMetric { apiStatus, ++mSequenceCounter); } catch (Exception e) { - Log.w(TAG, "Unexpected error during metric logging: " + e); + Slog.i(TAG, "Unexpected error during final metric emit: " + e); } } diff --git a/services/credentials/java/com/android/server/credentials/metrics/shared/ResponseCollective.java b/services/credentials/java/com/android/server/credentials/metrics/shared/ResponseCollective.java index 951aca733b93e..fd785c2f4dfcd 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/shared/ResponseCollective.java +++ b/services/credentials/java/com/android/server/credentials/metrics/shared/ResponseCollective.java @@ -17,7 +17,6 @@ package com.android.server.credentials.metrics.shared; import android.annotation.NonNull; -import android.util.Slog; import com.android.server.credentials.metrics.EntryEnum; @@ -65,9 +64,6 @@ public class ResponseCollective { * @return a string array for deduped classtypes */ public String[] getUniqueResponseStrings() { - if (mResponseCounts.isEmpty()) { - Slog.w(TAG, "There are no unique string response types collected"); - } String[] result = new String[mResponseCounts.keySet().size()]; mResponseCounts.keySet().toArray(result); return result; @@ -79,9 +75,6 @@ public class ResponseCollective { * @return a string array for deduped classtype counts */ public int[] getUniqueResponseCounts() { - if (mResponseCounts.isEmpty()) { - Slog.w(TAG, "There are no unique string response type counts collected"); - } return mResponseCounts.values().stream().mapToInt(Integer::intValue).toArray(); } @@ -90,9 +83,6 @@ public class ResponseCollective { * @return an int array for deduped entries */ public int[] getUniqueEntries() { - if (mEntryCounts.isEmpty()) { - Slog.w(TAG, "There are no unique entry response types collected"); - } return mEntryCounts.keySet().stream().mapToInt(Enum::ordinal).toArray(); } @@ -102,9 +92,6 @@ public class ResponseCollective { * @return a string array for deduped classtype counts */ public int[] getUniqueEntryCounts() { - if (mEntryCounts.isEmpty()) { - Slog.w(TAG, "There are no unique entry response type counts collected"); - } return mEntryCounts.values().stream().mapToInt(Integer::intValue).toArray(); }