Removal of Elasticsearch.Net dependency from Elastic.Apm.ElasticSearch

[thompson-tomo]

Is your feature request related to a problem? Please describe.
Currently adding Elastic.Apm.NetCoreAll is adding a dependency of Elastic.Apm.ElasticSearch which is ok and expected however this package has a dependency on Elasticsearch.Net which is no longer being mantained. The Elasticsearch.Net package has some polyfill packages hence adds un-necessary dependencies.

Describe the solution you'd like
I would like the classes needed to be moved into Elastic.Apm.ElasticSearch and the dependency to Elasticsearch.Net removed. This would result in a consistent experience to all the other Instrumentations where there is no dependency to a particular library.

Describe alternatives you've considered
Moving classes into abstractions library, making the dependency dev only.

Additional context
N/a

[stevejgordon]

Hi, @thompson-tomo.

The Elastic.Apm.NetCoreAll is a convenience package which adds commonly used instrumentation. This includes the low-level transport components for Elasticsearch from Elasticsearch.Net. That package is still supported but not receiving major new features. As a netstandard package it depends on a few Microsoft packages that are commonly going to be available already in modern .NET targets. Is there a specific issue with the packages you are experiencing? Due to the nature of how the instrumentation works, this design is not something we're likely to change at the current time.

If you prefer, we do document an approach to avoid using this package and bring in the specific instrumentation you need.

[thompson-tomo]

Hi @stevejgordon

My main area of concern is about the polyfil packages being brought in from Elasticsearch.Net as that increases the likelyhood of CVE's etc. Has there been any thought to switching the dependency over to Elastic.Clients.Elasticsearch which i believe is the replacement? That package does not have any of the polyfill packages for the newer targets. At the same time given that my understanding in that you should be matching the Elasticsearch package with the version you have deployed, hence i am wondering do we actually need to have it in the convience package or should the convience libraries focus on instrumentation and not transport.

[stevejgordon]

In this case, the thing we are instrumenting is Elasticsearch.net itself, which includes the transport components for communicating with ES. Elastic.Clients.Elasticsearch is instrumented using modern OTel compatible APIs so we don't need to take a direct dependency on it for the instrumentation to be picked up by our bridge. If you're not using the NEST client or Elasticsearch.net then you can avoid those dependencies by manually choosing which instrumentation you want. If you opt for the ease of NetCoreAll then those are included, including their transitive dependencies. On our side, we can review if we can and should trim any such dependencies with conditional dependencies.

[thompson-tomo]

Thanks @stevejgordon would a pr be accepted including adding net 6 as a TFM so that the conditions could be implemented similar to what i will do with #2305

[stevejgordon]

@thompson-tomo, apologies. I missed this reply. You could raise an issue to discuss on the Elasticsearch.NET repo. I don't think there's anything to do on the APM side, so I am closing this.