search

spandex-project / spandex

A platform agnostic tracing library
MIT License
335 stars 53 forks source link

Stop overriding completion_time when finishing span #60

Closed christopheonce closed 6 years ago

christopheonce commented 6 years ago

This should solve #55 and allow creating span based on logs.

sourcelevel-bot[bot] commented 6 years ago

Hello, @christopheonce! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+2.8%) to 73.399% when pulling fa5c7815de2c9df7b2f1b07e9e7ba838fe7a0f20 on christopheonce:fix-completion-time-override into b8779e1b157cc38f5df9fcfaf9f773b3537a1e4a on zachdaniel:master.

zachdaniel commented 6 years ago

I also have a branch up with a fix :) I'm happy to go with yours though. Make sure to account for the change in finish_trace also.

zachdaniel commented 6 years ago

Everything looks good to me, except that calling ensure_completion_time/2 from the first clause of ensure_completion_time/2 seems unnecessary.

zachdaniel commented 6 years ago

@christopheonce Ready to merge?

christopheonce commented 6 years ago

@zachdaniel I am done with this Pull Request

zachdaniel commented 6 years ago

@christopheonce Thanks for your contribution! This is now available (along with the recent adapter move) as version 2.0.0 in hex.