Add support for Micrometer Observation

marcingrzejszczak commented 1 year ago

Description

I'm a co-maintainer of Spring Cloud Sleuth and Micrometer projects (together with @shakuzen and @jonatan-ivanov).

Micrometer Observation is coming as part of the Micrometer 1.10 release and Micrometer Tracing is a new project. The idea of Micrometer Observation is that you instrument code once but you get multiple benefits out of it - e.g. you can get tracing, metrics, logging or whatever you see fit).

Since this project already supports Micrometer I was curious if there's interest in adding Micrometer Observation support so that except for metrics, spans could be created and tracing context propagation could happen too.

If there's such interest we could provide a PR to add support for that.

elasticsearchmachine commented 1 year ago

Pinging @elastic/es-analytics-geo (Team:Analytics)

bclozel commented 1 year ago

Hi, I'm a Spring team member and I'm working with @marcingrzejszczak in the Micrometer team. I'd like to explain in more details the intended scope and the goals of this proposal.

First, this issue is not about global instrumentation of the Elasticsearch server - this has been declined already in #83780. We would like to contribute the instrumentation for the Low Level REST client using the Micrometer Observation API.

Rationale for this change

As far as I understand, the official Java client is currently not instrumented for metrics nor traces. Elasticsearch itself supports tracing headers and can surface very useful information with it. Other official clients, like the JavaScript variant ship observability support.

The OpenTelemetry agent currently instruments the deprecated "org.elasticsearch.client:elasticsearch-rest-client". Here's an example of data that should be collected for a single request with that client:

[network instrumentation]
transport: TCP/IP
socket peer address: example.org

[db information]
system: ELASTICSEARCH;
statement: "GET /index-name/_doc/some-document-id"
operation: "GET"

I think we can provide more information for metrics and traces for the official client, with very little maintenance effort with direct Micrometer Observation instrumentation.

Proposal

Instrumenting the official Java client would probably be challenging, as its codebase is partially generated and very much user facing. This would allow higher level metrics/traces metadata, but at this point I'm not convinced that it's the best ROI.

Instrumenting the low level client here would bring significant results already.

This change would require:

adding the io.micrometer:micrometer-observation JAR (12KB) as an optional or mandatory dependency to the low level client
instrument the RestClient directly, this means Micrometer needs to be a mandatory dependency, but the instrumentation is very efficient
OR instrument the RestClient through an interceptor/filter-like contract, keeping Micrometer completely optional

With this instrumentation, developers would get, depending on their setup:

no-ops and no performance loss if they don't care about observability
a "elasticsearch.client.request" timer metric for requests, with metadata about endpoints, the HTTP query and other config details
a "elasticsearch.client.request" trace with all th einfo above, plus a complete contextual name (like "GET /index-name/_doc/some-document-id"); thanks to the context propagation support, this trace would be linked to the other ones in the app (such as HTTP server requests).
we can also consider instrumenting the sniffer if we would like to get specific metrics for this.

This can be achieved with very few classes and would allow developers to use their own conventions for extracting or adding more metadata to metrics and traces. The additional setup would be minimal and most likely configured automatically by Frameworks like Spring Boot.

You can find an example of documented instrumentation in Spring Framework for the web layer.

cc @tlrx and @swallez to get your opinion on the matter. I'd be happy to discuss this more, or start drafting a proof of concept if this can help. Even if this feature can be contributed by the Micrometer team, I think it's important for library developers to define the semantics of the observations as they're the ones who know better what should be measured, and what users should see in their dashboards.

Thanks!

swallez commented 1 year ago

@bclozel thank you for your interest. First of all, I'd like to point out that elasticsearch-rest-client (low level client) is not deprecated. What is deprecated (and has been removed in Elasticsearch 8) is the high level client. The low level client is still used by the newer Java API client.

Next, we want to keep dependencies of the Java client minimal and avoid introducing a dependency to a specific instrumentation library, when both our own APM agent and the OpenTelemetry agent already provide transparent support for it.

So considering that this proposal doesn't align with a number of our objectives, I'm closing this issue. Thanks anyway for these suggestions.

bclozel commented 1 year ago

Indeed, I mixed the artifacts names. Thanks for considering this proposal!

patpatpat123 commented 1 year ago

Hello @swallez ,

Thank you for the explanation here. Thank you for sharing your opinion on the high vs low level client, as well as the design to keep the dependencies light.

With that said, we are using the full elastic stack with APM server, and using elastic-java to send requests for our document (create delete update etc)

We are not seeing any metrics, not seeing any traces coming out of elasticsearch-java.

For instance, this is what we would have expected to see: meteor-call-2

But actually, neither of the two is available in APM server.

Having metrics and tracing is a requirement in many companies for production readiness, and having elasticsearch-java support those would be great.

swallez commented 1 year ago

Thanks for the feedback @patpatpat123, I will check with the APM team. And – forgive me if this is a stupid question – have you installed the Java APM agent?

patpatpat123 commented 1 year ago

This is not a stupid question, and actually one we asked ourselves first. We are actually using OTLP for traces, and a custom solution for metrics. We verified they are working since we can see the traces in APM as well as the metrics, but just not those from elasticsearch-java. Let me come up with a minimal reproducible example and track this issue there. Thank you all for this thread.

elastic / elasticsearch