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

xPack:defaultAdminEmail should be marked deprecated in the UI #25439

Closed
LeeDr opened this issue Nov 8, 2018 · 18 comments
Closed

xPack:defaultAdminEmail should be marked deprecated in the UI #25439

LeeDr opened this issue Nov 8, 2018 · 18 comments
Labels
bug Fixes for quality problems that affect the customer experience

Comments

@LeeDr
Copy link
Contributor

LeeDr commented Nov 8, 2018

See #22195

Kibana version: 6.5.0

Elasticsearch version: 6.5.0

Server OS version: Ubuntu

Browser version: Chrome

Browser OS version: Windows 10

Original install method (e.g. download page, yum, from source, etc.): tar.gz

Describe the bug: We let users go to Advanced Settings and enter xPack:defaultAdminEmail without any indication that it's deprecated, and then we log (only after it's used?) that it's deprecated in the the kibana log file.

Seems we should tell the user the setting is deprecated before they set it.

Steps to reproduce:

  1. Go to Advanced Settings and find that there's no indication that xPack:defaultAdminEmail is deprecated
  2. Set an email address in xPack:defaultAdminEmail
  3. check log for deprecation warning (nothing yet)
  4. trigger one of the built-in watcher alerts (starting up another node is one way)
  5. Check that you do get the email.
  6. Check that you also get the warning about the deprecation as shown below.

Expected behavior: Change the text that appears in the UI to say "DEPRECATED see ... some doc)

Screenshots (if relevant):
image

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):
{"type":"log","@timestamp":"2018-11-08T22:00:17Z","tags":["warning","stats-collection"],"pid":3478,"message":"Monitoring is using xPack:defaultAdminEmail for cluster alert notifications, which will not be supported in Kibana 7.0. Please configure xpack.monitoring.cluster_alerts.email_notifications.email_address in your kibana.yml settings"}

Any additional context:

@LeeDr LeeDr added bug Fixes for quality problems that affect the customer experience Team:Monitoring Stack Monitoring team labels Nov 8, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-monitoring

@legrego
Copy link
Member

legrego commented Nov 9, 2018

Hey @LeeDr,

The xPack:defaultAdminEmail setting actually has 2 uses today:

  1. Address to send cluster alert notifications from Monitoring.
  2. Default address when creating a new Watcher.

While they are similar to each other, only #1 is deprecated. The setting itself will not be removed in 7.0, but it's usage as part of 1 will be.

Rather than marking the setting as deprecated, what if we were to change the description to something like:

Default recipient email address for creating a new Watcher.

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 9, 2018

I don't see it doing #2 above. Is it the From or To email? When I create a new watch and add an email action the To field is blank (not defaulted to that value as I thought it might).

@legrego
Copy link
Member

legrego commented Nov 9, 2018

@LeeDr should be used as the default "To e-mail" when creating a watcher alert notification:

image

image

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 9, 2018

Sorry, my mistake. That advanced setting does appear as the default email on a new watch.

@cachedout cachedout removed the Team:Monitoring Stack Monitoring team label Apr 11, 2019
@legrego legrego removed their assignment Apr 11, 2019
@marius-dr
Copy link
Member

We should at least update the description in Advanced Settings now in 7.x since it's wrong as Monitoring won't be using it.

@marius-dr
Copy link
Member

also @elastic/stack-monitoring : was this supposed to be removed in 7.0? I still see the same warning about deprecation in the logs:

  log   [14:02:00.425] [warning][stats-collection] Monitoring is using xPack:defaultAdminEmail for cluster alert notifications, which will not be supported in Kibana 7.0. Please configure xpack.monitoring.cluster_alerts.email_notifications.email_address in your kibana.yml settings

@cachedout
Copy link
Contributor

@marius-dr I believe it's in progress over here: #33603

@renshuki
Copy link
Contributor

Is it possible to move this forward?
Having xPack:defaultAdminEmail with no deprecated mention or whatsoever in the UI is misleading.

@chrisronline
Copy link
Contributor

This is probably too old to do anything about. This setting was deprecated during 6.x and completely removed in 7.0

@ypid-geberit
Copy link

Hi @chrisronline

This is probably too old to do anything about. This setting was deprecated during 6.x and completely removed in 7.0

I am confused about this statement. The setting is still present in Elastic 7.8 (tested locally) and 7.7 (https://demo.elastic.co/) with no deprecation note.

I then checked where this setting is actually used by the system watches. Seems Stack monitoring somehow copies it to .monitoring-kibana-7-* where painless ultimately uses it for alerting (ctx.payload.kibana_settings.hits.hits[0]._source.kibana_settings.xpack.default_admin_email)

Ref: https://www.elastic.co/guide/en/kibana/current/advanced-options.html

Please give more details when closing an issue so that people finding this have more context.

@chrisronline
Copy link
Contributor

Hi @ypid-geberit,

Thanks for raising your concerns.

You are right, this particular setting still does appear in 7.7 and 7.8, as #33603 was not backported, due to breaking change concerns.

So it is a broken experience in 7.x where the code no longer reads this setting due to #60796.

This was a mistake on our side and I apologize for not fully understanding that from the start. I'll reopen this ticket and we will work to add a proper deprecation notice for this behavior, as well as also updating the code to read from both locations again.

@ypid-geberit
Copy link

ypid-geberit commented Jun 26, 2020

Thanks. Then only one thing is unclear to me. What is the recommended way to set the admin email (for the system watches) then? I only found https://www.elastic.co/guide/en/cloud/current/ec-manage-kibana-settings.html xpack.monitoring.cluster_alerts.email_notifications.email_address. Setting this in Kibana seems confusing to me. The watches using this setting live in ES. The cluster settings would feel more appropriate to me as there also the email account is probably configured. Cluster settings are probably not accessible for watches.

@chrisronline
Copy link
Contributor

The only way it will work (until we patch this) for 7.7 and beyond is to set it inside of kibana.yml under monitoring.cluster_alerts.email_notifications.email_address. We will have a fix for this soon so it will continue to work from the advanced setting, but that same PR will also mark that as deprecated and suggest to use the kibana.yml setting instead.

Does that help?

@igoristic
Copy link
Contributor

@chrisronline Shouldn't the xpack prefix be removed (for monitoring settings) from this doc: https://www.elastic.co/guide/en/cloud/current/ec-manage-kibana-settings.html?

@chrisronline
Copy link
Contributor

@igoristic Yes, but those older configs are only deprecated. We should ensure we update that before 8.0 is released though.

@ypid-geberit
Copy link

Does that help?

All clear, thank you :)

@igoristic
Copy link
Contributor

Should of been closed by: #70280

Stack Monitoring UI automation moved this from To do to Done Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience
Projects
No open projects
Development

No branches or pull requests

9 participants