Apm: Add `transaction.sampled` to Errors

Created on 14 Dec 2018 · 13Comments · Source: elastic/apm

If an error occurs during an unsampled transaction, transaction.id will still be added to the error. When querying for errors it is therefore not possible to easily find one that happened during a sampled transaction.

Use case
It should be possible to link the user from an error to the transaction where it occurred. The UI finds a random error sample for a particular error group, and shows a link to the corresponding transaction. If the user has a low sample rate, the link to the transaction sample will most often result in "Transaction could not be found".

Possible solutions

1) Add transaction.sampled to errors
This will make it trivial to filter for errors that happened during a sampled transaction.

2) Remove transaction.id from errors that happened during an unsampled transaction
This will also make it possible to filter for errors that happened during a sampled transaction.
However, it will no longer be possible to distinguish whether an error happened during an unsampled transaction or happened outside a transaction entirely.

3) Sample transactions if an error occurs
I don't know if it is technically possible to have part of a trace being sampled and the other part not.

Source

sqren

Most helpful comment

@elastic/apm-server @elastic/apm-agent-devs @roncohen anyone disagreeing on implementing option 1 for 6.6?

simitt on 17 Dec 2018

👍4

All 13 comments

It looks like between 1 and 2, 1 is preferable as it provides more info to work with and it is pretty much the same to implement.

Regarding 3- it depends on each specific agent/transaction-type, but I assume more often than not, most of the data you want to collect is still available at the time the agent decides to end the transaction. So even if you lose spans that ended (and were not sample) before the error occurred, it can still provide valuable info. I imagine this can be challenging when recording async spans, depending on the implementation, so we should assume we will get this wrong in some cases. Also, as @sqren says, we need to decide if it makes sense to do this without having the ability to retro-sample the entire trace when the error occurs in a backend transaction. If not, this means we will need to implement a response DT header to deliver the sampling decision upstream.

And there is also option 4- decide to increase sampling rate (or cancel sampling for a while) for transaction/traces with errors. This eliminates the partial/broken traces problem, but it also means we need to implement a two-way DT communication between agents, and as with 3- it will be difficult or impossible in some async scenarios.

Comparing 3 and 4- both have the risk of recording too much, so in both cases agents will have to implement guards. 4 is less error-prone, but 3 is better for capturing rare/sporadic errors.

Considering complexity, risk, effort and value, I would prioritise in this order: 1, then 4, then 3.

eyalkoren on 16 Dec 2018

I see value in keeping the relationship between an error and an unsampled transaction, especially if the sampling rate is low, as one could eg. query for all errors related to a certain transaction group. Therefore, I am not a fan of option 2.
Are there any plans to be able to also show unsampled transactions in the UI?

simitt on 17 Dec 2018

(2) is only possible as long as we have distinct events for unsampled transactions. If we start aggregating transactions agent-side, then there will be no transaction ID with which we can associate errors. When we come to that point we could include the transaction name and type, so we can identify the transaction group.

(3) doesn't seem feasible to me: we would either have to record and buffer spans until the transaction is ended, or just not report spans before the error occurs. I expect the former would be too expensive for agents, and the latter would mean we can no longer trust the spans to be complete (modulo occasional data loss).

My preference is for (1).

axw on 17 Dec 2018

Option 1 retains more info
Option 2 can be implemented without a change to the APM Server
Option 3 is going to be too complicated/costly to implement IMO
Option 4 is unrelated I think as it's not going to solve the issue for the specific transaction id

So in my opinion we have to choose either option 1, option 2, or a combination of 1 and 2 (i.e. add a sampled boolean, but don't report the transaction id).

Option 2 is easy and backwards compatible as it doesn't require any changes to the APM Server. It also works if we later decide not to send up unsampled transactions (sending up unsampled transactions doesn't scale that well I think).

But I'm fine with option 1 as well for now - or the option 1/2 combo.

watson on 17 Dec 2018

(2) is only possible as long as we have distinct events for unsampled transactions. If we start aggregating transactions agent-side, then there will be no transaction ID with which we can associate errors. When we come to that point we could include the transaction name and type, so we can identify the transaction group.

Just realised I got this a bit jumbled up. With (1) we can send "transaction.sampled" and rely on the ID for unsampled transactions, until we don't have them any more. Still I'd prefer to not lose information and go with (1).

axw on 17 Dec 2018

Option 4 is unrelated I think as it's not going to solve the issue for the specific transaction id

Right, this is indeed out of scope here. 1/2 is a must in any case and it will resolve the problem.

This was more an extension to 3- we need to strive to provide examples of error traces to any user that has high traffic app and must set sample rate to 0.1 or even 0.01. As APM one of our main goals is to provide visibility to errors.

eyalkoren on 17 Dec 2018

@simitt

Are there any plans to be able to also show unsampled transactions in the UI?

Unsampled transactions are used in aggregate calculations but are not displayed on their own. I don't think we have any plans to change this.

Going with option 1
It sounds like there is general consensus around option 1 (at least I don't see anyone arguing against this). I am also casting my vote on this option since it solves the problem from the UI POV in the most elegant way. Only possible drawback is that it requires changes in APM Server.

@elastic/apm-server Is it anywhere remotely likely that this change could make it into 6.6? FF is tomorrow (18th of Dec)
Even if we can't make it for 6.6 I still think we should go with option 1, and push it to 6.7.

@makwarth Any objections to pushing https://github.com/elastic/kibana/issues/21919 to 6.7 if necessary?

sqren on 17 Dec 2018

Option 1 means the agents would send up the additional information right? If so, this is not a big change and I can create a PR for it.

simitt on 17 Dec 2018

Created https://github.com/elastic/apm-server/issues/1660

simitt on 17 Dec 2018

@elastic/apm-server @elastic/apm-agent-devs @roncohen anyone disagreeing on implementing option 1 for 6.6?

simitt on 17 Dec 2018

👍4

@sqren @simitt I'm good with it