Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identity: Improve diagnosability #4744

Merged
merged 10 commits into from
Jul 5, 2023

Conversation

antkmsft
Copy link
Member

@antkmsft antkmsft commented Jun 29, 2023

Look at the bug #4723: the customer did a great job debugging the problem with SDK, finding the actual server response, and including it in the bug.

This change is to make diagnosability simpler: turn on the verbose level of logging and if there is token JSON parse error, see more details. It is easier to log a bug, or if a customer contacts us with just the exception, we can tell them how to easily get more details for the bug.

It is good to have this, because access token JSON is basically the only external input for Identity, and also it could be that we can't reproduce the same response in our environment, but when we have these details, we could much better see what kind of response is there, construct the similar input and make a fix.

If this functionality was in place when the customer has logged that bug, here's what they'd seen:

Azure::Core::Credentials::AuthenticationException with the description

"GetToken(): Token JSON object: can't find or parse 'expires_in' property.
See Azure::Core::Diagnostics::Logger for details (https://aka.ms/azsdk/cpp/identity/troubleshooting)."

would've been thrown.

The log would say:

[2023-07-01T08:28:30.5602314Z] DEBUG : Identity: TokenCredentialImpl::ParseToken(): Please report an 
issue with the following details:
Token JSON: Access token property ('access_token'): string.length=2062, relative expiration property 
('expires_in'): "84671", absolute expiration property (''): undefined, other properties: 'client_id': 
string.length=36, 'expires_on': "1687364401", 'ext_expires_in': "86399", 'not_before': "1687277701", 
'resource': string.length=28, 'token_type': string.length=6.

@antkmsft antkmsft requested a review from RickWinter June 29, 2023 19:24
@antkmsft antkmsft requested a review from RickWinter July 1, 2023 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants