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

Make telemetry task use a schedule instead of scheduling explicitly for midnight #153380

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Mar 21, 2023

Fixes #140973
Fixes https://github.com/elastic/kibana-team/issues/563

In this PR, I'm fixing flaky tests that caused extra telemetry runs whenever CI would run them near midnight UTC. The assertion expected two runs while sometimes a 3rd run would happen if the test ran near midnight when the telemetry task was scheduled to run again..

To fix this, I've moved away from the midnight scheduling given telemetry only needs to be reported daily, and moved the task to use a schedule within task manager to make the task run daily (+24hrs from the previous run). This also improves error handling given task manager will now know it's a recurring task and recurring tasks never get marked as failed.

The following verification steps can be done using this query in Dev Tools

GET .kibana_task_manager/_search
{
  "query": {
    "term": {
      "task.taskType": "actions_telemetry"
    }
  }
}

To verify existing tasks migrating to a schedule

  1. Using main, setup a fresh Kibana and ES instance
  2. Keep Elasticsearch running but shut down Kibana after setup is complete
  3. Switch from main to this PR
  4. Add await taskManager.runSoon(TASK_ID); after the ensureScheduled call within x-pack/plugins/actions/server/usage/task.ts.
  5. Startup Kibana
  6. Go in Dev Tools and pull the task information to see a new schedule attribute added

To verify fresh installs

  1. Using this PR code, setup a fresh Kibana and ES instance
  2. Go in Dev Tools and pull the task information to see a new schedule attribute added

Flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2017

@mikecote mikecote added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:Actions/Framework Issues related to the Actions Framework labels Mar 21, 2023
@mikecote mikecote self-assigned this Mar 21, 2023
@mikecote
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote mikecote marked this pull request as ready for review March 22, 2023 19:00
@mikecote mikecote requested a review from a team as a code owner March 22, 2023 19:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

} from '@kbn/task-manager-plugin/server';
import { PreConfiguredAction } from '../types';
import { getTotalCount, getInUseTotalCount, getExecutionsPerDayCount } from './actions_telemetry';

export const TELEMETRY_TASK_TYPE = 'actions_telemetry';

export const TASK_ID = `Actions-${TELEMETRY_TASK_TYPE}`;
export const SCHEDULE: IntervalSchedule = { interval: '15s' };
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be 1d?

Copy link
Contributor Author

@mikecote mikecote Mar 22, 2023

Choose a reason for hiding this comment

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

Whops, ended up committing something for testing. Reverted back in 412db8d. That would have been A LOT of telemetry collection 😆

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
taskManager 39 40 +1

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
taskManager 7 6 -1
Unknown metric groups

API count

id before after diff
taskManager 80 82 +2

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

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

cc @mikecote

@mikecote mikecote added release_note:skip Skip the PR/issue when compiling release notes v8.8.0 labels Mar 23, 2023
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!

@mikecote mikecote merged commit 6a16932 into elastic:main Mar 23, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Actions/Framework Issues related to the Actions Framework Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.8.0
Projects
None yet
5 participants