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

[BUG] Noisy Error Logs in azure-identity #19278

Closed
FishyJoel opened this issue Feb 17, 2021 · 4 comments
Closed

[BUG] Noisy Error Logs in azure-identity #19278

FishyJoel opened this issue Feb 17, 2021 · 4 comments
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Milestone

Comments

@FishyJoel
Copy link

Describe the bug
Unnecessary and noisy error logs from com.azure.identity.implementation.MSIToken class

To Reproduce
Steps to reproduce the behavior:
This issue occurs whenever a date in the format "02/17/2021 17:03:10 +00:00" is passed into the parseDateToEpochSeconds() method.

Code Snippet
MSIToken.java lines 60-78:

        try {
            return Long.parseLong(dateTime);
        } catch (NumberFormatException e) {
            logger.error(e.getMessage());
        }

        try {
            return Instant.from(dtf.parse(dateTime)).getEpochSecond();
        } catch (DateTimeParseException e) {
            logger.error(e.getMessage());
        }

        try {
            return Instant.from(dtfWindows.parse(dateTime)).getEpochSecond();
        } catch (DateTimeParseException e) {
            logger.error(e.getMessage());
        }

        throw logger.logExceptionAsError(new IllegalArgumentException("Unable to parse date time " + dateTime));

This makes three attempts to parse the input string, and each can generate an unnecessary error log. The NumberFormatException and DateTimeParseExceptions should instead be logged at a lower level (INFO or DEBUG).

Expected behavior
No errors are logged if the date is successfully parsed.

Setup (please complete the following information):

  • OS: CentOS
  • IDE : IntelliJ
  • Version of the Library used: This issue is present in Azure-Identity 1.0.4 and 1.2.3.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • [X ] Bug Description Added
  • [X ] Repro Steps Added
  • [X ] Setup information Added
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 17, 2021
@alzimmermsft alzimmermsft added Azure.Identity Client This issue points to a problem in the data-plane of the library. labels Feb 17, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 17, 2021
@alzimmermsft
Copy link
Member

FYI @g2vinay, @jianghaolu

@alzimmermsft alzimmermsft changed the title [BUG] [BUG] Noisy Error Logs in azure-identity Feb 17, 2021
@joshfree joshfree added bug This issue requires a change to an existing behavior in the product in order to be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 23, 2021
@joshfree joshfree added this to the [2021] June milestone Feb 23, 2021
@joshfree joshfree added feature-request This issue requires a new behavior in the product in order be resolved. and removed bug This issue requires a change to an existing behavior in the product in order to be resolved. labels Feb 23, 2021
@joshfree
Copy link
Member

Thanks for filing this detailed issue @FishyJoel. We'll include this improvement in an upcoming minor release of azure-identity.

Would you be interested in sending a PR with the change?

@joshfree joshfree added the help wanted This issue is tracking work for which community contributions would be welcomed and appreciated label Feb 23, 2021
@g2vinay
Copy link
Member

g2vinay commented Feb 24, 2021

@FishyJoel Thank you for reporting this issue.
I will look into this and potentially relax the log level and release the update in our March stable release.

@g2vinay
Copy link
Member

g2vinay commented May 6, 2021

@FishyJoel
The log levels have been relaxed and switched to verbose.
You can try out the change in latest release:

<dependency>
  <groupId>com.azure</groupId>
  <artifactId>azure-identity</artifactId>
  <version>1.2.5</version>
</dependency>

Let us know if any further issues are seen.

@g2vinay g2vinay closed this as completed May 6, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Jun 2, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Jun 2, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Jun 2, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Jun 2, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Jun 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Projects
None yet
Development

No branches or pull requests

4 participants