feat: log local TCP address

[maxmoehl]

Summary

When troubleshooting issues related to keepalive it is often useful to be able to correlate multiple requests made over the same connection. A first step was to introduce the information on whether the request was made on a re-used connection or not which was done with a7dbf84. This commit adds the local address of the TCP connection which adds the capability to distinguish between multiple, parallel keep-alive connections by looking at the client port of gorouter for the backend connection.

• Read the Contributing document.

Backward Compatibility

Breaking Change? Yes

[maxmoehl]

The big question is: how do we proceed with such log changes which could be considered breaking? I don't think it's a healthy practice to keep adding flags for each and every individual log field we add 🤔

[ameowlia]

The big question is: how do we proceed with such log changes which could be considered breaking? I don't think it's a healthy practice to keep adding flags for each and every individual log field we add 🤔

I agree, I hate having to add so many config properties. But sadly, I do think that this is a breaking change. I think of all the logs that operators look at, I think they DO look at access logs. And they are not a nice json format, they are an unwieldy string mess that you would have to write a custom parser for.

Here are the options I see...

1. Add the new field with no config property - risk breaking operators and facing their wrath (we should not do this)
2. Add the new field to the end of the log line - still risk breaking operators and facing their wrath, but maybe slightly less risk (we should not do this either)
3. Put it behind yet another config property - this fills up our code with config properties
4. Develop a more advanced config property that allows users to "opt in" to certain fields by name 1 by 1. So that way we can add new fields, but they can always opt in / out of certain ones. - Fixes this problem for the future. But this is more work. And this is just a config property for everything! Will operators even opt in to any of them!?
5. (From @geofffranks) add an additional access-log file that's json-based for people to migrate to, and we deprecate the old one eventually. When it is json based we can add new properties without it being a breaking change. - Personally I love this idea, but it is definitely more work.

So, in summary, I would say:

• sadly this is a breaking change (whomp)
• if you could chose one of options 3-5 that would be great

[maxmoehl]

sadly this is a breaking change (whomp)

That's what I was expecting 😄

Develop a more advanced config property that allows users to "opt in" to certain fields by name 1 by 1. So that way we can add new fields, but they can always opt in / out of certain ones. - Fixes this problem for the future. But this is more work. And this is just a config property for everything! Will operators even opt in to any of them!?

That's probably what I will be going for in this PR to prevent accumulating further config flags. Just a flat list, and if anyone needs special properties they can be added and exposed via this mechanism without breaking anyone.

add an additional access-log file that's json-based for people to migrate to, and we deprecate the old one eventually. When it is json based we can add new properties without it being a breaking change. - Personally I love this idea, but it is definitely more work.

This is my preferred long-term solution, but it will require quite some effort and it's not clear when we will be able to invest.

[maxmoehl]

I've added a new property which allows logging of additional fields in a generic way. The release PR will follow once we agree on this PR.

[ameowlia]

Can you add an test for...

• when extra_fields does not contain local_address, then the local address is not tested.
• when extra_fields contains "meow" it ... does not error? logs? idk does something reasonable?

[ameowlia]

❓ Can you add a PR to routing-release so I can deploy and test everything at once?