Fix compatibility with Zipkin 3

[adiachenko]

#34 introduced PHP 8 support which requires Zipkin PHP v3.

However, due to slight difference in contracts the package can't instantiate the Http reporter:

PHP Fatal error:  Uncaught TypeError: Zipkin\Reporters\Http::__construct(): Argument #1 ($options) must be of type array, null given, called in /opt/project/vendor/vinelab/tracing-laravel/src/Drivers/Zipkin/ZipkinTracer.php on line 325 and defined in /opt/project/vendor/openzipkin/zipkin/src/Zipkin/Reporters/Http.php:52
Stack trace:

It seems like the suggested order of parameters was changed back in v2, but it didn't become an issue due to additional layer of compatibility checks which are now removed. This will now work for both v2 and v3.

[jcchavezs]

I am curious about the error related to psr/log. Could you share a bit more details?

…

On Tue, Oct 26, 2021, 22:28 Alexander Diachenko ***@***.***> wrote: #34 <#34> introduced PHP 8 support which requires Zipkin PHP v3. However, due to slight difference in contracts the package can't instantiate the Http reporter: PHP Fatal error: Uncaught TypeError: Zipkin\Reporters\Http::__construct(): Argument #1 ($options) must be of type array, null given, called in /opt/project/vendor/vinelab/tracing-laravel/src/Drivers/Zipkin/ZipkinTracer.php on line 325 and defined in /opt/project/vendor/openzipkin/zipkin/src/Zipkin/Reporters/Http.php:52 Stack trace: It seems like the suggested order of parameters was changed back in v2, but it didn't become an issue due to additional layer of compatibility checks <https://github.com/openzipkin/zipkin-php/blob/e33b25412a6ad69b41d28f582f7166620783acab/src/Zipkin/Reporters/Http.php#L85> which are now removed. This will now work for both v2 and v3. ------------------------------ On a side note, I am not sure whether the new version constraint from #34 <#34> works quite as expected: "openzipkin/zipkin": "~2.0.2|3.0.x-dev", There is no issue for PHP 7.4 and Zipkin v2 here. However, when I tried to install the package in newer Laravel 8 project with PHP 8 I encountered a bunch of composer errors due to conflict between dependencies (seems to be related to psr/log package). Can be fixed by installing Zipkin v3 explicitly before this package. Please keep this mind for now, I'll create separate ticket once I confirm the issue. ------------------------------ You can view, comment on, or merge this pull request online at: #37 Commit Summary - Fix compatibility with Zipkin 3 <29ad28a> File Changes (1 file <https://github.com/Vinelab/tracing-laravel/pull/37/files>) - *M* src/Drivers/Zipkin/ZipkinTracer.php <https://github.com/Vinelab/tracing-laravel/pull/37/files#diff-c166a974b3334a217af992b6c7ba5d8b18565c4a498955146103b5e83535a7d0> (2) Patch Links: - https://github.com/Vinelab/tracing-laravel/pull/37.patch - https://github.com/Vinelab/tracing-laravel/pull/37.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#37>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXOYATJ4SAAQAJK37JMMA3UI4MRDANCNFSM5GYWBSDA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[adiachenko]

@jcchavezs So, I originally thought the issue was an unintended consequence of lax version constraint ("openzipkin/zipkin": "~2.0.2|3.0.x-dev") on our end, but it seems to be something else which is why I updated original description.

Starting from v8.62.0 Laravel constrained the version of psr/log in its own composer.json to ^1.0 || ^2.0. If you create Laravel project now, your composer.lock will have your psr/log dependency locked to 2.0. Actually, if you follow the linked PR, they even suggest enforcing psr/log ^3.0 for Laravel 9.

This seems to conflict with Zipkin PHP that expects psr/log ^1.0.

For now, this can be solved by downgrading psr/log dependency manually in your project, but it's certainly a bit confusing.

I wonder if it would be possible to allow different versions of psr/log in Zipkin PHP.

[jcchavezs]

I am fine with upgrading psr/log in zipkin-php. Would you put up a PR with such change?

…

On Wed, Oct 27, 2021, 00:25 Alexander Diachenko ***@***.***> wrote: @jcchavezs <https://github.com/jcchavezs> So, I originally thought the issue was an unintended consequence with lax version constraint ("openzipkin/zipkin": "~2.0.2|3.0.x-dev") on our end, but it seems to be something else which is why I updated original description. Starting from v8.62.0 Laravel constrained <laravel/framework#38851> the version of psr/log in its own composer.json to ^1.0 || ^2.0. If you create Laravel project now, your composer.lock will have your psr/log dependency locked to 2.0. This seems to contradict Zipkin PHP that expects ^1.0. For now, this can be solved by downgrading psr/log dependency manually in your project, but it's certainly a bit confusing. I wonder if it would be possible to allow "psr/log": "^1.0 || ^2.0" in Zipkin PHP as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#37 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXOYAW4SLQ4TEC5DXUMPYDUI42G5ANCNFSM5GYWBSDA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[adiachenko]

I am fine with upgrading psr/log in zipkin-php. Would you put up a PR with such change?
…

Sure, I'll submit one later today.