spandex-project / spandex

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

Fix broken `Tracer.configure(disabled?: true)` #71

Closed sekiyama58 closed 6 years ago

sekiyama58 commented 6 years ago

Currently, even when Tracer.configure(disabled?: true) is called, the spandex is not disabled.

There is another issue that calls of trace_start and span_start after dynamic configuration of tracer using Tracer.configure(service: :app, adapter: ..., disabled?: true) will cause exception like:

    ** (EXIT) an exception was raised:
        ** (ArgumentError) Opt Validation Error: service - is required, adapter - is required
            (optimal) lib/optimal.ex:44: Optimal.validate!/2
            (mikoto) lib/mikoto/tracer.ex:2: Mikoto.Tracer.config/2
            (mikoto) lib/mikoto/tracer.ex:2: Mikoto.Tracer.start_span/2

This is because the option is not Application.put_env when disabled?: true.

This patch fixes the issue.

sourcelevel-bot[bot] commented 6 years ago

Hello, @sekiyama58! 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

Pull Request Test Coverage Report for Build 312


Totals Coverage Status
Change from base Build 311: 1.8%
Covered Lines: 189
Relevant Lines: 226

💛 - Coveralls
zachdaniel commented 6 years ago

Thanks for your contribution! Additionally, thanks for including a test to validate your change. Much appreciated.