Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log message suggesting setting network.host if using EC2 discovery #15518

Closed
peterskim12 opened this issue Dec 17, 2015 · 14 comments
Closed

Log message suggesting setting network.host if using EC2 discovery #15518

peterskim12 opened this issue Dec 17, 2015 · 14 comments
Labels
:Core/Infra/Logging Log management and logging utilities >enhancement good first issue low hanging fruit help wanted adoptme

Comments

@peterskim12
Copy link
Contributor

@dadoonet looked into the feasibility of setting more helpful defaults for network.host if EC2 discovery is being used with the AWS plugin in #13970. He found good reasons not to do so though.

I still wonder if we can improve the user experience by logging a message suggesting they may want to consider setting network.host to ec2 or one of the other EC2 specific options if discovery.type: ec2 is set. @dedemorton 's idea.

@rmuir
Copy link
Contributor

rmuir commented Dec 17, 2015

The problem is not specific to ec2. Its that ES was so promiscuous before, bound to every interface, and then screamed via multicast to any computers it possibly could.

Now defaults are for localhost only, but people don't read the documentation.

So maybe we should just loudly scream something like:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@ WARNING: DEFAULTING TO LOCALHOST ONLY
@ Please set network.host....

Basically impl-wise this would mean we default to some internal value like _local_warn_ or something instead of _local_

@rmuir
Copy link
Contributor

rmuir commented Dec 17, 2015

I would even think about something like 'network is not configured' and pointing to a docs page specific to how to configure it for most use cases (worst case: https://www.elastic.co/guide/en/elasticsearch/reference/2.x/modules-network.html#common-network-settings)

The problem is: typically I think at a minimum, you need to adjust two things, regardless of what plugins you have:

  1. what addresses you listen on: (network.host)
  2. what addresses you talk to: (discovery settings)

For most users, this means setting up the unicast list as well as setting network.host.

This issue as worded assumes the user correctly installed the plugin, and correctly did 2 but not 1. There are more ways that things can go wrong.

@dedemorton
Copy link
Contributor

I'd avoid providing links to specific doc pages, though, because content moves around, and the links could get stale.

@rmuir
Copy link
Contributor

rmuir commented Dec 18, 2015

docs are versioned.

@debadair
Copy link
Contributor

But do we want to have to update error messages for each version? Otherwise, we'd have error messages pointing to a specific version that isn't the current version. If we just pointed to 2.x or current, then we're back to the issue of links potentially getting stale.

While it's not a big deal for one error message, it could turn into a real headache if we start doing this more generally without having a way to update/validate the links for each release.

@rmuir
Copy link
Contributor

rmuir commented Dec 18, 2015

if we want to link to docs, we just need a constant in Version like DOCROOT or whatever.

@debadair
Copy link
Contributor

But then when the version is bumped, there's no guarantee that the links are still valid in the new version--we'd need to validate them, which can't really be done until the docs for the new version go live.

Totally agree that linking to the docs would be really helpful--I could totally see wanting to do this in a number of cases, as long as they can be tested & easily updated.

@rmuir
Copy link
Contributor

rmuir commented Dec 18, 2015

There are no guarantees on anything, i mean if i look at the docs for a random page (https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-transport.html) I can see that transport.tcp.compress is now wrong, since we no longer use the LZF algorithm.

Does this mean we should just do nothing for this issue at all? Its somehow quickly turned into 'but our error messages and docs must be unbreakable!' when they are far, far from that right now... when IMO all that is needed is some improved messaging "Hey, you didn't configure your network, we defaulted to localhost only". It could reduce a lot of confusion.

@debadair
Copy link
Contributor

Not saying we should do nothing--absolutely agree we should add the log message. My personal preference would be for a more-verbose message over embedding a link to the docs, but if we can add tests that extract the anchor from the URL and verify that the anchor still exists in the doc repo, I think the links would be great, too.

@rmuir
Copy link
Contributor

rmuir commented Dec 18, 2015

I guess my problem is we already spent half a day figuring out the docs for https://www.elastic.co/guide/en/elasticsearch/reference/2.x/modules-network.html#common-network-settings

So i wanted to reuse that, rather than re-enter a bikeshed about what the exact log messages should be and all that nonsense. This is why these kinds of things do not get done...

@debadair
Copy link
Contributor

Understood. I think we can all agree that any message is better than no message, it doesn't have to be perfect. I would suggest the following:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@ WARNING: NO NETWORK CONFIGURED, DEFAULTING TO LOCALHOST
@ Set network.host to specify the host's listening address. To use Zen Discovery to
@ join a cluster, set discovery.zen.ping.unicast.hosts.

@dedemorton
Copy link
Contributor

+1 for Deb's suggestion. Deep linking is great when you can validate that the links work, but difficult to maintain and potentially very frustrating to users when the links are broken (been there, done that). You need an infrastructure to support deep linking, or you're opening up a can of worms (and the worms always end up in someone's belly). Of course, if someone wants to step forward and create an infrastructure of validating the links, I'm all for that....

@clintongormley clintongormley added >enhancement good first issue low hanging fruit help wanted adoptme :Core/Infra/Logging Log management and logging utilities labels Feb 29, 2016
@asettouf
Copy link

I made my first ever pull request to contribute to this enhancement based on what was suggested in that topic. Please do let me know if there is anything wrong with it (pretty likely I guess) as it is my first time making one.

@jasontedor
Copy link
Member

I do not think we should log a message here, we already make it clear in the logs the bound addresses. I think the particular issue here is best left to the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Logging Log management and logging utilities >enhancement good first issue low hanging fruit help wanted adoptme
Projects
None yet
Development

No branches or pull requests

7 participants