Add Audience as span attribute for AAD/MSA/Common, Fixes #2970888

[tanmaymanolkar1]

https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2970888

[github-actions]

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

Click here to Learn more.

[mohitc1]

let's have one more: TENANTED, when there's target tenant in authority

So,

COMMON,
MSA,
ORGANIZATIONS,
TENANTED

[shahzaibj]

Wouldn't tenanted be AAD?

[shahzaibj]

I think the goal here is to identify which service we are talking to and be able to easily identify that in telemetry. With your suggestion @mohitc1 I think it is better engineering data but needs the reader to know how to interpret it i.e. reader needs to know that tenanted and organizations represent AAD

[mohitc1]

We can choose another name like SINGLE_ORGANIZATION (similar to Audience class names). This for completeness.
We don't need to comeback and update this later if we realize need of /organizations vs / in future.

[shahzaibj]

I think there's actually 3 different types of things can track and emit:

1. Service we are talking to. Possible values:

• MSA
• AAD

2. Audience type in authority. Possible values:

• COMMON
• MSA
• ORGANIZATIONS
• TENANTED

3. The actual authority (OII). Possible values:

• login.micosoftonline.com/common
• login.microsoftonine.us/common
• login.microsoftonine.cn/common
• login.microsoftonline.com/consumers
• login.microsoftonline.com/organizations
• login.microsoftonline.com/<MSA_MEGA_TENANT_ID>
• login.micorsoftonline.com<SOME_AAD_TENANT_ID>

My 2 cents, I think there is value in no.3 because this will help us know actually which endpoint we are talking to and could be super helpful for debugging issues. It can even help track if we are talking to a different cloud and which one specifically and also help track tenant id for guest scenarios.

Number 1 is more for tracking quickly which service we are talking to (AAD vs MSA) for things such as easily filtering out MSA no_account_found errors as expected.

The number 2 is somewhere between 1 and 3 i.e. it is somewhat product information but also somewhat debugging information. I'm not too sure if we need all 3 as some may be overkill. I'm thinking we should do no. 3 and then either one of no. 1 and no. 2.

Thoughts? @mohitc1 @tanmaymanolkar1

[mohitc1]

Agree we need all to capture the request target completely. But we need not capture all these separately.
We can optimize on what we capture.
Option 1:
If we capture just two fields
Cloud: login.microsoftonline.com, login.microsoftonine.cn, login.microsoftonine.us (or any shortened form to save data)
AND
(Audience type without OII: COMMON, ORGANIZATIONS, SINGLE_ORGANIZATION, MSA, MSA_MEGA_TENANT
OR audience type with OII: COMMON, ORGANIZATIONS, MSA, MSA_MEGA_TENANT, )

Option2:
We simply capture authority URL e.g. login.microsoftline.com/.. everything else can be derived from this. This would be OII.

[shahzaibj]

My worry with Option 2 is just not knowing how much it will add to complexity in the telemetry backend. We can write helper functions to derive these things but need to make sure that those helper functions get integrated into any materialized views etc and not sure if there would be any perf impact. @tanmaymanolkar1 You may want to do a quick analysis about how easy this will be on the backend.

[mohitc1]

I think we should move this and the method to broker itself. Telemetry is for broker only.

At this layer it's extra stuff present in AzureActiveDirectoryAudience

[shahzaibj]

+1

[tanmaymanolkar1]

Closing this PR as I will move the changes to broker