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

[jaeger-client-java] Span class does not report multiple time on multiple invocations of finish() #299

Merged
merged 2 commits into from
Dec 1, 2017

Conversation

dray92
Copy link
Contributor

@dray92 dray92 commented Dec 1, 2017

Resolves #295

…iple invocations of finish()

Signed-off-by: Debosmit Ray <debo@uber.com>
@codecov
Copy link

codecov bot commented Dec 1, 2017

Codecov Report

Merging #299 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #299      +/-   ##
============================================
+ Coverage     82.96%   83.01%   +0.04%     
- Complexity      551      553       +2     
============================================
  Files            91       91              
  Lines          2160     2166       +6     
  Branches        244      245       +1     
============================================
+ Hits           1792     1798       +6     
  Misses          267      267              
  Partials        101      101
Impacted Files Coverage Δ Complexity Δ
...aeger-core/src/main/java/com/uber/jaeger/Span.java 81.48% <100%> (+1.08%) 40 <3> (+2) ⬆️

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 7250579...277151a. Read the comment docs.

@@ -162,6 +163,11 @@ public void finish(long finishMicros) {

private void finishWithDuration(long durationMicros) {
synchronized (this) {
if (finished) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to log a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep - I think it does. It seems important to know that the subsequent calls to finish(), will have no real effect. Making the change.

Signed-off-by: Debosmit Ray <debo@uber.com>
@yurishkuro yurishkuro merged commit bb45e70 into jaegertracing:master Dec 1, 2017
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.

3 participants