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

Add public method isFinished() on JaegerSpan #634

Merged
merged 2 commits into from
Aug 3, 2019

Conversation

dougEfresh
Copy link
Contributor

@dougEfresh dougEfresh commented May 30, 2019

Which problem is this PR solving?

In complicated future/multi-threading environments, such as RX java, you need to pass trace context to the executing threads. In some cases we noticed that spans were activated long after the span has finished (like 30 minutes).
I've noticed hourly repeating tasks used stale trace context. By adding isFinished on the JaegerSpan, we can keep track (via metrics) of spans that are being activated but are themselves finished.

Short description of the changes

Adding public method isFinished to JaegerSpan.

@codecov
Copy link

codecov bot commented May 30, 2019

Codecov Report

Merging #634 into master will decrease coverage by 0.23%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #634      +/-   ##
============================================
- Coverage     89.62%   89.38%   -0.24%     
+ Complexity      564      563       -1     
============================================
  Files            69       69              
  Lines          2063     2073      +10     
  Branches        262      263       +1     
============================================
+ Hits           1849     1853       +4     
- Misses          134      138       +4     
- Partials         80       82       +2
Impacted Files Coverage Δ Complexity Δ
...ain/java/io/jaegertracing/internal/JaegerSpan.java 93.63% <100%> (+0.11%) 49 <1> (+1) ⬆️
...rtracing/internal/reporters/CompositeReporter.java 71.42% <0%> (-28.58%) 6% <0%> (-1%)
...gertracing/internal/reporters/LoggingReporter.java 81.81% <0%> (-9.1%) 4% <0%> (-1%)
...n/java/io/jaegertracing/internal/JaegerTracer.java 88.92% <0%> (-0.05%) 26% <0%> (ø)

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 43bfc75...d290ed2. Read the comment docs.

@@ -75,6 +75,10 @@ public long getStart() {
return startTimeMicroseconds;
}

public boolean isFinished() {
return finished;
Copy link
Member

Choose a reason for hiding this comment

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

needs synchronized (this), see finish() method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@yurishkuro
Copy link
Member

@dougEfresh are you planning to finish this PR?

@yurishkuro yurishkuro merged commit f1a7ca9 into jaegertracing:master Aug 3, 2019
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.

2 participants