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

TransportMasterNodeAction debug messages #50056

Closed
ppf2 opened this issue Dec 10, 2019 · 7 comments · Fixed by #50074
Closed

TransportMasterNodeAction debug messages #50056

ppf2 opened this issue Dec 10, 2019 · 7 comments · Fixed by #50074
Assignees
Labels
:ml Machine learning

Comments

@ppf2
Copy link
Member

ppf2 commented Dec 10, 2019

A few things related to the o.e.a.s.m.TransportMasterNodeAction logging messages.

User reported the following debug messages showing up after upgrading to 7.5.0 (from 7.3):

[2019-12-03T22:47:59,669][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:09,650][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:19,618][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:29,713][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:39,655][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:49,669][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'
[2019-12-03T22:48:59,668][DEBUG][o.e.a.s.m.TransportMasterNodeAction] [node.infra] Get stats for datafeed '_all'

These are showing up in the logs because Elasticsearch has the logger for org.elasticsearch.action set to DEBUG by default. It seems like we may want to use a different package to log these under?

Also, this user has a Gold license but their nodes had ML enabled (explicitly via xpack.ml.enabled: true) because they were previously testing ML. After setting xpack.ml.enabled: false, the above messages stopped showing up (expected). Should a gold license have disabled this type of monitoring stats for ML even when xpack.ml.enabled is set to true?

@ppf2 ppf2 added the :ml Machine learning label Dec 10, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

@dimitris-athanasiou
Copy link
Contributor

We'll surely move the log message to a more suitable place.

Also, this user has a Gold license but their nodes had ML enabled (explicitly via xpack.ml.enabled: true) because they were previously testing ML. After setting xpack.ml.enabled: false, the above messages stopped showing up (expected). Should a gold license have disabled this type of monitoring stats for ML even when xpack.ml.enabled is set to true?

Licensing does not affect what gets loaded as far as I understand. We check the license in specific API calls. But we want the flexibility of being able to load the plugin as that allows different features in the plugin to be licensed differently (e.g. basic features).

@dimitris-athanasiou dimitris-athanasiou self-assigned this Dec 11, 2019
@droberts195
Copy link
Contributor

Licensing does not affect what gets loaded as far as I understand

Yes, this is true, and we definitely don't want to change that. For example, someone who starts with a platinum license, creates some ML jobs, then downgrades to a gold license would be very frustrated if they then couldn't delete the jobs. So APIs like delete ML job are available and perform the expected action with a gold license. The feature table simplifies this down to "ML is platinum" because, for example, the delete ML job is pretty useless if you cannot create a job in the first place, and from a high level perspective it would be extremely misleading to say "some ML features are gold". But in the low level details it makes sense for the info/maintenance/cleanup bits of ML to work with gold.

@DaveCTurner
Copy link
Contributor

Note that #46431 adjusts the logger for this message to org.elasticsearch.xpack.ml.action.TransportGetDatafeedsStatsAction.

@dimitris-athanasiou
Copy link
Contributor

It does but only on master. Are there plans on backporting that @DaveCTurner ?

@davidkyle
Copy link
Member

Ah I was wondering why the message was logged at all as log4j2.properties specifies a different package

# log action execution errors for easier debugging
logger.action.name = org.elasticsearch.action
logger.action.level = debug

but this class and TransportGetJobStatsAction was using the super's logger. Thanks for cleaning that up @DaveCTurner but I think it is better to remove the debug messages which look like a remanent of some earlier testing.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Dec 11, 2019
Adjusts the subclasses of `TransportMasterNodeAction` to use their own loggers
instead of the one for the base class.

Relates elastic#50056.
Partial backport of elastic#46431 to 7.x.
@DaveCTurner
Copy link
Contributor

Yes, backporting those logging adjustments seems like a good idea. I opened #50076 for that.

DaveCTurner added a commit that referenced this issue Dec 11, 2019
Adjusts the subclasses of `TransportMasterNodeAction` to use their own loggers
instead of the one for the base class.

Relates #50056.
Partial backport of #46431 to 7.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants