Skip to content

Conversation

@somalaya
Copy link
Contributor

@somalaya somalaya commented Sep 23, 2025

Using baggage to get parent span's (AcquireTokenInteractive) correlationId, calling app package name and setting them on the child spans created during webcp redirects in webview.

Why this needed?

  • We do not have correlationId set on webcp spans as of today. Although, we can check for the parent span and get correlationId from there, it is just an additional step. I want to avoid this.
  • We cannot directly set attributes to indicate that is webcp flow on the parent span (ATI) since the context passed to the activity is immutable. Recommended way is to create a child span and set attributes as needed on it.
  • We also cannot get parent span's attributes on child span directly using the context passed. Support for this was added by using baggage in this PR Using Baggage to propagate attributes from parent Span, Fixes AB#3262849 #2671. So, I am leveraging this.

Related broker PR to remove the redundant attributes : https://github.com/AzureAD/ad-accounts-for-android/pull/3230

Fixes AB#3385536

@github-actions
Copy link

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

@somalaya somalaya marked this pull request as ready for review September 23, 2025 20:33
@somalaya somalaya requested a review from a team as a code owner September 23, 2025 20:33
Copilot AI review requested due to automatic review settings September 23, 2025 20:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the telemetry implementation to use OpenTelemetry baggage for propagating parent span attributes to child spans created during WebView redirects. The change addresses the limitation where parent span attributes cannot be directly accessed from immutable contexts passed to activities.

  • Introduces baggage-based attribute propagation for correlation_id and calling_package_name from parent spans
  • Refactors span creation to use a centralized method that extracts attributes from baggage
  • Removes redundant WebCP-specific attribute names that are no longer needed

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
SpanName.java Adds new span names for WebCP enrollment and authorize URL redirects
AttributeName.java Removes deprecated WebCP-specific attribute names
AzureActiveDirectoryWebViewClient.java Implements baggage-based attribute propagation and refactors span creation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@somalaya somalaya changed the title Use baggage for spans Use baggage for spans, Fixes AB#3385536 Sep 26, 2025
@github-actions
Copy link

✅ Work item link check complete. Description contains link AB#3385536 to an Azure Boards work item.

@somalaya somalaya force-pushed the somalaya/useBaggageForSpan branch from 8a87e4a to fde86eb Compare September 29, 2025 05:56
@somalaya somalaya added the No-Changelog This Pull-Request has no associated changelog entry. label Sep 29, 2025
final boolean isWebCpFlightEnabled = CommonFlightsManager.INSTANCE.getFlightsProviderForTenant(homeTenantId, waitForFlightsTimeOut).isFlightEnabled(CommonFlight.ENABLE_WEB_CP_IN_WEBVIEW);
SpanExtension.current().setAttribute(AttributeName.web_cp_flight_get_time.name(), (System.currentTimeMillis() - webCpGetFlightStartTime));

SpanExtension.current().setAttribute(AttributeName.tenant_id.name(), homeTenantId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems you are adding tenant id here, which is good

@somalaya somalaya merged commit 6f0b749 into dev Sep 29, 2025
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No-Changelog This Pull-Request has no associated changelog entry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants