Skip to content

Commit 3273fa3

Browse files
authored
Using Baggage to propagate attributes from parent Span, Fixes AB#3262849 (#2671)
### Summary The passkey telemetry dashboard (and maybe some others) has had issues with latency due to joins being done between the Fido span and the parent span related to acquire token interactive. These join statements by trace id allowed us to associate attributes (such as tenant id) to the Fido span that weren't available in the context of the Fido classes. In order to directly save the attributes onto the Fido span itself, we can make use of [Otel's Baggage](https://opentelemetry.io/docs/specs/otel/baggage/api/), which is meant to help hold data across multiple spans (from parent to children). We can then use an implementation of [Otel's TextMapPropagator](https://opentelemetry.io/docs/specs/otel/context/api-propagators/) to serialize the Otel Context and pass it to the AuthenticationActivity as an Extra (similar to what we are doing for spanContext). This Context can then be accessed by the WebViewClient and Fido related classes, where they will retrieve the Baggage. This PR has the changes to implement this behavior and adds a few static helper classes for TextMapPropagator and Baggage (the baggage class is more relevant for the corresponding Broker PR: AzureAD/ad-accounts-for-android#3119). While this PR tackles attribute propagation for AuthorizationActivity specifically, Baggage and the helper classes can be used in other areas of the code where propagation is needed as well. [AB#3262849](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/3262849)
1 parent 28d09ea commit 3273fa3

File tree

13 files changed

+411
-18
lines changed

13 files changed

