Guzzle 6 Integration bug about dd-trace-php HOT 9 CLOSED

Hi @inverse

Do you have a plan in mind how you would like to support multiple versions of libraries for example? I know that guzzle exposes a constant with a version but the way I saw you init is based on class only and not considering other factors.

Proposals are welcomed of course, but this is the TL;DR version of what we were thinking to do:

• Each integration, e.g. guzzle would have a file DDTrace\Integrations\Guzzle\GuzzleIntegration.php which exposes a single method ::load() to be called to load the integration (1)
• That method tries to guess the version based of any info it can have access to, e.g. a version file vs a class that exists or not
• Then it delegates the load() to the version-specific ...\Vx\GuzzleIntegration.php file (2)/(3).

DDTrace/
	Integrations/
		…
		Guzzle/
			V5/
				GuzzleIntegration.php (2)
			V6/
				GuzzleIntegration.php (3)
			GuzzleConfiguration.php  // Optional
			GuzzleIntegration.php (1)
		…
		IntegrationLoader.php
	Configuration.php

Also for testing - I've not got any experience with this but have seen tools out there such as https://packagist.org/packages/g1a/composer-test-scenarios

The link that you provided seems very promising, and in fact we were looking for something similar to what we use in python tracer and this library seems to be the answer. Very good suggestion I believe.

from dd-trace-php.

Feel free to take it over :) I didn't get that far as I'm not able to work on this during work time and home life is another balancing act :) interested to see the implementation though :)

from dd-trace-php.

Hi @inverse, thanks for reporting this issue. We currently support (and tests) 5.x but we were aware of 6.x version that had to be addresses! Let me get back to you soon with an ETA. In the meantime if you have free cycles we would more than welcome a PR 😄

from dd-trace-php.

Totally would love to contribute to this! but not sure about free cycles right now.

Do you have a plan in mind how you would like to support multiple versions of libraries for example? I know that guzzle exposes a constant with a version but the way I saw you init is based on class only and not considering other factors.

Also for testing - I've not got any experience with this but have seen tools out there such as https://packagist.org/packages/g1a/composer-test-scenarios

Cheers for the feedback!

from dd-trace-php.

Hey @inverse, we may have some spare cycles to adopt the library you suggested in our tests. But I don't want to step over if you already did some work. Happy to merge/integrate in that case.
I played a few minutes with it and seems to be working very well.

from dd-trace-php.

@inverse just an heads-up, if you are curious you can follow this PR #203 that introduces the test scenarios

from dd-trace-php.

thanks @labbati 👍

from dd-trace-php.

@inverse I am closing this issue as Guzzle 6 support has been added, finally 😄
Feel free to re-open it if you experience any other problem.
Thanks again for your advice on using https://packagist.org/packages/g1a/composer-test-scenarios it definitely helped moving forward in supporting (and testing) multiple versions of the same integration.

from dd-trace-php.

@labbati that was quick! Will let the team involved in this know so that they can continue our integration.

from dd-trace-php.