Expose end_timestamp in Span#finish and Transaction#finish#1946
Expose end_timestamp in Span#finish and Transaction#finish#1946sl0thentr0py merged 1 commit intomasterfrom
end_timestamp in Span#finish and Transaction#finish#1946Conversation
Codecov ReportBase: 96.56% // Head: 98.35% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## neel/otel/expose-span-id #1946 +/- ##
============================================================
+ Coverage 96.56% 98.35% +1.78%
============================================================
Files 121 151 +30
Lines 7723 9380 +1657
============================================================
+ Hits 7458 9226 +1768
+ Misses 265 154 -111
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
st0012
left a comment
There was a problem hiding this comment.
I think this can base on master instead.
sentry-ruby/lib/sentry/span.rb
Outdated
| def finish | ||
| def finish(end_timestamp: nil) | ||
| # already finished | ||
| return if @timestamp |
There was a problem hiding this comment.
This isn't mentioned in the spec so I want to double check it: we only allow the first override but not the second time or later?
If that's true, can we add a test case for it?
There was a problem hiding this comment.
ah no, it should override either way, good catch!
| @@ -90,11 +90,11 @@ def initialize( | |||
|
|
|||
| # Finishes the span by adding a timestamp. | |||
| # @return [self] | |||
There was a problem hiding this comment.
We forgot to mention that it could return nil too (early return)
There was a problem hiding this comment.
I think it should never return nil, will also fix
f2d6d71 to
e863e25
Compare
st0012
left a comment
There was a problem hiding this comment.
I think we can merge this once the changelog entry is updated + the build is fixed by updating the base branch.
954d6a2 to
833392f
Compare
e863e25 to
85d1c67
Compare
No description provided.