+411
-18
lines changed

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ vNext
33
- [PATCH] Fix caching of secret key and add retries for InvalidKeyException during unwrap (#2659)
44
- [MINOR] Replace AbstractSecretKeyLoader with ISecretKeyProvider (#2666)
55
- [MINOR] Update IP phone app teams signature constants to use SHA-512 format (#2700)
6+
- [MINOR] Using Baggage to propagate attributes from parent Span (#2671)
67
- [PATCH] Fix a few small switch browser bugs (#2710)
78

89
Version 21.4.0

common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,6 +2026,8 @@ public static final class AuthorizationIntentKey {
20262026
public static final String WEB_VIEW_ZOOM_CONTROLS_ENABLED = "com.microsoft.identity.web.view.zoom.controls.enabled";
20272027

20282028
public static final String WEB_VIEW_ZOOM_ENABLED = "com.microsoft.identity.web.view.zoom.enabled";
2029+
2030+
public static final String OTEL_CONTEXT_CARRIER = "otel_context_carrier";
20292031
}
20302032

20312033
public static final class AuthorizationIntentAction {

common/src/main/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandler.kt

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@ import com.microsoft.identity.common.internal.ui.webview.challengehandlers.IChal
3030
import com.microsoft.identity.common.java.constants.FidoConstants
3131
import com.microsoft.identity.common.java.constants.FidoConstants.Companion.PASSKEY_PROTOCOL_ERROR_PREFIX_STRING
3232
import com.microsoft.identity.common.java.opentelemetry.AttributeName
33+
import com.microsoft.identity.common.java.opentelemetry.BaggageExtension
3334
import com.microsoft.identity.common.java.opentelemetry.OTelUtility
35+
import com.microsoft.identity.common.java.opentelemetry.SpanExtension
3436
import com.microsoft.identity.common.java.opentelemetry.SpanName
3537
import com.microsoft.identity.common.logging.Logger
3638
import io.opentelemetry.api.trace.Span
37-
import io.opentelemetry.api.trace.SpanContext
3839
import io.opentelemetry.api.trace.StatusCode
40+
import io.opentelemetry.context.Context
3941
import kotlinx.coroutines.CancellationException
4042
import kotlinx.coroutines.launch
4143
import java.util.*
@@ -51,18 +53,35 @@ import java.util.*
5153
class AuthFidoChallengeHandler (
5254
private val fidoManager: IFidoManager,
5355
private val webView: WebView,
54-
private val spanContext : SpanContext?,
56+
private val oTelContext: Context?,
5557
private val lifecycleOwner: LifecycleOwner?
5658
) : IChallengeHandler<FidoChallenge, Void> {
5759
val TAG = AuthFidoChallengeHandler::class.simpleName.toString()
60+
companion object {
61+
private val parentAttributeNames = arrayListOf(
62+
AttributeName.correlation_id,
63+
AttributeName.tenant_id,
64+
AttributeName.account_type,
65+
AttributeName.calling_package_name
66+
)
67+
}
5868

5969
override fun processChallenge(fidoChallenge: FidoChallenge): Void? {
6070
val methodTag = "$TAG:processChallenge"
6171
Logger.info(methodTag, "Processing FIDO challenge.")
62-
val span = if (spanContext != null) {
63-
OTelUtility.createSpanFromParent(SpanName.Fido.name, spanContext)
72+
val span : Span
73+
if (oTelContext != null) {
74+
val parentSpan = SpanExtension.fromContext(oTelContext)
75+
val spanContext = parentSpan.spanContext
76+
span = OTelUtility.createSpanFromParent(SpanName.Fido.name, spanContext)
77+
val baggage = BaggageExtension.fromContext(oTelContext)
78+
parentAttributeNames.forEach { attributeName ->
79+
baggage.getEntryValue(attributeName.name)?.let { value ->
80+
span.setAttribute(attributeName.name, value)
81+
}
82+
}
6483
} else {
65-
OTelUtility.createSpan(SpanName.Fido.name)
84+
span = OTelUtility.createSpan(SpanName.Fido.name)
6685
}
6786
span.setAttribute(
6887
AttributeName.fido_challenge_handler.name,

common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationActivity.java

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common.internal.providers.oauth2;
2424

25+
import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.OTEL_CONTEXT_CARRIER;
26+
2527
import android.os.Bundle;
2628

2729
import androidx.annotation.Nullable;
@@ -31,9 +33,14 @@
3133
import com.microsoft.identity.common.internal.util.CommonMoshiJsonAdapter;
3234
import com.microsoft.identity.common.java.exception.TerminalException;
3335
import com.microsoft.identity.common.java.opentelemetry.SerializableSpanContext;
36+
import com.microsoft.identity.common.java.opentelemetry.TextMapPropagatorExtension;
37+
import com.microsoft.identity.common.java.util.StringUtil;
3438
import com.microsoft.identity.common.logging.Logger;
3539

40+
import java.util.HashMap;
41+
3642
import io.opentelemetry.api.trace.SpanContext;
43+
import io.opentelemetry.context.Context;
3744
import lombok.Getter;
3845
import lombok.experimental.Accessors;
3946

@@ -43,6 +50,11 @@ public class AuthorizationActivity extends DualScreenActivity {
4350
@Getter
4451
@Accessors(prefix = "m")
4552
private SpanContext mSpanContext;
53+
54+
@Getter
55+
@Accessors(prefix = "m")
56+
private Context mOtelContext;
57+
4658
private AuthorizationFragment mFragment;
4759

4860
public AuthorizationFragment getFragment() {
@@ -54,21 +66,32 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
5466
super.onCreate(savedInstanceState);
5567

5668
final String methodTag = TAG + ":onCreate";
57-
if (getIntent().getExtras() != null) {
69+
final Bundle bundle = getIntent().getExtras();
70+
if (bundle != null) {
5871
try {
59-
mSpanContext = new CommonMoshiJsonAdapter().fromJson(
60-
getIntent().getExtras().getString(SerializableSpanContext.SERIALIZABLE_SPAN_CONTEXT),
72+
String spanContextJson = bundle.getString(SerializableSpanContext.SERIALIZABLE_SPAN_CONTEXT);
73+
mSpanContext = StringUtil.isNullOrEmpty(spanContextJson) ? null : new CommonMoshiJsonAdapter().fromJson(
74+
spanContextJson,
6175
SerializableSpanContext.class
6276
);
63-
} catch (final TerminalException e) {
64-
// Don't want to block any features if an error occurs during deserialization of the span context.
65-
mSpanContext = null;
77+
78+
final HashMap<String, String> carrier;
79+
// For Android Tiramisu and above, we use the new Serializable interface, which is a bit safer because it performs type checking at the framework level.
80+
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.TIRAMISU) {
81+
carrier = bundle.getSerializable(OTEL_CONTEXT_CARRIER, HashMap.class);
82+
} else {
83+
carrier = (HashMap<String, String>) bundle.getSerializable(OTEL_CONTEXT_CARRIER);
84+
}
85+
mOtelContext = TextMapPropagatorExtension.extract(carrier);
86+
} catch (final TerminalException | ClassCastException | NullPointerException e) {
87+
// Don't want to block any features if an error occurs during deserialization.
88+
Logger.error(methodTag, "Exception thrown during extraction: " + e.getMessage(), e);
6689
}
6790
}
6891
final Fragment fragment = AuthorizationActivityFactory.getAuthorizationFragmentFromStartIntent(getIntent());
6992
if (fragment instanceof AuthorizationFragment) {
7093
mFragment = (AuthorizationFragment) fragment;
71-
mFragment.setInstanceState(getIntent().getExtras());
94+
mFragment.setInstanceState(bundle);
7295
} else {
7396
final IllegalStateException ex = new IllegalStateException("Unexpected fragment type.");
7497
Logger.error(methodTag, "Did not receive AuthorizationFragment from factory", ex);

common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationActivityFactory.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import android.content.Intent
2626
import android.os.Bundle
2727
import androidx.fragment.app.Fragment
2828
import com.microsoft.identity.common.adal.internal.AuthenticationConstants
29+
import com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.OTEL_CONTEXT_CARRIER
2930
import com.microsoft.identity.common.internal.msafederation.getIdProviderExtraQueryParamForAuthorization
3031
import com.microsoft.identity.common.internal.msafederation.getIdProviderHeadersForAuthorization
3132
import com.microsoft.identity.common.internal.msafederation.google.SignInWithGoogleApi.Companion.getInstance
@@ -39,8 +40,10 @@ import com.microsoft.identity.common.java.AuthenticationConstants.SdkPlatformFie
3940
import com.microsoft.identity.common.java.configuration.LibraryConfiguration
4041
import com.microsoft.identity.common.java.exception.ClientException
4142
import com.microsoft.identity.common.java.logging.DiagnosticContext
43+
import com.microsoft.identity.common.java.opentelemetry.OtelContextExtension
4244
import com.microsoft.identity.common.java.opentelemetry.SerializableSpanContext
4345
import com.microsoft.identity.common.java.opentelemetry.SpanExtension
46+
import com.microsoft.identity.common.java.opentelemetry.TextMapPropagatorExtension
4447
import com.microsoft.identity.common.java.ui.AuthorizationAgent
4548
import com.microsoft.identity.common.java.util.CommonURIBuilder
4649
import java.net.URISyntaxException
@@ -119,6 +122,10 @@ object AuthorizationActivityFactory {
119122
.build()
120123
)
121124
)
125+
putExtra(
126+
OTEL_CONTEXT_CARRIER,
127+
TextMapPropagatorExtension.inject(OtelContextExtension.current())
128+
)
122129
if (parameters.sourceLibraryName != null) {
123130
putExtra(PRODUCT, parameters.sourceLibraryName)
124131
}

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@
104104
import io.opentelemetry.api.trace.Span;
105105
import io.opentelemetry.api.trace.SpanContext;
106106
import io.opentelemetry.api.trace.StatusCode;
107+
import io.opentelemetry.context.Context;
107108
import io.opentelemetry.context.Scope;
108109

109110
/**
@@ -269,7 +270,7 @@ private boolean handleUrl(final WebView view, final String url) {
269270
Logger.info(methodTag,"WebView detected request for passkey protocol.");
270271
final FidoChallenge challenge = FidoChallenge.createFromRedirectUri(url);
271272
final Activity currentActivity = getActivity();
272-
final SpanContext spanContext = currentActivity instanceof AuthorizationActivity ? ((AuthorizationActivity)currentActivity).getSpanContext() : null;
273+
final Context oTelContext = currentActivity instanceof AuthorizationActivity ? ((AuthorizationActivity)currentActivity).getOtelContext() : null;
273274
// The legacyManager should only be getting added if the device is on Android 13 or lower and the library is MSAL/OneAuth with fragment or dialog mode.
274275
// The legacyManager logic should be removed once a larger majority of users are on Android 14+.
275276
final IFidoManager legacyManager =
@@ -285,7 +286,7 @@ private boolean handleUrl(final WebView view, final String url) {
285286
legacyManager
286287
),
287288
view,
288-
spanContext,
289+
oTelContext,
289290
ViewTreeLifecycleOwner.get(view));
290291
challengeHandler.processChallenge(challenge);
291292
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_ATTACH_NEW_PRT_HEADER_WHEN_NONCE_EXPIRED) && isNonceRedirect(formattedURL)) {

common/src/test/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandlerTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class AuthFidoChallengeHandlerTest {
6666
authFidoChallengeHandler = AuthFidoChallengeHandler(
6767
fidoManager = testFidoManager,
6868
webView = webView,
69-
spanContext = null,
69+
oTelContext = null,
7070
lifecycleOwner = testLifecycleOwner
7171
)
7272
}
@@ -126,7 +126,7 @@ class AuthFidoChallengeHandlerTest {
126126
authFidoChallengeHandler = AuthFidoChallengeHandler(
127127
fidoManager = testFidoManager,
128128
webView = webView,
129-
spanContext = null,
129+
oTelContext = null,
130130
lifecycleOwner = null
131131
)
132132
assertFalse(webView.urlLoaded)

common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,29 @@ public enum AttributeName {
365365
*/
366366
is_switch_browser_resume_handled,
367367

368+
/**
369+
* The tenant id for the home tenant of the account for which PRT is required.
370+
*/
371+
tenant_id,
372+
373+
/**
374+
* Indicates the type of account such as AAD or MSA.
375+
*/
376+
account_type,
377+
378+
/**
379+
* Indicates the broker app that emits the event.
380+
* The broker is not necessarily the active broker.
381+
* e.g. An inactive broker app might be invoked during OnUpgrade.
382+
* (It should be renamed, but that would mess up the dashboard)
383+
*/
384+
active_broker_package_name,
385+
386+
/**
387+
* Indicates the current broker package name processing the request.
388+
*/
389+
current_broker_package_name,
390+
368391
/**
369392
* Records if the request is a webcp authorize request.
370393
*/
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// All rights reserved.
3+
//
4+
// This code is licensed under the MIT License.
5+
//
6+
// Permission is hereby granted, free of charge, to any person obtaining a copy
7+
// of this software and associated documentation files(the "Software"), to deal
8+
// in the Software without restriction, including without limitation the rights
9+
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
10+
// copies of the Software, and to permit persons to whom the Software is
11+
// furnished to do so, subject to the following conditions :
12+
//
13+
// The above copyright notice and this permission notice shall be included in
14+
// all copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
// THE SOFTWARE.
23+
package com.microsoft.identity.common.java.opentelemetry;
24+
25+
import com.microsoft.identity.common.java.logging.Logger;
26+
27+
import io.opentelemetry.api.baggage.Baggage;
28+
import io.opentelemetry.context.Context;
29+
import io.opentelemetry.context.Scope;
30+
31+
/**
32+
* Utility class for working with OpenTelemetry Baggage objects.
33+
*/
34+
public class BaggageExtension {
35+
36+
private static final String TAG = BaggageExtension.class.getSimpleName();
37+
38+
/**
39+
* Default constructor.
40+
*/
41+
private BaggageExtension() {
42+
// Utility class, private constructor to prevent instantiation
43+
}
44+
45+
/**
46+
* Makes the provided Baggage current in the context, catching any exceptions silently.
47+
* This is useful in scenarios where Baggage propagation should not interrupt normal operation flow.
48+
*
49+
* @param baggage The Baggage to make current.
50+
* @return the resulting scope.
51+
*/
52+
public static Scope makeBaggageCurrent(final Baggage baggage) {
53+
try {
54+
if (baggage == null) {
55+
return SpanExtension.NoopScope.INSTANCE;
56+
}
57+
return baggage.storeInContext(Context.current()).makeCurrent();
58+
} catch (final Exception e) {
59+
Logger.error(TAG + ":makeBaggageCurrent", "Failed to make baggage current", e);
60+
return SpanExtension.NoopScope.INSTANCE;
61+
}
62+
}
63+
64+
/**
65+
* Returns the current Baggage from the context, or a NoopBaggage if an error occurs.
66+
*
67+
* @return the current Baggage.
68+
*/
69+
public static Baggage fromContext(final Context context) {
70+
try {
71+
return Baggage.fromContext(context);
72+
} catch (final Exception e) {
73+
Logger.error(TAG + ":fromContext", "Failed to get baggage from context", e);
74+
return new NoopBaggage();
75+
}
76+
}
77+
}

0 commit comments

Comments
 (0)