ecs-logging-nodejs icon indicating copy to clipboard operation
ecs-logging-nodejs copied to clipboard

url.full coming from req instead of ecs

Open benvmatheson opened this issue 3 years ago • 1 comments

https://github.com/elastic/ecs-logging-nodejs/blob/26aaa6a8e75dd1cfe73023e81f676ef0a526b111/helpers/lib/http-formatters.js#L59

It looks like url attribute is being set with information from req instead of ecs like the rest of the attributes. This is causing full to reflect a different address than domain in our environment. Would this be more appropriate?

ecs.url.full = (socket && socket.encrypted ? 'https://' : 'http://') + ecs.headers.host + ecs.url.path

benvmatheson avatar Apr 04 '22 21:04 benvmatheson

@benvmatheson Thank you for this issue. My apologies for having take such a ridiculously long time to reply. I imagine you have moved on, but in the off chance that this is still relevant to you, I'd like to discuss it. I understand if you don't respond.

The formatHttpRequest() function is effectively (a) setting url.full using the hostname from req.headers.host and (b) setting url.domain from req.hostname. Looking at this code again, this code could be documented much better.

req.hostname is not actually a property of a core Node.js IncomingMessage request object (https://nodejs.org/api/http.html#class-httpincomingmessage). It is a property of an Express app request object: https://expressjs.com/en/4x/api.html#req.hostname Are you, or do you know if you were using Express in this example?

Express's req.hostname is from the Host header -- or from the X-Forwarded-Host header depending on configuration. Could this be what you are/were seeing? Are you able to show some data from an example?

(Aside: There is a similar issue -- https://github.com/elastic/ecs-logging-nodejs/issues/53 -- about the value used for setting the client.ip field from either core Node.js req.socket.remoteAddress or from Express's req.ip (https://expressjs.com/en/4x/api.html#req.ip), which considers an X-Forwarded-For header.)


Or I might be misunderstanding you. You suggest using ecs.headers.host. Is your logging statement explicitly setting headers.host to some value that differs from the req.headers.host?

trentm avatar Oct 30 '23 21:10 trentm