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

[Alerting] Deprecated config telemetry #112609

Closed

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Sep 20, 2021

Resolves #112585

This adds usage telemetry for:

  • xpack.actions.whitelistedHosts (We had to slightly refactor how this deprecation worked as a result of [Alerting] Add telemetry for config deprecation #112585 (comment))
  • 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, then hit the stats HTTP API (https://localhost:5601/api/stats?extended), and look for kibana_config_usage key, such as:

{
  "kibana_config_usage": {
    "xpack_actions_whitelisted_hosts": ["smtp.gmail.com"],
    "xpack_actions_reject_unauthorized": true,
    "xpack_actions_proxy_reject_unauthorized_certificates": true,
    "xpack_alerting_legacy_tracking_purposes_health_check_interval": "120m",
    "xpack_alerting_legacy_tracking_purposes_invalidate_api_keys_task_interval": "5m",
    "xpack_alerting_legacy_tracking_purposes_invalidate_api_keys_task_removal_delay": "2h",
    "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 added Feature:Actions/Framework Issues related to the Actions Framework Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Task Manager Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0 labels Sep 21, 2021
@chrisronline chrisronline self-assigned this Sep 21, 2021
@chrisronline chrisronline added the release_note:skip Skip the PR/issue when compiling release notes label Sep 21, 2021
@chrisronline chrisronline marked this pull request as ready for review September 21, 2021 18:05
@chrisronline chrisronline requested a review from a team as a code owner September 21, 2021 18:05
@elasticmachine
Copy link
Contributor

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

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

code LGTM, but I added a comment about using the old and new allowHosts settings in a config and not getting a warning about it - feels like something we should do unless I'm misunderstanding something

x-pack/plugins/actions/server/config.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/index.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/index.ts Outdated Show resolved Hide resolved
@ymao1
Copy link
Contributor

ymao1 commented Sep 22, 2021

It looks like xpack_alerting_health_check_interval, xpack_alerting_invalidate_api_keys_task_interval and xpack_alerting_invalidate_api_keys_task_removal_delay are counted whether the config is xpack.alerts.* or xpack.alerting.*. Do we need to differentiate between the two so we can keep track specifically of deprecated usages of xpack.alerts.*?

@chrisronline chrisronline force-pushed the alerting/config_telemetry branch from 424cc3d to eb12721 Compare September 22, 2021 17:49
@chrisronline chrisronline requested a review from a team as a code owner September 22, 2021 17:49
@chrisronline
Copy link
Contributor Author

@ymao1 Great point, I'll fix that up

@chrisronline
Copy link
Contributor Author

@Bamieh Would you mind looking at the implementation of this and letting me know if you think it'll work on the telemetry side of things?

@@ -54,10 +55,61 @@ export { ACTION_SAVED_OBJECT_TYPE } from './constants/saved_objects';

export const plugin = (initContext: PluginInitializerContext) => new ActionsPlugin(initContext);

// Use a custom copy function here so we can perserve the telemetry provided for the deprecated config
// See https://github.com/elastic/kibana/issues/112585#issuecomment-923715363
function renameFromRootAndTrackDeprecatedUsage(
Copy link
Member

Choose a reason for hiding this comment

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

I dont believe this is needed. please let me know if this comment makes sense: #112585 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this clarification!

@chrisronline
Copy link
Contributor Author

After syncing with @Bamieh in #112609 (comment), I think what we need here can be accomplished with much less code. I'm going to close this out and reopen a new PR with the changes necessary.

@kibanamachine
Copy link
Contributor

kibanamachine commented Sep 23, 2021

💔 Build Failed

Failed CI Steps


Test Failures

Kibana Pipeline / jest / Jest Tests.x-pack/plugins/alerting/server.config validation alerting defaults

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

Error: expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `config validation alerting defaults 1`

- Snapshot  - 9
+ Received  + 0

@@ -4,16 +4,7 @@
    },
    "invalidateApiKeysTask": Object {
      "interval": "5m",
      "removalDelay": "1h",
    },
-   "legacyTrackingPurposes": Object {
-     "healthCheck": Object {
-       "interval": "60m",
-     },
-     "invalidateApiKeysTask": Object {
-       "interval": "5m",
-       "removalDelay": "1h",
-     },
-   },
    "maxEphemeralActionsPerAlert": 10,
  }
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/plugins/alerting/server/config.test.ts:13:43)
    at Promise.then.completed (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at _runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at run (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

Metrics [docs]

✅ unchanged

History

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

cc @chrisronline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

[Alerting] Add telemetry for config deprecation
7 participants