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.

Sign up for GitHub

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

Jump to bottom

[Alerting] Track deprecated configs #113015

Merged
merged 11 commits into from
Oct 6, 2021
Merged

[Alerting] Track deprecated configs #113015

merged 11 commits into from
Oct 6, 2021

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Sep 23, 2021 •
edited
Loading

Resolves #112585
Replaces #112609

This adds usage telemetry for:

  • xpack.actions.whitelistedHosts
  • xpack.actions.customHostSettings.ssl.rejectUnauthorized
  • xpack.actions.rejectUnauthorized
  • xpack.actions.proxyRejectUnauthorizedCertificates
  • xpack.alerts.healthCheck
  • xpack.alerts.invalidateApiKeysTask.interval
  • xpack.alerts.invalidateApiKeysTask.removalDelay
  • xpack.task_manager.max_workers

To test, add these configs to your kibana.yml and hit the stats HTTP API (https://localhost:5601/api/stats?extended). You should see some data in kibana_config_usage and some in deprecated_keys

{
  "deprecated_keys": {
    "unset": [
      "xpack.actions.whitelistedHosts",
      "xpack.actions.customHostSettings.ssl.rejectUnauthorized",
      "xpack.actions.rejectUnauthorized",
      "xpack.actions.proxyRejectUnauthorizedCertificates",
      "xpack.alerts.healthCheck",
      "xpack.alerts.invalidateApiKeysTask.interval",
      "xpack.alerts.invalidateApiKeysTask.removalDelay"
    ]
  }
}

{
  "kibana_config_usage": {
    "xpack_task_manager_max_workers": 120,
  }
}

(kibana.yml for the above)

xpack.actions.whitelistedHosts: ['smtp.gmail.com']
xpack.actions.allowedHosts: ['smtp.gmail.com']
xpack.actions.rejectUnauthorized: true
xpack.actions.proxyRejectUnauthorizedCertificates: true
xpack.alerting.healthCheck.interval: 60m
xpack.alerts.healthCheck.interval: 120m
xpack.alerts.invalidateApiKeysTask.interval: 5m
xpack.alerts.invalidateApiKeysTask.removalDelay: 2h
xpack.task_manager.max_workers: 120

@chrisronline chrisronline self-assigned this Sep 23, 2021
@chrisronline chrisronline added Feature:Actions/Framework Issues related to the Actions Framework Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0 labels Sep 23, 2021
@chrisronline chrisronline marked this pull request as ready for review September 23, 2021 17:41
@chrisronline chrisronline requested a review from a team as a code owner September 23, 2021 17:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@ymao1
Copy link
Contributor

ymao1 commented Sep 28, 2021

This is unrelated to this PR but maybe a quick fix in this PR?

I had this setting in my config:

xpack.actions.customHostSettings:
  - url: smtp://smtp.gmail.com
    ssl:
      rejectUnauthorized: false
      certificateAuthoritiesFiles: '/Users/ymao/Code/kibana/packages/kbn-dev-utils/certs/ca.crt'
      certificateAuthoritiesData: |
          -----BEGIN CERTIFICATE-----
          ... multiple lines of certificate data here ...
          -----END CERTIFICATE-----
    smtp:
      requireTLS: true

and I noticed I was not getting a deprecation warning for rejectUnauthorized and not seeing the unset for the deprecated value. It looks like the logic checking for rejectUnauthorized does not evaluate correctly if rejectUnauthorized: false. Should I open a new issue to fix or do you think we can include in this PR?

@ymao1
Copy link
Contributor

ymao1 commented Sep 28, 2021 •
edited
Loading

Same thing for xpack.actions.rejectUnauthorized and xpack.actions.proxyRejectUnauthorizedCertificates when they are set to false...the deprecation warning does not get logged.

@chrisronline
Copy link
Contributor Author

@ymao1 Nice catch, I'll fix those in this PR

@chrisronline chrisronline force-pushed the alerting/deprecated_config_telemetry branch from 94b1986 to 3bdfcc1 Compare September 28, 2021 14:23
@chrisronline
Copy link
Contributor Author

@ymao1 I made those changes, ready for another pass!

ymao1
ymao1 approved these changes Sep 30, 2021
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

YulNaumenko
YulNaumenko approved these changes Oct 4, 2021
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @chrisronline

@chrisronline chrisronline merged commit f861147 into elastic:master Oct 6, 2021
@chrisronline chrisronline deleted the alerting/deprecated_config_telemetry branch October 6, 2021 16:09
@chrisronline chrisronline mentioned this pull request Oct 6, 2021
Merged
chrisronline added a commit to chrisronline/kibana that referenced this pull request Oct 6, 2021
@chrisronline
[Alerting] Track deprecated configs (elastic#113015)
726ab4f 
* Track deprecated configs

* PR feedback

* Be more careful

* Add test back in

* Fix types

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	x-pack/plugins/actions/server/index.test.ts
chrisronline added a commit that referenced this pull request Oct 6, 2021
@chrisronline
[Alerting] Track deprecated configs (#113015) (#114147)
1916ab4 
* Track deprecated configs

* PR feedback

* Be more careful

* Add test back in

* Fix types

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	x-pack/plugins/actions/server/index.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YulNaumenko YulNaumenko YulNaumenko approved these changes

@ymao1 ymao1 ymao1 approved these changes

Assignees

@chrisronline chrisronline

Labels
Feature:Actions/Framework Issues related to the Actions Framework Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Alerting] Add telemetry for config deprecation
5 participants
@chrisronline @elasticmachine @ymao1 @kibanamachine @YulNaumenko