flux-relay: initialize log prefix to hostname when possible #4454
Conversation
src/cmd/builtin/relay.c
Outdated
| * for end users that may be uknowningly using flux-relay as part of | ||
| * the ssh connector. | ||
| */ | ||
| if (gethostname (hostname, MAXHOSTNAMELEN) < 0) |
There was a problem hiding this comment.
By my read of gethostname(1) the length parameter should be the size of the buffer not the maximum string length, so use MAXHOSTNAMELEN + 1, or maybe better, sizeof (hostname) here?
Also, not sure if this is important but the man page references HOST_NAME_MAX not MAXHOSTNAME. (Might be an alias but the former is apparently the POSIX one). Looking around our code base we use both :-(
Another thought is to use "unknown-host" or similar as the fallback if gethostname() fails (very unlikely). Could just statically initialize hostname[] to that value and avoid the duplicate call to log_init().
There was a problem hiding this comment.
Yeah, I did also see the HOST_NAME_MAX reference in the man page as well, but when I spot checked our codebase I saw use of MAXHOSTNAMELEN. Rather than fix it now, I chose MAXHOSTNAMELEN for consistency (though I guess I missed the uses of HOST_NAME_MAX)
grondo
force-pushed
the
flux-relay-hostname
branch
from
fa9df7f
to
f7142ac
Compare
Problem: Users almost always use flux-relay indirectly via the ssh connector by specifying a ssh:// URI or a high-level URI which resolves to an ssh URI. Therefore, the flux-relay log prefix on errors from this utility may not be useful to users. If possible, use the hostname as the log prefix. This will not only remove the possibly confusing flux-relay prefix, but will allow pinpointing the actual host on which the flux-relay is failing to, for example, open a local connector socket. In the unlikely event that gethostname(3) fails, then fall back to "uknown-host".
grondo
force-pushed
the
flux-relay-hostname
branch
from
f7142ac
to
bdce25e
Compare
|
@garlick, I addressed your comments. I switched to
Therefore it seems like existence of |
|
Thanks! Setting MWP. |
Codecov Report
@@ Coverage Diff @@
## master #4454 +/- ##
=======================================
Coverage 83.34% 83.34%
=======================================
Files 400 400
Lines 67258 67226 -32
=======================================
- Hits 56055 56032 -23
+ Misses 11203 11194 -9
|
When the ssh connector fails to connect to a remote uri, but the ssh was successfully able to execute
flux-relay, an error such as the following is displayedThe
flux-relayprefix probably isn't all that helpful for users, and may be confusing, sinceflux-relaywas only indirectly run as part of thesshconnector. Instead it may be more useful to use the hostname as the command prefix. This way, the user is made aware of the host on which the local connection failed e.g