Improve showShort to avoid parsing Uri #3590
Conversation
| @@ -10,6 +10,8 @@ import scala.collection.immutable.Seq | |||
| trait ServerRequest extends RequestMetadata { | |||
| def protocol: String | |||
| def connectionInfo: ConnectionInfo | |||
| /** Override in backend-specific implementation if you want a more efficient implementation and avoid uri parsing overhead */ | |||
| def uriStr: String = uri.copy(scheme = None, authority = None, fragmentSegment = None).toString | |||
There was a problem hiding this comment.
or maybe impls should simple override showShort? Since we are doing some customisations here (removing the scheme etc.), which are not necessarily general
There was a problem hiding this comment.
alternatively, we should rename uriStr to showUriShort
There was a problem hiding this comment.
Right, since uriStr will be used only rather only for showShort it seems like a good idea to skip it and just override that method 👍
kciesielski
changed the title
| @@ -24,6 +24,7 @@ case class NettyServerRequest(req: HttpRequest, attributes: AttributeMap = Attri | |||
| QueryParams.fromMultiMap(multiMap) | |||
| } | |||
| override lazy val method: Method = Method.unsafeApply(req.method().name()) | |||
| override lazy val showShort: String = s"$method ${req.uri()}" | |||
There was a problem hiding this comment.
we don't have to strip anything, or will this include the scheme & fragment here?
also, maybe worth adding some docs to showShort or ServerRequest that impls should override for performance?
There was a problem hiding this comment.
I ran some tests on our Netty servers (and Vert.X, which is based on Netty), and req.uri returned only path + query params, without fragment, user info, etc.
Good idea to add a comment about overriding showShort, I'll do it.
Fixes #3550, #3547, #3272
The
ServerRequest.showShortmethod is used inDefaultServerLogand possibly in user logs as well. It is quite expensive due to the overhead ofsttp.model.Uri.unsafeParse. This cost can be reduced by overridingshowShort, which prints the uri from the request as directly as possible.P.S. It's ok to just get the full
.uri()String in Netty/Vert.X, because this string contains only the path + query params.