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

Enable Custom log-level for Classes via log-level Filters #2620

Merged
merged 24 commits into from
Apr 1, 2025

Conversation

RubenCerna2079
Copy link
Contributor

@RubenCerna2079 RubenCerna2079 commented Mar 14, 2025

Why make this change?

This change solves tasks, #2584, #2585, #2586, #2587, #2588, #2569.
All of them are part of the bigger task of #2563 which adds filters for logging.

What is this change?

This change allows the users to add more specific logging filters to their config file, this means that log-levels meant for a specific class will take priority over the default log-level.
In the following example, even if the default is in debug, the logging for IQueryExecutor will be in information.

"Azure.DataApiBuilder.Core.Resolvers.IQueryExecutor": "information",
"default": "debug"

Specific Changes:

  • Changed the schema file so that it allows for different keywords that represent the classes, allowing each of them to have a different log-level. It also allows the user to add a general log-level through the use of the keyword default 
  • Updated DAB Validate so that it checks if the keywords used in the config file are valid classes that we use to give logging information to the users.
  • Change logic that deserializes the new properties in the config file and stores them inside of the TelemetryOptions class and it uses the saved log-levels to dictate if a specific logger uses a different log-level from the default. This change gets rid of the loggerFactory and changes it to a dictionary.

How was this tested?

  • Integration Tests
  • Unit Tests

Following tests were added:

  • Test that filters are deserialized correctly and their values are what we expect them to be.
  • Test that invalid filters throw an error.
  • Test that between two filters, the one that is more specific takes priority.

Sample Request(s)

image

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@RubenCerna2079 RubenCerna2079 force-pushed the dev/rubencerna/Add_LogLevel_Filters branch from 0b89823 to 24c3d2c Compare March 17, 2025 19:07
@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Aniruddh25
Aniruddh25 previously approved these changes Mar 28, 2025
Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM, suggest to change PR title to explain a bit more about what this change is enabling

e.g. "Enable custom log level for classes via log-level filters"

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079 RubenCerna2079 changed the title Log Level Filters Enable Custom Log Level for Classes via Log-Level Filters Mar 28, 2025
Copy link
Contributor

@sezal98 sezal98 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all the comments so far. Looks good to merge, just have a couple of pointers

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

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

Looks good so far, I am around half way through, will try to finish today.

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

sezal98
sezal98 previously approved these changes Apr 1, 2025
@RubenCerna2079
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM

@RubenCerna2079 RubenCerna2079 linked an issue Apr 1, 2025 that may be closed by this pull request
@RubenCerna2079 RubenCerna2079 changed the title Enable Custom Log Level for Classes via Log-Level Filters Enable Custom log-level for Classes via log-level Filters Apr 1, 2025
@RubenCerna2079 RubenCerna2079 merged commit f1f2019 into main Apr 1, 2025
11 checks passed
@RubenCerna2079 RubenCerna2079 deleted the dev/rubencerna/Add_LogLevel_Filters branch April 1, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants