Include actual URI in "Connection closed error=invalid URI" error output

[deusxanima]

What problem are you trying to solve?

Users who configure invalid URIs will be met with 400 errors and Linkerd logs stating Connection closed error=invalid URI. Currently the Linkerd logs do not bubble up the actual URI in question which makes it difficult to troubleshoot in large environments.

How should the problem be solved?

Pass the actual URI to the error message at Debug level for Connection closed error=invalid URI errors.

Any alternatives you've considered?

n/a

How would users interact with this feature?

Linkerd proxy logs.

Would you like to work on this feature?

No response

[tkuik]

Thank you for opening the enhancement.

[tkuik]

One other thing that would be nice as part of this issue is to update the documentation to the URI validations performed.

[dwilliams782]

I've just found this scattered in our logs too, both suggestions (log and docs) would be useful for us.

[hawkw]

The simplest way to include the invalid value in the proxy logs will involve changes to the HTTP libraries the proxy uses. I've opened hyperium/hyper#3043 and hyperium/http#571 to track the relevant upstream changes, which I'm working on currently.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[dwilliams782]

Commenting to keep open..

[hawkw]

Just to add some context to this, this is currently blocked on potential upstream changes in the library that Linkerd uses for parsing HTTP requests. We can't actually add a URI to the log message using information that's known to the proxy, because this error is being returned by the HTTP parser when it attempts to parse a request from the wire --- the proxy doesn't know what the URI is yet.

Displaying the invalid URI string requires modifying the HTTP parser to return the invalid value that failed to be parsed as part of that error. We've started a discussion around making this change, but would need to get upstream maintainers to agree on it: hyperium/http#571

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.