search

opentracing / opentracing-php

OpenTracing API for PHP
Apache License 2.0
505 stars 56 forks source link

Add IsGlobalTracerRegistered to GlobalTracer #84

Closed MikeGoldsmith closed 5 years ago

MikeGoldsmith commented 5 years ago

Adds support for knowing if a global tracer has been registered via a global variable including tests.

Issue first reported here: opentracing/opentracing-python#109

This change has been accepted into C#, Java and Go and there are pending reviews for Python and NodeJS.

ellisv commented 5 years ago

I really do not want to see many cute features in this API that does not bring much value but on the other hand I am all for having similar API between all the languages.

jcchavezs commented 5 years ago

I honestly don’t see the value of this but I know it is included in some other languages so probably there is not much room for discussion here.

  1. des. 2018 kl. 16:44 skrev Eligijus Vitkauskas notifications@github.com:

I really do not want to see many cute features in this API that does not bring much value but on the other hand I am all for have similar API between all the languages.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/opentracing/opentracing-php/pull/84#issuecomment-446633963, or mute the thread https://github.com/notifications/unsubscribe-auth/AC7sAgor4N-8VmCZ0d_tP8BsrCi1E3Wkks5u4SRxgaJpZM4ZNVfa.

MikeGoldsmith commented 5 years ago

I understand the general discontent with a feature that seems may be rarely used, however as a library integrator this is especially useful for determining in the application has provided a tracer or not. This allows us to set a custom tracer if one is not set by the application.

jcchavezs commented 5 years ago

I see, thanks for the answer. I would discourage everyone to do that tho and I kind of regret we included the global tracer which is convenient but still a global thing.

As I said, we should probably merge it to keep consistency with other OT libraries.

Any feedback @beberlei?

  1. des. 2018 kl. 17:25 skrev Mike Goldsmith notifications@github.com:

I understand the general discontent with a feature that seems may be rarely used, however as a library integrator this is especially useful for determining in the application has provided a tracer or not. This allows us to set a custom tracer if one is not set by the application.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/opentracing/opentracing-php/pull/84#issuecomment-446649653, or mute the thread https://github.com/notifications/unsubscribe-auth/AC7sAjLgdek17Ic2KgoCOcSYvYSsCyQKks5u4S3sgaJpZM4ZNVfa.

MikeGoldsmith commented 5 years ago

Any further input or can we look to merge this change?

jcchavezs commented 5 years ago

I am going to merge this tomorrow if none comes with a great reason for not to.

Den ons. 30. jan. 2019, 18:45 skrev Mike Goldsmith <notifications@github.com :

Any further input or can we look to merge this change?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/opentracing/opentracing-php/pull/84#issuecomment-459039878, or mute the thread https://github.com/notifications/unsubscribe-auth/AC7sApwtw5Wp8xMANgv5cIUAZWcTfCrfks5vIdpKgaJpZM4ZNVfa .

MikeGoldsmith commented 5 years ago

Thanks @jcchavezs