markmcdowell / NLog.Targets.ElasticSearch

NLog target for Elasticsearch
MIT License
176 stars 89 forks source link

feat: Add support for EnableApiVersioningHeader option for servers v8+ #156

Closed Rassi closed 12 months ago

Rassi commented 2 years ago

We have upgraded Elastic server to version 8.1.3 and experienced errors. After reading https://www.elastic.co/guide/en/elasticsearch/client/net-api/7.17/connecting-to-elasticsearch-v8.html#enabling-compatibility-mode and https://github.com/elastic/elasticsearch-net/issues/6154 we tried setting the ELASTIC_CLIENT_APIVERSIONING environment variable which fixed it.

One of our applications, though, connects to two different Elastic servers, one for logging and one for a search index. The search index server is on version 7.10 which does not yet support ApiVersioningHeader, and we cannot upgrade it yet. Thus we need NLog target to run with EnableApiVersioningHeader and the other client without.

This PR adds support for enabling ApiVersioningHeader just for the NLog target from config.

A limitation is, that it will not work when enabling it for servers running <7.13 (as per https://github.com/elastic/elasticsearch-net/issues/6154#issuecomment-1095182360) but I think that could be adequately explained in documentation.

Checklist

Rassi commented 2 years ago

It was the following low level error:

Invalid NEST response built from a successful (200) low level call on POST: /_bulk
# Invalid Bulk items:
  operation[0]: index returned 201 _index: x-2022.04.21 _type:  _id: vUmiS4ABsXhEMlfGFdaJ _version: 1 error: 
# Audit trail of this API call:
 - [1] PingSuccess: Node: https://x/ Took: 00:00:00.2103319
 - [2] HealthyResponse: Node: https://x/ Took: 00:00:00.3831254
# Request:
<Request stream not captured or already read to completion by serializer. Set DisableDirectStreaming() on ConnectionSettings to force it to be set on the response.>
# Response:
<Response stream not captured or already read to completion by serializer. Set DisableDirectStreaming() on ConnectionSettings to force it to be set on the response.>
snakefoot commented 12 months ago

@markmcdowell Think all 3 pull-requests #156 + #159 + #160 should be merged, and then release NLog.Targets.ElasticSearch ver. 7.8