python-elasticsearch-logger icon indicating copy to clipboard operation
python-elasticsearch-logger copied to clipboard

Set localhost if gethostbyname() is un-resolvable

Open geek876 opened this issue 6 years ago • 5 comments

… the 'host' can't be resolved either via DNS or via local-hosts file. Setting 'localhost' under such cases.

geek876 avatar Nov 19 '17 02:11 geek876

Codecov Report

Merging #34 into master will decrease coverage by 0.96%. The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
- Coverage   82.78%   81.81%   -0.97%     
==========================================
  Files           2        2              
  Lines         151      154       +3     
  Branches       18       18              
==========================================
+ Hits          125      126       +1     
- Misses         22       24       +2     
  Partials        4        4
Impacted Files Coverage Δ
cmreslogging/handlers.py 80.95% <50%> (-1%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6765a24...1539d2c. Read the comment docs.

codecov-io avatar Nov 19 '17 02:11 codecov-io

Appreciate the pull request, but I'm not entirely sure I'm comfortable adding this to master for the following reason:

  • The fix basically sets a default to localhost with ip 127.0.0.1 if we cannot resolve the name of the current host... If we've come to this point is because the host network has been poorly configured or has been misconfigured somehow.
  • Instead to fix the root cause (make the system resolve properly the current host) the PR basically sets a default to localhost with ip 127.0.0.1... So for a distributed logging system we end up reporting that logs are coming from 127.0.0.1/localhost which will be misleading too, at least from the point of view of identifying which of the nodes reporting logs in a distributed network has issues.

Rather than adding a default source that might be misleading and not addressing the problem, I think I'd rather prefer a behaviour that forces a fail fast and points out what the problem is and how to fix it so users can remediate root problems at source.

cmanaha avatar Nov 20 '17 23:11 cmanaha

Ran into a similar problem today, and came across this SO answer for determining your IP address.

https://stackoverflow.com/a/28950776

This worked for me, even though socket.gethostbyname(socket.gethostname()) threw an exception.

I completely agree with the fail fast approach, however, I think we can add some robustness to the library if we develop a simple function that runs through a variety a different methods and then ultimately fails if none are successful.

dougiep16 avatar Nov 21 '17 20:11 dougiep16

I think we can add some robustness to the library if we develop a simple function that runs through a variety a different methods and then ultimately fails if none are successful.

@dougiep16: That makes sense to me as well.

@yamadmia: If you opt for this approach, l'd suggest considering moving that part of the code (finding out info on the current running host) to a different file so we try to keep the handler clean. Most of other methods to find out the IP address might be platform dependent. Whatever we do, we must ensure the handler remains platform independent.

cmanaha avatar Nov 21 '17 23:11 cmanaha

seems still reasonable for mac users

wotori avatar Nov 18 '22 12:11 wotori