search

twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 406 forks source link

finatra-http: User defined admin routes not available during server warmup #495

Closed jlawrienyt closed 5 years ago

jlawrienyt commented 5 years ago

finatra-http: User defined admin routes not available during server warmup

Expected behavior

When performing warmup action on an http server, I should be able to exercise user-defined admin routes. In particular, the HttpWarmup class implies this should work since it has logic for sending requests to the admin server when admin routes are detected, or by setting the forceRouteToAdminHttpMuxers parameter to true on the send method.

Actual behavior

Any requests to user-defined admin routes during warmup result in 404 Not Found responses. This appears to be because user-defined admin routes aren't added until the postWarmup phase. It seems straightforward enough to move that call into the postInjectorStartup but perhaps there are other ramifications for that?

Steps to reproduce the behavior

See the attached sample project. Run either sbt run or sbt testOnly example.NoAdminWarmupServerFailingFeatureTest to see it fail.

no-admin-warmup.zip

cacoco commented 5 years ago

@jlawrienyt it is not necessarily true that the HttpWarmup implies this (being able to call user-defined admin routes) should work because it has logic for sending requests to admin server, right? Since there are non-user defined admin routes (from the TwitterServer HTTP Admin interface) that you may be trying to hit.

In any case, yes, it is not as simple as moving the addition of routes from the postWarmup phase as it is done there for a reason but we can look to see what we can do to address this.

cacoco commented 5 years ago

@jlawrienyt this has been addressed in 0cd3ed691d4d86b5500dfddb858e0f25f9dc5058. Please take a look and hopefully it makes sense now.

You should be able to route to your user-defined admin endpoints with the HttpWarmup utility by simply doing warmup.send(request, admin = true)(). Note that trying to hit any thing not defined on the HttpRouter explicitly does not work as the HttpWarmup only routes to endpoints defined on a configured HttpRouter and does not expose any way to route to the TwitterServer HTTP Admin interface during warmup. That will take exposing the TwitterServer AdminHttpServer HttpMuxer which there is not appetite for at the moment.

Thanks!

jlawrienyt commented 5 years ago

Great, thanks @cacoco!