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

Fix missing node id prefix in startup logs #29534

Merged
merged 2 commits into from
Apr 19, 2018

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Apr 16, 2018

When node.name is not set, some log traces at startup time does not show the node id. It does not look nice as some traces have the id and some others haven't:

[2018-04-16T16:02:14,812][INFO ][o.e.n.Node               ] [] initializing ...
[2018-04-16T16:02:14,847][INFO ][o.e.e.NodeEnvironment    ] [P2NUhLl] heap size [989.8mb], compressed ordinary object pointers [true]
[2018-04-16T16:02:14,848][INFO ][o.e.n.Node               ] node name [P2NUhLl] derived from node ID [P2NUhLlQTkmAHzlNM4HF2A]; set [node.name] to override
[2018-04-16T16:02:14,849][INFO ][o.e.n.Node               ] version[7.0.0-alpha1-SNAPSHOT], ... ]
[2018-04-16T16:02:14,849][INFO ][o.e.n.Node               ] JVM arguments [-Xms1g, -Xmx1g, ...]
[2018-04-16T16:02:14,849][WARN ][o.e.n.Node               ] version [7.0.0-alpha1-SNAPSHOT] is a pre-release version of Elasticsearch and is not suitable for production
...
[2018-04-16T16:02:16,373][INFO ][o.e.d.DiscoveryModule    ] [P2NUhLl] using discovery type [zen]
[2018-04-16T16:02:16,642][INFO ][o.e.n.Node               ] initialized

We can improve this a bit in order to make startup logs parsing easier (and to not hurt my eyes):

[2018-04-16T16:02:14,812][INFO ][o.e.n.Node               ] [] initializing ...
[2018-04-16T16:02:14,847][INFO ][o.e.e.NodeEnvironment    ] [P2NUhLl] heap size [989.8mb], compressed ordinary object pointers [true]
[2018-04-16T16:02:14,848][INFO ][o.e.n.Node               ] [P2NUhLl] node name [P2NUhLl] derived from node ID [P2NUhLlQTkmAHzlNM4HF2A]; set [node.name] to override
[2018-04-16T16:02:14,849][INFO ][o.e.n.Node               ] [P2NUhLl] version[7.0.0-alpha1-SNAPSHOT], ... ]
[2018-04-16T16:02:14,849][INFO ][o.e.n.Node               ] [P2NUhLl] JVM arguments [-Xms1g, -Xmx1g, ...]
[2018-04-16T16:02:14,849][WARN ][o.e.n.Node               ] [P2NUhLl] version [7.0.0-alpha1-SNAPSHOT] is a pre-release version of Elasticsearch and is not suitable for production
...
[2018-04-16T16:02:16,373][INFO ][o.e.d.DiscoveryModule    ] [P2NUhLl] using discovery type [zen]
[2018-04-16T16:02:16,642][INFO ][o.e.n.Node               ] [P2NUhLl] initialized

When `node.name` is not set, some log traces at startup time does not
show the node id.
@tlrx tlrx added review :Core/Infra/Logging Log management and logging utilities v7.0.0 labels Apr 16, 2018
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good plan! Shame about the first initializing ... line :)

Does it make sense to remove the node name from this message, since it now appears twice?

[2018-04-16T16:02:14,848][INFO ][o.e.n.Node               ] [P2NUhLl] node name [P2NUhLl] derived from node ID [P2NUhLlQTkmAHzlNM4HF2A]; set [node.name] to override

@tlrx
Copy link
Member Author

tlrx commented Apr 17, 2018

Shame about the first initializing ... line :)

Well, at least it shows that the node.name is unknown when initializing the node. And it also respects the same log pattern :)

Does it make sense to remove the node name from this message, since it now appears twice?

Yes, I removed it.

Thanks for the review!

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brill. LGTM.

@hub-cap hub-cap added :Core/Infra/Logging Log management and logging utilities and removed :Core/Infra/Logging Log management and logging utilities labels Apr 17, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@tlrx tlrx merged commit e2d770d into elastic:master Apr 19, 2018
@tlrx
Copy link
Member Author

tlrx commented Apr 19, 2018

Thanks @DaveCTurner :)

@tlrx tlrx added the v6.3.0 label Apr 19, 2018
tlrx added a commit that referenced this pull request Apr 19, 2018
When `node.name` is not set, some log traces at startup time does not
show the node id.
@tlrx tlrx deleted the node-name-in-startup-logs branch April 19, 2018 08:04
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 19, 2018
* master: (21 commits)
  Remove bulk fallback for write thread pool (elastic#29609)
  Fix an incorrect reference to 'zero_terms_docs' in match_phrase queries.
  Update the version compatibility for zero_terms_query in match_phrase.
  Account translog location to ram usage in version map
  Remove extra spaces from changelog
  Add support to match_phrase query for zero_terms_query. (elastic#29598)
  Fix incorrect references to 'zero_terms_docs' in query parsing error messages. (elastic#29599)
  Build: Move java home checks to pre-execution phase (elastic#29548)
  Avoid side-effect in VersionMap when assertion enabled (elastic#29585)
  [Tests] Remove accidental logger usage
  Add tests for ranking evaluation with aliases (elastic#29452)
  Deprecate use of `htmlStrip` as name for HtmlStripCharFilter (elastic#27429)
  Update plan for the removal of mapping types. (elastic#29586)
  [Docs] Add rankEval method for Jva HL client
  Make ranking evaluation details accessible for client
  Rename the bulk thread pool to write thread pool (elastic#29593)
  [Test] Minor changes to rank_eval tests (elastic#29577)
  Fix missing node id prefix in startup logs (elastic#29534)
  Added painless execute api. (elastic#29164)
  test: also assert deprecation warning after clusters have been closed.
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants