Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Span#pretty_print for empty duration #1183

Merged
merged 1 commit into from Sep 23, 2020
Merged

Fix Span#pretty_print for empty duration #1183

merged 1 commit into from Sep 23, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Sep 21, 2020

This PR ensures Span#pretty_print is able to handle all valid values it prints.
The required changes were around the span's timing values.

This PR also adds a smoke test to ensure Span#pretty_print at least outputs a string without any errors.

The main error we stated seeing

     NoMethodError:
       undefined method `-' for nil:NilClass
     # ./lib/ddtrace/span.rb:392:in `duration`

was actually addressed in a previous PR by @ericmustin, but I remove a few of the safe-guard rescue statements from def duration when making changes to improve performance. I seek forgiveness with this PR 馃檱.

@marcotc marcotc added the core Involves Datadog core libraries label Sep 21, 2020
@marcotc marcotc requested a review from a team September 21, 2020 21:13
@marcotc marcotc self-assigned this Sep 21, 2020
@@ -346,8 +346,8 @@ def to_json(*args)

# Return a human readable version of the span
def pretty_print(q)
start_time = (start_time.to_f * 1e9).to_i rescue '-'
Copy link
Member Author

Choose a reason for hiding this comment

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

These were always wrong, as start_time in start_time = start_time.to_f references the newly declared variable, which it was always nil. It now references self.start_time, the instance value.

@marcotc marcotc merged commit d7f5a0e into master Sep 23, 2020
@marcotc marcotc deleted the span-pretty-print branch September 23, 2020 18:01
@marcotc marcotc added this to the 0.41.0 milestone Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants