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

Separate log field for request port #4396

Closed
ghost opened this issue Oct 25, 2021 · 4 comments · Fixed by #4403
Closed

Separate log field for request port #4396

ghost opened this issue Oct 25, 2021 · 4 comments · Fixed by #4403
Labels
feature ⚙️ New feature or request

Comments

@ghost
Copy link

ghost commented Oct 25, 2021

request>remote_addr of http.log.access logs are in format of <ipa>:<port>. I think the port should be a separate field.

@francislavoie francislavoie added the feature ⚙️ New feature or request label Oct 25, 2021
@francislavoie
Copy link
Member

Related to #4148

@mholt
Copy link
Member

mholt commented Oct 26, 2021

Hey @ledlamp, thanks for participating and submitting feedback!

I think the port should be a separate field.

Why do you think that? Why is this needed? In my experience, the remote address is usually combined.

@ghost
Copy link
Author

ghost commented Oct 26, 2021 via email

@francislavoie
Copy link
Member

Having then separate would avoid having to do some extra processing to split them up in situations where only the IP address is desired (i.e. most of the time because the internal port of the connection is really not that useful to know about).

This was a pain-point with the format-encoder module because Common Log specifically wants just the remote IP, not including the port.

I kinda feel we should rip off the band-aid and split them up at the same time as we remove common_log (i.e. 2.5.0 I guess) into two fields in the log JSON.

If both the remote IP and port are desired, they can be concatenated trivially. But if only one of the two is desired, then it's a lot more complicated, especially if IPv6 comes into play, etc.

francislavoie added a commit that referenced this issue Nov 1, 2021
Closes #4396, related to #4148

This is a breaking change, but we're already planning on removing  the`common_log` field as well at the same time, so might as well all do it together.

In general, the remote IP is the useful part of the remote address. The port is rarely useful, because it only identifies ephemeral information about the connection from the client. But we were logging both in one field, so certain tooling that would want to only get the remote IP would need to split it up. Splitting is non-trivial, because of IPv4, IPv6, shenanigans. So it's best if we split it up-front before logging. If the log consumer actually cares about the remote port, it can re-assemble it.
francislavoie added a commit that referenced this issue Nov 25, 2021
Closes #4396, related to #4148

This is a breaking change, but we're already planning on removing  the`common_log` field as well at the same time, so might as well all do it together.

In general, the remote IP is the useful part of the remote address. The port is rarely useful, because it only identifies ephemeral information about the connection from the client. But we were logging both in one field, so certain tooling that would want to only get the remote IP would need to split it up. Splitting is non-trivial, because of IPv4, IPv6, shenanigans. So it's best if we split it up-front before logging. If the log consumer actually cares about the remote port, it can re-assemble it.
francislavoie added a commit that referenced this issue Nov 29, 2021
Closes #4396, related to #4148

This is a breaking change, but we're already planning on removing  the`common_log` field as well at the same time, so might as well all do it together.

In general, the remote IP is the useful part of the remote address. The port is rarely useful, because it only identifies ephemeral information about the connection from the client. But we were logging both in one field, so certain tooling that would want to only get the remote IP would need to split it up. Splitting is non-trivial, because of IPv4, IPv6, shenanigans. So it's best if we split it up-front before logging. If the log consumer actually cares about the remote port, it can re-assemble it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants