[rake] shutdown tracer after execution #487
Conversation
There was a problem hiding this comment.
Thanks for this PR @kissrobber!
Looks really good.
I've added a few small comments about reducing code duplication and ensuring the traces are flushed even when exceptions are thrown.
Additionally a few tests verifying new behavior would future proof these changes.
Thanks!
| super | ||
| annotate_invoke!(span, args) | ||
| end | ||
| tracer.shutdown! if tracer.active_span.nil? && ::Rake.application.top_level_tasks.include?(name) |
There was a problem hiding this comment.
Would it be possible to extract this to a separate method? So that there will be less duplication with def execute.
| super | ||
| annotate_execute!(span, args) | ||
| end | ||
| tracer.shutdown! if tracer.active_span.nil? && ::Rake.application.top_level_tasks.include?(name) |
There was a problem hiding this comment.
Looks good!
It would be great to have some tests for this new behavior.
I think also this could be put in ensure block so that even when task throws an exception we will still be able to flush the trace with info about that exception.
| @@ -48,7 +50,7 @@ def annotate_execute!(span, args) | |||
| end | |||
|
|
|||
| def quantize_args(args) | |||
| quantize_options = Datadog.configuration[:rake][:quantize][:args] | |||
| quantize_options = configuration[:quantize][:args] | |||
|
@pawelchcki Hi! Why this test failed 😿 |
|
@kissrobber Sometimes that test flakes out (it tests some kind of throughput.) I'll re-run this for you. |
|
@pawelchcki @delner Hi, I changed this PR accoding to your comments. |
This sample doesn't seem to be working https://github.com/DataDog/dd-trace-rb/blob/master/docs/GettingStarted.md#rake
Rake task process exits before flushing.