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

Set default-constructed timestamp to current time in StartSpan #18

Merged
merged 4 commits into from
Nov 22, 2017

Conversation

isaachier
Copy link
Contributor

Fixes #14

@rnburn
Copy link
Contributor

rnburn commented Nov 22, 2017

If one of the system/steady timestamps is specified but not the other, shouldn't this try to convert the provided timestamp instead of using now?

Also, I think this same sort of logic is missing for finish as well.

@isaachier
Copy link
Contributor Author

Good point. Let me add both of those suggestions.

@codecov
Copy link

codecov bot commented Nov 22, 2017

Codecov Report

Merging #18 into master will increase coverage by 0.04%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   85.44%   85.49%   +0.04%     
==========================================
  Files          93       93              
  Lines        2219     2240      +21     
  Branches      202      205       +3     
==========================================
+ Hits         1896     1915      +19     
- Misses        252      254       +2     
  Partials       71       71
Impacted Files Coverage Δ
src/jaegertracing/Span.cpp 54.23% <100%> (+2.45%) ⬆️
src/jaegertracing/Tracer.cpp 91.11% <86.66%> (-0.89%) ⬇️
src/jaegertracing/Span.h 92.23% <0%> (+0.23%) ⬆️

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 60de3d3...95a7002. Read the comment docs.

Signed-off-by: Isaac Hier <[email protected]>
@isaachier isaachier merged commit aeefccc into jaegertracing:master Nov 22, 2017
@isaachier isaachier deleted the default-timestamp-now-fix branch November 22, 2017 11:27
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