Skip to content

fix:Change Log level from WARN to DEBUG for number values in Audience Conditions for revisionNumber #459

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

Closed
wants to merge 3 commits into from

Conversation

The-inside-man
Copy link
Contributor

Summary

  • Changing log level from WARN to DEBUG for Audience Conditions when UnknownMatchTypeException or UnexpectedValueTypeException are caught.

Several customers have expressed concerns with logs being blown up with warnings due to usage of numbers for revisionNumber and other instances that require the use of a number value in their application where the SDK expects a string value. To avoid warnings in production environments, the log-level has been changed to DEBUG only now for those instances.

Test plan

  • New Unit Test
  • FSC

Issues

  • "OASIS-8129"

…assed for revisionNumber in Audience Conditions
@coveralls
Copy link

coveralls commented Jan 21, 2022

Pull Request Test Coverage Report for Build 1933

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 90.735%

Totals Coverage Status
Change from base Build 1926: 0.0%
Covered Lines: 4867
Relevant Lines: 5364

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1933

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 90.735%

Totals Coverage Status
Change from base Build 1926: 0.0%
Covered Lines: 4867
Relevant Lines: 5364

💛 - Coveralls

@The-inside-man The-inside-man marked this pull request as ready for review January 24, 2022 13:28
@The-inside-man The-inside-man requested a review from a team as a code owner January 24, 2022 13:28
@The-inside-man
Copy link
Contributor Author

@jaeopt Could you please take a look at this. All I have done is change the Log level for when the exception is caught to avoid blowing up the logs for the unknown and incorrect types.

Copy link

@dustin-sier dustin-sier left a comment

Choose a reason for hiding this comment

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

LGTM. I checked C# (its debug) and looking through Go to see if we're doing similar.

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

I think we should stick to WARN level for UnknownMatchTypeException and UnexpectedValueTypeException. It must be an error on their settings.
The one with potentially exploding log messages is when they do not provide attribute for audience (instead of wrong type). It's interesting that the code already logs missing attribute as a debug level with correct messages (line 97).

@The-inside-man The-inside-man marked this pull request as draft January 24, 2022 17:33
@jaeopt jaeopt closed this Feb 11, 2022
@jaeopt
Copy link
Contributor

jaeopt commented Feb 11, 2022

We'll keep the current WARNING level for these issues. It's confirmed that we currently generate a DEBUG level log for missing attribute as expected. The only issue was with Sematic Version, which has been fixed with #463.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants