Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Log SenderExceptions the first time they occur in a row #729

Merged
merged 1 commit into from
Jul 31, 2020
Merged

Log SenderExceptions the first time they occur in a row #729

merged 1 commit into from
Jul 31, 2020

Conversation

pschichtel
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

  • From looking at the code, currently the SenderExceptions occurring in the Sender actually don't get logged at all. The RemoteReporter merely took the number of lost spans, but otherwise discards the exception. This change tracks if the previous command of its kind failed (using set of command classes that saw an error previously) and logs out 2 messages:
  1. A message on the first failed execution
  2. A message on the first successful message after it previously failed

Alternative approach

Alternatively I think we could check if the SenderException was caused by a SocketException and either:

  • Log only the first SocketException
  • Log only the first SocketException of its kind (comparing messages).

@codecov
Copy link

codecov bot commented Jul 30, 2020

Codecov Report

Merging #729 into master will increase coverage by 0.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #729      +/-   ##
============================================
+ Coverage     88.76%   89.02%   +0.26%     
- Complexity      596      598       +2     
============================================
  Files            73       73              
  Lines          2242     2251       +9     
  Branches        289      291       +2     
============================================
+ Hits           1990     2004      +14     
+ Misses          159      155       -4     
+ Partials         93       92       -1     
Impacted Files Coverage Δ Complexity Δ
...egertracing/internal/reporters/RemoteReporter.java 90.62% <100.00%> (+0.96%) 7.00 <0.00> (ø)
...gertracing/internal/reporters/LoggingReporter.java 90.90% <0.00%> (+9.09%) 5.00% <0.00%> (+1.00%)
...rtracing/internal/reporters/CompositeReporter.java 100.00% <0.00%> (+28.57%) 7.00% <0.00%> (+1.00%)

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 9b0ed16...a3c28e7. Read the comment docs.

this should close #728

Signed-off-by: Phillip Schichtel <phillip@schich.tel>
Copy link
Collaborator

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Successfully merging this pull request may close these issues.

None yet

2 participants