Don't shell out to get the hostname

[ardichoke]

I've encountered issues where the JVM cannot allocate memory in order to shell out, causing the puppet report integration to fail when trying to run hostname. Considering the socket library is already being loaded, switching the shell out to use socket.gethostname should make this code more reliable with less overhead.

Review checklist (to be filled by reviewers)

• Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
• PR title must be written as a CHANGELOG entry (see why)
• Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
• PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
• PR should not have do-not-merge/ label attached.
• If Applicable, issue must have kind/ and severity/ labels attached at least.

[therve]

Unfortunately Socket.gethostname doesn't return a fqdn, so we can't make that change. Some people suggest Socket.gethostbyname(Socket.gethostname).first but it doesn't seem to be perfect either.

It seems that you should be able to pass the hostname explicitly to avoid from that call to happen altogether, no?

[ardichoke]

That's quite odd, when I tested this initially, Socket.gethostname did return the fqdn. Subsequent testing on other hosts doesn't though.

Passing a hostname explicitly would be an option if I was writing something custom that uses dogapi, but what I'm trying to fix is the puppet datadog report tool. When our puppetservers are running hot, the report tool starts failing because the JVM cannot allocate memory to shell out and run hostname.

[therve]

I'm not very familiar with the puppet integration, but don't wee pass the host here: https://github.com/DataDog/puppet-datadog-agent/blob/0d3fedc5669bbc65558ab7d0820853c725829443/lib/puppet/reports/datadog_reports.rb#L125 ?

[ardichoke]

Hmm... curious. It certainly seems like that should be passing the hostname. This issue was from when we were using the report plugin previously, we actually ripped it out of our environment because it was so unreliable. It looks like the code that passes in the hostname predates when we had this issue, but we definitely had issues with JVM being unable to allocate memory to shell out for the hostname call in dogapi which was what led us to entirely abandoning the report tool. I'll have to dig into this further.

Just my 2 cents, even ignoring the issues we have experienced with this in the past, it seems avoiding the shell-out whenever possible is just good practice especially as Ruby has built in functions to provide this. Would a change to my PR which tried the Socket functions first, and only if those failed attempts to shell out be more welcome?

[therve]

The issue is that the socket functions will give you a value, but I don't know if you can tell if it failed because the value won't be correct, but you won't have anything to compare it to.

Ruby has built in functions to provide this.

The core issue is that it doesn't seem like Ruby has the exact function that we need.

[ardichoke]

Okay, to be fair, the hostname binary can also return the wrong value in the same situations. Namely, where the host is misconfigured. I see that you've made your mind up that shelling out unnecessarily is the better option though, so I won't waste more of my time on this.

[therve]

I haven't really made up my mind, I'm just cautious to touch some code that has been that way for 10 years. #179 (comment) is somewhat giving the reasons why we need to be careful here. If we find the Ruby API that consistently gives the same result as hostname -f I'm happy to use it.