opensearch-project / opensearch-php

Official PHP Client for OpenSearch
Other
105 stars 58 forks source link

[FEATURE] Out of the box New Relic PHP agent support #216

Open normalerweise opened 3 months ago

normalerweise commented 3 months ago

Is your feature request related to a problem?

Using OpenSearch client in combination with New Relic (e.g. when migrating from Elasticsearch) does not work immediately. I.e. OpenSearch client does not get instrumented, so no span for the OpenSearch requests gets created. So the time the OpenSearch request takes gets attributed to time spend in PHP in New Relic. This distorts and complicates run-time analysis.

The reason is using an old and relatively unmaintained dependency ezimuel/ringphp instead of up to date version of Guzzle Library. See here))) and here

What solution would you like?

Upgrade to work with Guzzle HTTP client library 7

What alternatives have you considered?

Dealing with custom instrumentation

Do you have any additional context?

none

shyim commented 3 months ago

I would not use Guzzle when we change the HTTP client. https://www.php-fig.org/psr/psr-7/ and https://www.php-fig.org/psr/psr-18/ are better I think.

Just using Guzzle because a random provider has support for it feels wrong, and in my opinion, they developed it wrong. They should instrument curl and not a specific Http Client library implementation which other providers like Blackfire, Tideways, Datadog do.

dblock commented 3 months ago

~Thanks for reporting this @normalerweise.~

~1. Is there a trivial fix like a version upgrade of a dependency?~ ~2. In other languages I see clients support multiple HTTP clients/adapters. In java we have 3, in Ruby we use Faraday that is flexible, etc. @shyim do you think it makes sense to add support for Guzzle next to whatever we have here now?~ ~3. @normalerweise Would you like to try and contribute any of the above? We could really use some help.~

I started upgrading Guzzle in https://github.com/dblock/opensearch-php/tree/upgrade-guzzle. There are ... a lot of changes.

dblock commented 2 months ago

Catch All Triage - 1, 2, 3