Add method to get URLEncoded version of HttpRequest

[amorozow42]

Surprisingly, it is not possible to get string representation of HTTP request. I need to do simple thing - log my requests, but have no idea how to achieve this. WebClient built in logging is not working for me (3.8.4).

[MeYoGui]

Just throwing an idea here but what I actually do for that purpose is to extract useful information from the HttpRequest itself (you can get the HttpRequest from the Webclient.request() or any get() / post()... methods).

So I can use these information (http method, query params, headers, url, etc.) in the logs. Hope it can help.

[amorozow42]

@MeYoGui This is wrong way for several reasons:

1. You should write boilerplate code each time you use WebClient.
2. We create inconsistency between actual request string and your string from log.
3. It runs counter to OOP paradigm because we violate encapsulation principle.

I want to know what exactly will be sent over the network. So we need something like this:

HttpRequest::toQueryString()

[MeYoGui]

Hey @romero

1. I don't have any boilerplate that I need to write each time I use the WebClient. The WebClient needs to be created only once per event loop, so the dependency injection is taking that into account. I'm using an abstraction layer that is wrapping the WebClient so it enables me to use a decorator pattern to hook myself before or after the request is made. One of these decorators is here for logging purposes.
2. I'm using the getters that are available on the HttpRequest, and it represents the actual request that is sent. Why would there be some inconsistencies here ?
3. IMO you can violate encapsulation if you access data that is not meant to be exposed. Here I'm using public getters to access data that was "by design" exposed.

Your approach with the HttpRequest::toQueryString() seems nice but one concern would be that what is actually returned may not be what everyone expect from it.

Hence why I suggested that you can create your own "toString()" based on what is inside the HttpRequest itself.

[amorozow42]

@MeYoGui

I don't want to create decorators or another abstractions of any kind. All I want (and other developers too) is to implement my domain logic. When we talk "Query String" everybody knows what it means - encoded url of request. So here is no problems to add this method I think.

[MeYoGui]

When we talk "Query String" everybody knows what it means - encoded url of request.

I may disagree a bit here. For me a "Query String" is not only the "encoded URL" : what do you do with the headers, the HTTP method or the eventual request body that you may want to display in the logs (using MDC for some of them maybe) ?

All I'm saying is that it can be hard for an "unopinionated" framework like Vertx to choose what needs to be displayed in the logs. They choose to give us the flexibility to implement the logs we want ourselves.

But I agree that maybe a "default" way could be implemented, I'll let the Vertx maintainers see if it seems like a good idea :)

[amorozow42]

@MeYoGui

I may disagree a bit here. For me a "Query String" is not only the "encoded URL" : what do you do with the headers, the HTTP method or the eventual request body that you may want to display in the logs (using MDC for some of them maybe)?

I agree, sometimes we need additional info about request. For this purpose there should be method "HttpRequest::toString".

[pmlopes]

@romero @MeYoGui, could you tell an example string representation? I'm not sure if I follow the full request here. Are you looking for something like?

http://example.com/a%25b?x%20yz

This IMHO will not inform me what kind of HTTP verb is/was used, or if there's a body.

[amorozow42]

@pmlopes, yes, we are looking for this string. The job could be done by method HttpRequest::toQueryString(). Method HttpRequest::toString() could return all info (HTTP method, body, etc.).

[MeYoGui]

I still have the feeling that VertX is already providing everything needed with the "accessors" on the HttpRequest, and it's on us to display it the way we want (some of us may don't want the body to be displayed by default for security reasons for example).

What we've been using is this format though :

"[REQUEST] {HttpMethod} {Path}"

[REQUEST] GET /api/users
[REQUEST] POST /api/whatever/foo

We omitted the query parameters as we put them in MDC (Mapped Diagnostic Context) to make it clearer on logs.

[amorozow42]

@MeYoGui

1. Using accessors is procedure way, not OOP.
2. I don't want to repeat the same logic each time I use WebClient.
3. It is very important to be sure that what we see in log is what we send by network.
4. Maybe we just need fix logging for WebClient.

[vietj]

I think we could have this static util method in vertx-web-client if you can make a PR.

[amorozow42]

@vietj Oh no! So many weird words in one place) Can we achieve this by using WebClientOptions::setLogActivity?

[stiyyagura]

I for sure think, there should be a method available to do the encoding of the URI. Otherwise, we needed to do the same for every HTTP method by overriding the operations.