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

url.full coming from req instead of ecs #118

Open
benvmatheson opened this issue Apr 4, 2022 · 1 comment
Open

url.full coming from req instead of ecs #118

benvmatheson opened this issue Apr 4, 2022 · 1 comment
Labels
agent-nodejs Make available for APM Agents project planning. community

Comments

@benvmatheson
Copy link

benvmatheson commented Apr 4, 2022

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

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

@github-actions github-actions bot added agent-nodejs Make available for APM Agents project planning. community triage labels Apr 4, 2022
@trentm
Copy link
Member

trentm commented Oct 30, 2023

@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 -- #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 trentm removed the triage label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. community
Projects
None yet
Development

No branches or pull requests

2 participants