Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the x-elastic-client-meta header #1089

Merged
merged 6 commits into from
Dec 16, 2020
Merged

Add the x-elastic-client-meta header #1089

merged 6 commits into from
Dec 16, 2020

Conversation

ezimuel
Copy link
Contributor

@ezimuel ezimuel commented Dec 15, 2020

This PR adds a x-elastic-client-meta header in the HTTP request to send version information about the PHP language, the client library, the cURL version, the usage of async, etc.
An example of x-elastic-client-meta is as follows:

x-elastic-client-meta: es=7.10.0,php=7.4.11,a=0,c=7.68.0

where es is Elasticsearch version, phpis PHP version, a=0 means the HTTP call was sync (1=async), c is the cURL version.

This header can be disabled using the function ClientBuilder::setElasticMetaHeader(false), as follows:

$client = ClientBuilder::create()
    ->setElasticMetaHeader(false)  // default is true
    ->build();

@ezimuel ezimuel added this to the 7.11.0 milestone Dec 15, 2020
@ezimuel ezimuel assigned philkra and unassigned philkra Dec 15, 2020
Copy link

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo this looks awesome! 🤩 One tiny comment for you :)

private function getElasticMetaHeader(array $connectionParams): string
{
$clientMeta = sprintf(
"es=%s,php=%s,a=%d",
Copy link

@sethmlarson sethmlarson Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add t=... for PHP transport version is the same as client version I assume?

should be: x-elastic-client-meta: es=7.10.0,php=7.4.11,t=7.10.0,a=0,c=7.68.0 in the end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't insert the transport t parameter because I read this in the specification: "If there is no transport library separate from the client library then the key must be omitted".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the t parameter since the spec was outdated.

Copy link
Contributor

@philkra philkra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ezimuel ezimuel merged commit c8084ec into elastic:7.x Dec 16, 2020
@ezimuel ezimuel deleted the add/x-elastic-client-meta branch December 16, 2020 17:23
ezimuel added a commit that referenced this pull request Dec 16, 2020
* Replaced array_walk with array_map

In Connection::getURI, array_walk was used together with passing the
values as references, to change the original array.
Passing values as references is error-prone and discouraged for quite
some time.
Also, when using in conjunction with PHP 8.0, it will fail.

array_map can do the same thing as the original array_walk
implementation, but without the downsides of having side effects and
having to pass values as references.

* Add the x-elastic-client-meta header (#1089)

* Added the x-elastic-client-meta header

* Removed @ExpectedException usage in PHPUnit

* Removed prestissimo plugin for composer in github action

* Added .phpunit.result.cache in .gitignore

* Add the t transport parameter in telemetry client header

* Fixed semver format for PHP version in client telemetry header

Co-authored-by: Enrico Zimuel <e.zimuel@gmail.com>
ezimuel added a commit that referenced this pull request Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants