Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If client closes socket prematurely, mark transaction in a special way #154

Open
watson opened this issue Oct 9, 2019 · 4 comments
Open

Comments

@watson
Copy link
Contributor

watson commented Oct 9, 2019

Description of the issue

It came up in elastic/apm-agent-nodejs#1411 that the Node.js agent doesn't record the fact that a client closes the socket to the app being monitored. Most (if not all?) agents just happily record the transaction as successful - which, from the point of view of the service it also kind of is.

It would, therefore, be a nice feature if, when we're tracing an incoming HTTP request and the client closes the socket prematurely, that the agent can detect this and mark the transaction. Today the transaction just looks like it was successful even though the client never received the response.

Possible ways to mark the transaction:

  1. Change the transaction result from the regular result (e.g. HTTP 2xx) to for example aborted
  2. Add a "mark" to the transaction when the socket was closed similar to how the RUM agent marks "time to first byte" etc.
  3. Invent a new field, e.g. socket.status: "aborted"

I don't think I'm a big fan of option 3 as not all transactions have a socket associated with them. I really like option 2 as it allows us to record exactly when it happened. Option 1 is nice as well as it allows us to easily filter by this and display it in the UI - though maybe that's not actually a use case?

What we are voting on

@elastic/apm-agent-devs Should the agents who can implement something like this?

Please also share your thoughts in the comments on which way you think this could best be implemented or if you think it shouldn't be something that is recorded.

Vote

Agent Yes No Indifferent N/A Link to agent issue
.NET
Go
Java
Node.js
Python
Ruby
RUM
@graphaelli
Copy link
Member

Does response.finished cover this case or is that insufficient? afaik only the node agent uses it but that could be stale information.

@axw
Copy link
Member

axw commented Oct 10, 2019

I like the idea of adding a mark (option 2) as a minimal improvement, as it'll give people a simple visual means of understanding odd behaviour.

Option 1 sounds like it could be useful for identifying trends in unusual client behaviour, e.g. increasing rate of aborted transactions.

@watson
Copy link
Contributor Author

watson commented Oct 10, 2019

@graphaelli

Does response.finished cover this case or is that insufficient? afaik only the node agent uses it but that could be stale information.

Good question. Currently response.finished and response.headers_sent is only used for errors. If the node agent captures an error and the agent records information about the associated HTTP request (if one exists). If the error happened before the HTTP headers were sent to the client response.headers_sent will be set to false, if it happened before the request was finished response.finished is set to false. So the node agent currently never set these properties on transactions.

I'm personally still most in favor of option 2, but it could maybe be combined with using this field if it gave the user extra info.

@Qard
Copy link

Qard commented Oct 10, 2019

I'd go with a hybrid of 1 and 2. The searchability of 1 is nice, and the timeline display of 2 is nice too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants