elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

context.destination.service.resource should include the logical database name for SQL Databases #473

Open tobiasstadler opened 3 years ago

tobiasstadler commented 3 years ago

According to https://github.com/elastic/apm/blob/master/specs/agents/tracing-instrumentation-db.md#sql-databases context.destination.service.resource should by set to the database vendor/span subtype. In my opinion this is not granular enough. A single application might talk to different databases (of the same vendor), which may be on same or on different hosts/servers. In a distributed architecture different applications might talk to the same or different databases (of the same vendor), which may be on same or on different hosts/server. In both cases context.destination.service.resource will be the same (e.g. postgresql for PotsgreSQL). Thus Kibana cannot distinguish between the different databases in the service map/dependencies. It is also not possible to see the latency, throughput and error rate for each database individually, because these metrics are aggregated based on context.destination.service.resource.

I propose to add the logical database name to context.destination.service.resource for vendors that support the notion of a logical database name. The auto inference algorithm described in https://github.com/elastic/apm/blob/master/specs/agents/tracing-spans-destination.md#contextdestinationserviceresource does something similar.

E.g. for PostgreSQL context.destination.service.resource would be postgresql/mydb if one connects to a database called mydb (CREATE DATABASE mydb).

What do you think, does this make sense? How should the logical database name determined for the different database vendors? Should there be config option to switch between both behaviors?

eyalkoren commented 3 years ago

I completely agree it's time we address that!

We will probably have to do some research and apply the proper implementation per DB type/vendor. One thing to keep in mind is that we prefer to err to the insufficient-granularity direction. While too-low cardinality makes it not as useful as we would like, a too-high cardinality can make it entirely non-useful.

I also think having a configuration option for this is a good idea, as long as we have a good default for it and we properly document very clearly what would be the effect of it. What I am not sure about is how it would look like. Ideally, the user would be able to decide how to construct the service.resource with as much flexibility as possible, but maybe not as the first step.

tobiasstadler commented 3 years ago

Any opinions?

eyalkoren commented 3 years ago

@alex-fedotyev wdyt?

alex-fedotyev commented 3 years ago

Definitely, it is time to work on this, since we have new dependencies UI - this improvement will fit very nicely!

CC @AlexanderWert - as discussed, let's plan on which backend services we prioritize first.

AlexanderWert commented 3 years ago

cc @SylvainJuge

tobiasstadler commented 2 years ago

Any news?

AlexanderWert commented 2 years ago

Hi @tobiasstadler ,

we are working on the definition / technical design and task breakdown for this enhancement.

tobiasstadler commented 2 years ago

@AlexanderWert Thank you for the info!

ghyslaindetry commented 7 months ago

Hello,

I still see this issue open.

Do you have any news about this ?

Thank you !