Add callback for OneAuth for measuring Broker Discovery Client Perf, Fixes AB#3416279

changelog.txt

@@ -1,5 +1,6 @@
 vNext
 ----------
+- [MINOR] Add callback for OneAuth for measuring Broker Discovery Client Perf (#2796)
 - [MINOR] Add new span name for DELEGATION_CERT_INSTALL's telemetry (#2790)
 - [MINOR] Refactor getAccountByLocalAccountId (#2781)
 - [MINOR] Add OTel Benchmarker (#2786)

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt

@@ -275,19 +275,39 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
 
     override fun getActiveBroker(shouldSkipCache: Boolean): BrokerData? {
         return runBlocking {
-            return@runBlocking getActiveBrokerAsync(shouldSkipCache)
+            return@runBlocking getActiveBrokerAsync(shouldSkipCache, null)
         }
     }
 
-    private suspend fun getActiveBrokerAsync(shouldSkipCache:Boolean): BrokerData?{
+    override fun getActiveBroker(
+        shouldSkipCache: Boolean,
+        telemetryCallback: IBrokerDiscoveryClientTelemetryCallback
+    ): BrokerData? {
+        return runBlocking {
+            return@runBlocking getActiveBrokerAsync(shouldSkipCache, telemetryCallback)
+        }
+    }
+
+    private suspend fun getActiveBrokerAsync(shouldSkipCache:Boolean,
+                                             telemetryCallback: IBrokerDiscoveryClientTelemetryCallback?): BrokerData?{
         val methodTag = "$TAG:getActiveBrokerAsync"
+
+        val timeStartAcquiringLock = System.nanoTime()
         classLevelLock.withLock {
+            telemetryCallback?.onLockAcquired(System.nanoTime() - timeStartAcquiringLock)
             if (!shouldSkipCache) {
                 if (cache.shouldUseAccountManager()) {
+                    telemetryCallback?.onUseAccountManager()
                     return getActiveBrokerFromAccountManager()
                 }
+                val timeStartReadingFromCache = System.nanoTime()
                 cache.getCachedActiveBroker()?.let {
-                    if (!isPackageInstalled(it)) {
+                    telemetryCallback?.onReadFromCache(System.nanoTime() - timeStartReadingFromCache)
+
+                    val timeStartIsPackageInstalled = System.nanoTime()
+                    val isPackageInstalled = isPackageInstalled(it)
+                    telemetryCallback?.onFinishCheckingIfPackageIsInstalled(System.nanoTime() - timeStartIsPackageInstalled)
+                    if (!isPackageInstalled) {
                         Logger.info(
                             methodTag,
                             "There is a cached broker: $it, but the app is no longer installed."
@@ -296,7 +316,10 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
                         return@let
                     }
 
-                    if (!isValidBroker(it)) {
+                    val timeStartIsValidBroker = System.nanoTime()
+                    val isValidBroker = isValidBroker(it)
+                    telemetryCallback?.onFinishCheckingIfValidBroker(System.nanoTime() - timeStartIsValidBroker)
+                    if (!isValidBroker) {
                         Logger.info(
                             methodTag,
                             "Clearing cache as the installed app does not have a matching signature hash."
@@ -305,7 +328,11 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
                         return@let
                     }
 
-                    if(!ipcStrategy.isSupportedByTargetedBroker(it.packageName)){
+                    val timeStartIsSupportedByTargetedBroker = System.nanoTime()
+                    val isSupportedByTargetedBroker =
+                        ipcStrategy.isSupportedByTargetedBroker(it.packageName)
+                    telemetryCallback?.onFinishCheckingIfSupportedByTargetedBroker(System.nanoTime() - timeStartIsSupportedByTargetedBroker)
+                    if(!isSupportedByTargetedBroker){
                         Logger.info(
                             methodTag,
                             "Clearing cache as the installed app does not provide any IPC mechanism to communicate to. (e.g. the broker code isn't shipped with this apk)"
@@ -319,12 +346,14 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
                 }
             }
 
+            val timeStartQueryFromBroker = System.nanoTime()
             val brokerData = queryFromBroker(
                 brokerCandidates = brokerCandidates,
                 ipcStrategy = ipcStrategy,
                 isPackageInstalled = isPackageInstalled,
                 isValidBroker = isValidBroker
             )
+            telemetryCallback?.onFinishQueryingResultFromBroker(System.nanoTime() - timeStartQueryFromBroker)
 
             if (brokerData != null) {
                 cache.setCachedActiveBroker(brokerData)
@@ -343,6 +372,7 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
                 )
             )
 
+            telemetryCallback?.onUseAccountManager()
             val accountManagerResult = getActiveBrokerFromAccountManager()
             Logger.info(
                 methodTag, "Tried getting active broker from account manager, " +

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClient.kt

@@ -37,6 +37,20 @@ interface IBrokerDiscoveryClient {
      * **/
     fun getActiveBroker(shouldSkipCache: Boolean = false) : BrokerData?
 
+
+    /**
+     * Performs a discovery to figure out which broker app the SDK (MSAL/OneAuth)
+     * has to send its request to.
+     *
+     * @param shouldSkipCache       If true, this will skip cached value (if any)
+     *                              and always query the broker for the result.
+     * @param telemetryCallback     callback with telemetry data.
+     * @return BrokerData package name and signature hash of the targeted app.
+     * **/
+    fun getActiveBroker(shouldSkipCache: Boolean = false,
+                        telemetryCallback: IBrokerDiscoveryClientTelemetryCallback) : BrokerData?
+
+
     /**
      * Force a broker app to figure out which broker app the SDK (MSAL/OneAuth)
      * has to send its request to.

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt

@@ -0,0 +1,80 @@
+//  Copyright (c) Microsoft Corporation.
+//  All rights reserved.
+//
+//  This code is licensed under the MIT License.
+//
+//  Permission is hereby granted, free of charge, to any person obtaining a copy
+//  of this software and associated documentation files(the "Software"), to deal
+//  in the Software without restriction, including without limitation the rights
+//  to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+//  copies of the Software, and to permit persons to whom the Software is
+//  furnished to do so, subject to the following conditions :
+//
+//  The above copyright notice and this permission notice shall be included in
+//  all copies or substantial portions of the Software.
+//
+//  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+//  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+//  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+//  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+//  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+//  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+//  THE SOFTWARE.
+package com.microsoft.identity.common.internal.activebrokerdiscovery
+
+interface IBrokerDiscoveryClientTelemetryCallback {
+    /**
+     * Time spent acquiring the lock/mutex for broker discovery.
+     *
+     * @param timeSpentInNanoSeconds Time spent acquiring the lock/mutex (in nanoseconds).
+     **/
+    fun onLockAcquired(timeSpentInNanoSeconds: Long)
+
+    /**
+     * If triggered, the broker discovery client is using Account Manager
+     * (e.g. one of the installed broker is too old and doesn't support this new mechanism).
+     **/
+    fun onUseAccountManager()
+
+    /**
+     * Triggered when BrokerDiscoveryClient finishes reading from its cache.
+     *
+     * @param timeSpentInNanoSeconds Time spent reading the cache (in nanoseconds).
+     **/
+    fun onReadFromCache(timeSpentInNanoSeconds: Long)
+
+    /**
+     * Triggered when BrokerDiscoveryClient finishes validating if the cached broker package is installed.
+     *
+     * @param timeSpentInNanoSeconds Time spent to validate if the package is installed (in nanoseconds).
+     **/
+    fun onFinishCheckingIfPackageIsInstalled(timeSpentInNanoSeconds: Long)
+
+    /**
+     * Triggered when BrokerDiscoveryClient finishes validating if the cached broker package supports the given IPC strategy.
+     *
+     * @param timeSpentInNanoSeconds Time spent to validate if the package supports the IPC strategy (in nanoseconds).
+     **/
+    fun onFinishCheckingIfSupportedByTargetedBroker(timeSpentInNanoSeconds: Long)
+
+    /**
+     * Triggered when BrokerDiscoveryClient finishes validating if the cached broker package is a valid broker.
+     *
+     * @param timeSpentInNanoSeconds Time spent to validate if the package is a valid broker (in nanoseconds).
+     **/
+    fun onFinishCheckingIfValidBroker(timeSpentInNanoSeconds: Long)
+
+    /**
+     * Triggered when BrokerDiscoveryClient finishes querying active broker from one of the broker apps.
+     *
+     * There are 2 possible cases when this is triggered.
+     * 1. Both Broker and this MSAL/OneAuth app are freshly installed.
+     * This will trigger the broker discovery in broker, and will take longer time.
+     *
+     * 2. Broker has already done broker discovery before (e.g. by other MSAL/OneAuth app). Only this MSAL/OneAuth app is freshly installed.
+     * When the request reaches the broker, the broker would just return the cached value, which will be faster.
+     *
+     * @param timeSpentInNanoSeconds Time spent to query the active broker (in nanoseconds).
+     **/
+    fun onFinishQueryingResultFromBroker(timeSpentInNanoSeconds: Long)
+}

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/LegacyBrokerDiscoveryClient.kt

@@ -35,6 +35,15 @@ class LegacyBrokerDiscoveryClient(val context: Context): IBrokerDiscoveryClient
             .getActiveBrokerFromAccountManager()
     }
 
+    override fun getActiveBroker(
+        shouldSkipCache: Boolean,
+        telemetryCallback: IBrokerDiscoveryClientTelemetryCallback
+    ): BrokerData? {
+        telemetryCallback.onUseAccountManager()
+        return AccountManagerBrokerDiscoveryUtil(context)
+            .getActiveBrokerFromAccountManager()
+    }
+
     override fun forceBrokerRediscovery(brokerCandidate: BrokerData): BrokerData {
         throw UnsupportedOperationException("LegacyBrokerDiscoveryClient does not support forceBrokerRediscovery.")
     }