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

Add verboseSocketExceptions flag #1577

Merged
merged 2 commits into from Feb 11, 2019

Conversation

trustin
Copy link
Member

@trustin trustin commented Feb 8, 2019

Motivation:

Since we fixed the behavior of verboseExceptions flag with respect to
Exceptions.isExpected(), we received the report of increased
exceptions logged. However, most of them were harmless socket-related
ones such as connection reset by peer, where we do not have control
over, because network disconnection can always happen.

Modifications:

  • Added a new flag verboseSocketException which defaults to false.
  • Modified Exceptions.isExpected() to use verboseSocketException
    instead of verboseException.

Result:

  • Less noisy log messages

Motivation:

Since we fixed the behavior of `verboseExceptions` flag with respect to
`Exceptions.isExpected()`, we received the report of increased
exceptions logged. However, most of them were harmless socket-related
ones such as `connection reset by peer`, where we do not have control
over, because network disconnection can always happen.

Modifications:

- Added a new flag `verboseSocketException` which defaults to `false`.
- Modified `Exceptions.isExpected()` to use `verboseSocketException`
  instead of `verboseException`.

Result:

- Less noisy log messages
@trustin trustin removed the request for review from hyangtack February 11, 2019 10:56
@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #1577 into master will increase coverage by 0.08%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1577      +/-   ##
============================================
+ Coverage     72.77%   72.85%   +0.08%     
- Complexity     7495     7518      +23     
============================================
  Files           695      695              
  Lines         30171    30190      +19     
  Branches       3683     3687       +4     
============================================
+ Hits          21956    21994      +38     
+ Misses         6310     6298      -12     
+ Partials       1905     1898       -7
Impacted Files Coverage Δ Complexity Δ
...a/com/linecorp/armeria/common/util/Exceptions.java 39.13% <0%> (+9.56%) 25 <11> (+11) ⬆️
...c/main/java/com/linecorp/armeria/common/Flags.java 65.26% <100%> (+0.36%) 47 <1> (+1) ⬆️
...rp/armeria/server/tomcat/ManagedTomcatService.java 50.84% <0%> (-11.87%) 6% <0%> (-1%)
...armeria/server/tomcat/Tomcat90ProtocolHandler.java 48.14% <0%> (-7.41%) 11% <0%> (-2%)
...spring/web/reactive/ArmeriaHttpHandlerAdapter.java 75% <0%> (-3.95%) 5% <0%> (ø)
...lient/circuitbreaker/CircuitBreakerHttpClient.java 69.69% <0%> (-3.04%) 10% <0%> (-1%)
.../linecorp/armeria/server/tomcat/TomcatService.java 63.82% <0%> (+0.09%) 24% <0%> (ø) ⬇️
.../armeria/internal/grpc/ArmeriaMessageDeframer.java 69.33% <0%> (+0.94%) 44% <0%> (+1%) ⬆️
...com/linecorp/armeria/server/HttpServerHandler.java 78.7% <0%> (+1.52%) 79% <0%> (+5%) ⬆️
...inecorp/armeria/client/grpc/ArmeriaClientCall.java 81.67% <0%> (+1.52%) 37% <0%> (+1%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70f781f...11c90fa. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #1577 into master will increase coverage by 0.08%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1577      +/-   ##
============================================
+ Coverage     72.77%   72.85%   +0.08%     
- Complexity     7495     7518      +23     
============================================
  Files           695      695              
  Lines         30171    30190      +19     
  Branches       3683     3687       +4     
============================================
+ Hits          21956    21994      +38     
+ Misses         6310     6298      -12     
+ Partials       1905     1898       -7
Impacted Files Coverage Δ Complexity Δ
...a/com/linecorp/armeria/common/util/Exceptions.java 39.13% <0%> (+9.56%) 25 <11> (+11) ⬆️
...c/main/java/com/linecorp/armeria/common/Flags.java 65.26% <100%> (+0.36%) 47 <1> (+1) ⬆️
...rp/armeria/server/tomcat/ManagedTomcatService.java 50.84% <0%> (-11.87%) 6% <0%> (-1%)
...armeria/server/tomcat/Tomcat90ProtocolHandler.java 48.14% <0%> (-7.41%) 11% <0%> (-2%)
...spring/web/reactive/ArmeriaHttpHandlerAdapter.java 75% <0%> (-3.95%) 5% <0%> (ø)
...lient/circuitbreaker/CircuitBreakerHttpClient.java 69.69% <0%> (-3.04%) 10% <0%> (-1%)
.../linecorp/armeria/server/tomcat/TomcatService.java 63.82% <0%> (+0.09%) 24% <0%> (ø) ⬇️
.../armeria/internal/grpc/ArmeriaMessageDeframer.java 69.33% <0%> (+0.94%) 44% <0%> (+1%) ⬆️
...com/linecorp/armeria/server/HttpServerHandler.java 78.7% <0%> (+1.52%) 79% <0%> (+5%) ⬆️
...inecorp/armeria/client/grpc/ArmeriaClientCall.java 81.67% <0%> (+1.52%) 37% <0%> (+1%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70f781f...11c90fa. Read the comment docs.

@trustin trustin merged commit 5d9a70b into line:master Feb 11, 2019
@trustin trustin deleted the verbose_socket_exceptions branch February 11, 2019 13:53
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this pull request Sep 19, 2020
Motivation:

Since we fixed the behavior of `verboseExceptions` flag with respect to
`Exceptions.isExpected()`, we received the report of increased
exceptions logged. However, most of them were harmless socket-related
ones such as `connection reset by peer`, where we do not have control
over, because network disconnection can always happen.

Modifications:

- Added a new flag `verboseSocketException` which defaults to `false`.
- Modified `Exceptions.isExpected()` to use `verboseSocketException`
  instead of `verboseException`.

Result:

- Less noisy log messages
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.

None yet

2 participants