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

Updating Netty Logging to only display errors and default logging to debug #181

Merged
merged 1 commit into from
Oct 11, 2022
Merged

Updating Netty Logging to only display errors and default logging to debug #181

merged 1 commit into from
Oct 11, 2022

Conversation

saratvemulapalli
Copy link
Member

Signed-off-by: Sarat Vemulapalli [email protected]

Description

Updating Netty logging to only print out errors.
Netty is using reflective access which is denied by default for Java > 8.
See: netty/netty#7817

As recommendation says, its just a warning but its not fatal problems for the application. For SDK we really dont care about Netty debug/trace logs, error should be good enough.

This PR also updates default logging to debug as trace is not really helping a human unless we really want to debug core problems.

Issues Resolved

Closes #180

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@saratvemulapalli saratvemulapalli requested a review from a team October 11, 2022 01:36
@codecov-commenter
Copy link

Codecov Report

Merging #181 (839d9a1) into main (8fbcc6e) will increase coverage by 0.93%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##               main     #181      +/-   ##
============================================
+ Coverage     66.00%   66.94%   +0.93%     
- Complexity       96      103       +7     
============================================
  Files            25       24       -1     
  Lines           456      475      +19     
  Branches         13       25      +12     
============================================
+ Hits            301      318      +17     
+ Misses          147      141       -6     
- Partials          8       16       +8     
Impacted Files Coverage Δ
...main/java/org/opensearch/sdk/TransportActions.java 81.81% <ø> (ø)
...main/java/org/opensearch/sdk/ExtensionsRunner.java 70.83% <50.00%> (+7.59%) ⬆️
src/main/java/org/opensearch/sdk/Extension.java 60.00% <100.00%> (+15.55%) ⬆️
...rch/sdk/handlers/ExtensionsInitRequestHandler.java 100.00% <100.00%> (ø)
...ch/sdk/sample/helloworld/rest/RestHelloAction.java 70.96% <0.00%> (-29.04%) ⬇️
...ain/java/org/opensearch/sdk/ExtensionSettings.java 88.88% <0.00%> (ø)
...java/org/opensearch/sdk/ExtensionRestResponse.java
...rch/sdk/handlers/ExtensionsRestRequestHandler.java 47.36% <0.00%> (+4.51%) ⬆️
...nsearch/sdk/handlers/OpensearchRequestHandler.java 66.66% <0.00%> (+6.66%) ⬆️
...c/main/java/org/opensearch/sdk/NettyTransport.java 100.00% <0.00%> (+7.14%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

I'd even be happy with INFO level logging, with instructions how to change it in the developer guide.

@saratvemulapalli
Copy link
Member Author

I'd even be happy with INFO level logging, with instructions how to change it in the developer guide.

+1 We'll start with debug and we could move over to info eventually overtime.

@saratvemulapalli saratvemulapalli merged commit 78bc6b3 into opensearch-project:main Oct 11, 2022
@saratvemulapalli saratvemulapalli deleted the netty-logging branch October 11, 2022 18:25
owaiskazi19 pushed a commit that referenced this pull request Oct 12, 2022
Signed-off-by: Sarat Vemulapalli <[email protected]>

Signed-off-by: Sarat Vemulapalli <[email protected]>
Signed-off-by: Owais Kazi <[email protected]>
dbwiddis pushed a commit that referenced this pull request Oct 12, 2022
Signed-off-by: Sarat Vemulapalli <[email protected]>

Signed-off-by: Sarat Vemulapalli <[email protected]>
Signed-off-by: Owais Kazi <[email protected]>
dbwiddis pushed a commit that referenced this pull request Oct 12, 2022
Signed-off-by: Sarat Vemulapalli <[email protected]>

Signed-off-by: Sarat Vemulapalli <[email protected]>
Signed-off-by: Owais Kazi <[email protected]>
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.

[BUG] Gradle run on the project has reflective access warnings
4 participants