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

Add aroundRequest directive which captures complete request / response processing duration #2216

Open
jrudolph opened this issue Sep 19, 2018 · 13 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:routing Issues related to the routing DSL t:server Issues related to the HTTP server

Comments

@jrudolph
Copy link
Contributor

Many people have attempted to write an aroundRequest directive that captures the full request / response cycle to precisely measure and log processing times.

It needs to take into regard:

  • sending request and response entity out
  • timeouts
  • exceptions / rejections

It would be good if we would provide this directive out-of-box so there's only one implementation to maintain.

Here are some third-party attempts:

https://medium.com/@krzysztof.ciesielski/i-extended-this-implementation-to-make-it-handle-timeouts-27fefd5b2e23
https://blog.softwaremill.com/measuring-response-time-in-akka-http-7b6312ec70cf

@jrudolph jrudolph added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:server Issues related to the HTTP server t:routing Issues related to the routing DSL labels Sep 19, 2018
@kstokoz
Copy link
Contributor

kstokoz commented Sep 19, 2018

Honestly i think this boils down to provide out of the box directives to allow people create proper access logs (see #2181) which would allow connecting request/response, properly handling rejections and timeouts, having callbacks for measuring response times for SLAs (last byte in-fist byte out, last byte in - last byte out).
Existing logRequestResult directive is not flexible enough.

In my current project we have similar to linked custom directives to write access logs, it would be quite convenient to have it out of the box. I think for any enterprise proper access logs is a must.

@patriknw
Copy link
Member

We can go ahead with this.

@kstokoz
Copy link
Contributor

kstokoz commented Nov 13, 2018

For example server idle timeout which is happening outside of routes are not even reported correctly right now.

For put/post requests when entity idle timeout is triggered original HttpIdleTimeoutException is lost in debug log and in handleExceptions receives akka.http.scaladsl.model.EntityStreamException: Entity stream truncation

@jrudolph jrudolph added 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding and removed 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Nov 22, 2018
@jrudolph jrudolph added this to the 10.1.6 milestone Nov 22, 2018
@kstokoz
Copy link
Contributor

kstokoz commented Nov 29, 2018

It would be also great to be able to log requests rejected by GracefulTerminatorStage during shutdown.

@jrudolph
Copy link
Contributor Author

jrudolph commented Dec 5, 2018

Thanks, @kstokoz, these are all good points.

We have two issues here:

  • On which level and how to gather this information
  • What the API should be to access this information

The original suggestion was to write a directive similar to the ones listed in the blog posts above. However, it will be quite hard to incorporate the features you suggested as a directive.

@kstokoz
Copy link
Contributor

kstokoz commented Dec 6, 2018

@jrudolph i would agree that with current implementation addressing all of cases can be problematic.
In current project we have something similar to the suggested around directive + combination of exception handlers + timeout handlers - i can cover most of the mentioned cases and manage to write access log entry. Only which is fully out of our control is requests rejected during termination.

It would be really awesome if akka would either provided out of the box directive for all of them or ability to register a callback to write access log, if not as a part of this issue though.

@jrudolph jrudolph modified the milestones: 10.1.6, 10.1.7 Dec 18, 2018
@raboof raboof modified the milestones: 10.1.7, 10.1.8 Jan 3, 2019
@raboof raboof modified the milestones: 10.1.8, 10.1.9 Mar 15, 2019
@raboof raboof removed 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding labels Jun 27, 2019
@raboof raboof removed this from the 10.1.9 milestone Jul 4, 2019
@jrudolph jrudolph added the 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted label Jul 10, 2019
@ssgao
Copy link

ssgao commented Oct 23, 2019

The examples provided here won't really capture the full response time. It only captures the time right before the response starts streaming. That's more of a first byte time. A full response time should include the time it takes for the client to receive it. So far I haven't found a way to capture that. Would be interested to know if anyone knows.

@mantisMolloy
Copy link

I have a PR which I think addresses this use case. It handles last byte time as well @ssgao .

Any and all feedback on approach taken welcomed.

#2814

@mantisMolloy
Copy link

@ssgao is correct. The approach taken in #2814 will capture the timing for first byte sent on non-chunked responses. For chunked responses it will capture the time to first byte of the last sent chunk.

@shestero
Copy link

shestero commented Nov 29, 2019

I'm sorry, but is possible to force a response to be chunked and just send at the end an empty chunk? In the most of HTTP use cases it's ok to send a space or zero byte at the end.
PS This is a dirty workaround but we users need a solution.

@piotrp
Copy link

piotrp commented May 14, 2020

For people that got here while looking for logging with response bytes, a third-party attempt:
https://github.com/derekwyatt/ahalog/blob/master/src/main/scala/org/derekwyatt/ahalog/AhaLogDirectives.scala

@mantisMolloy
Copy link

Hey @Crack, thanks for the new example. I tested it out. It looks like it works well for it's intended purpose of capturing entity size 👍

However, the onComplete function will be applied on first-byte out and not last-byte so will not capture full response time if anyone is looking for that.

@sdehors-ibm
Copy link

sdehors-ibm commented Oct 30, 2021

Hi, is #2814 the final answer to the problem? @kstokoz do you expect to merge it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:routing Issues related to the routing DSL t:server Issues related to the HTTP server
Projects
None yet
Development

No branches or pull requests

9 participants