🐞 Bug - v2.3.2 - Invalid sampler class. Expected `BinarySampler` or `PercentageSampler`, got 0.000000

Rikj000 commented 1 year ago

Issue Description

Since updating to the new v2.3.2 release,
following error log gets spammed in the laravel.log file:

[2023-11-09 10:32:47] testing.ERROR: Invalid sampler class. Expected `BinarySampler` or `PercentageSampler`, got 0.000000 {

    "exception":"[object] (InvalidArgumentException(code: 0): Invalid sampler class. Expected `BinarySampler` or `PercentageSampler`, got 0.000000 at /vendor/vinelab/tracing-laravel/src/TracingDriverManager.php:83)

    [stacktrace]
    #0  /vendor/vinelab/tracing-laravel/src/TracingDriverManager.php(60): Vinelab\\Tracing\\TracingDriverManager->getZipkinSampler()
    #1  /vendor/laravel/framework/src/Illuminate/Support/Manager.php(106): Vinelab\\Tracing\\TracingDriverManager->createZipkinDriver()
    #2  /vendor/laravel/framework/src/Illuminate/Support/Manager.php(80): Illuminate\\Support\\Manager->createDriver()
    #3  /vendor/vinelab/tracing-laravel/src/TracingServiceProvider.php(77): Illuminate\\Support\\Manager->driver()
    #4  /vendor/laravel/framework/src/Illuminate/Container/Container.php(908): Vinelab\\Tracing\\TracingServiceProvider->Vinelab\\Tracing\\{closure}()
    #5  /vendor/laravel/framework/src/Illuminate/Container/Container.php(795): Illuminate\\Container\\Container->build()
    #6  /vendor/laravel/framework/src/Illuminate/Foundation/Application.php(955): Illuminate\\Container\\Container->resolve()
    #7  /vendor/laravel/framework/src/Illuminate/Container/Container.php(731): Illuminate\\Foundation\\Application->resolve()
    #8  /vendor/laravel/framework/src/Illuminate/Foundation/Application.php(940): Illuminate\\Container\\Container->make()
    #9  /vendor/laravel/framework/src/Illuminate/Container/Container.php(1454): Illuminate\\Foundation\\Application->make()
    #10 /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(236): Illuminate\\Container\\Container->offsetGet()
    #11 /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(207): Illuminate\\Support\\Facades\\Facade::resolveFacadeInstance()
    #12 /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(347): Illuminate\\Support\\Facades\\Facade::getFacadeRoot()
    #13 /vendor/vinelab/tracing-laravel/src/TracingServiceProvider.php(57): Illuminate\\Support\\Facades\\Facade::__callStatic()
    #14 /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36): Vinelab\\Tracing\\TracingServiceProvider->Vinelab\\Tracing\\{closure}()
    #15 /vendor/laravel/framework/src/Illuminate/Container/Util.php(41): Illuminate\\Container\\BoundMethod::Illuminate\\Container\\{closure}()
    #16 /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(81): Illuminate\\Container\\Util::unwrapIfClosure()
    #17 /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(37): Illuminate\\Container\\BoundMethod::callBoundMethod()
    #18 /vendor/laravel/framework/src/Illuminate/Container/Container.php(662): Illuminate\\Container\\BoundMethod::call()
    #19 /vendor/laravel/framework/src/Illuminate/Foundation/Application.php(1272): Illuminate\\Container\\Container->call()
    #20 /vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(220): Illuminate\\Foundation\\Application->terminate()
    #21 /artisan(49): Illuminate\\Foundation\\Console\\Kernel->terminate()
    #22 {main}"
}

Workaround

Manually add the new tracing.zipkin.sampler_class settings to config/tracing:

'sampler_class' => \Zipkin\Samplers\BinarySampler::class,
'percentage_sampler_rate' => env('ZIPKIN_PERCENTAGE_SAMPLER_RATE', 0.2),

PR #49 - Diff of config/tracing

Fix Suggestion

[ ] Populate the new tracing.zipkin.sampler_class settings in config/tracing with default values if not set.
[ ] Document config/tracing changes in the ChangeLog of new releases

PR #49 - Review

Linked Issues/PRs

summerKK commented 1 year ago

I made a mistake.😂😂 This is a break change., I ignored the need to update the configuration file. You can first synchronize the configuration file from vendor/vinelab/tracing-laravel/config/tracing.php

Rikj000 commented 1 year ago

I made a mistake.😂😂 This is a break change., I ignored the need to update the configuration file. You can first synchronize the configuration file from vendor/vinelab/tracing-laravel/config/tracing.php

I figured it out already, just finished editing my issue description's work-around :slightly_smiling_face:

summerKK commented 1 year ago

Will it be better to get it from env?

'sampler_class' => env('ZIPKIN_SAMPLER_CLASS', \Zipkin\Samplers\BinarySampler::class),

.env

ZIPKIN_SAMPLER_CLASS="Zipkin\\Samplers\\BinarySampler"

Rikj000 commented 1 year ago

Will it be better to get it from env?

I'd say exposing config settings to the .env, with default fallback values, so they're optional, is always desirable :smile:

summerKK commented 1 year ago

I added configurable parameters #51

Vinelab / tracing-laravel