Skip to content

Commit

Permalink
Make telemetry task use a schedule instead of scheduling explicitly f…
Browse files Browse the repository at this point in the history
…or midnight (#153380)

Fixes #140973
Fixes elastic/kibana-team#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

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
mikecote and kibanamachine authored Mar 23, 2023
1 parent bbe3d52 commit 6a16932
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 15 deletions.
12 changes: 6 additions & 6 deletions x-pack/plugins/actions/server/usage/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@
*/

import { Logger, CoreSetup } from '@kbn/core/server';
import moment from 'moment';
import {
RunContext,
TaskManagerSetupContract,
TaskManagerStartContract,
IntervalSchedule,
} 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: '1d' };

export function initializeActionsTelemetry(
logger: Logger,
Expand Down Expand Up @@ -71,6 +72,7 @@ async function scheduleTasks(logger: Logger, taskManager: TaskManagerStartContra
taskType: TELEMETRY_TASK_TYPE,
state: {},
params: {},
schedule: SCHEDULE,
});
} catch (e) {
logger.debug(`Error scheduling task, received ${e.message}`);
Expand Down Expand Up @@ -133,14 +135,12 @@ export function telemetryTaskRunner(
count_connector_types_by_action_run_outcome_per_day:
totalExecutionsPerDay.countRunOutcomeByConnectorType,
},
runAt: getNextMidnight(),
// Useful for setting a schedule for the old tasks that don't have one
// or to update the schedule if ever the frequency changes in code
schedule: SCHEDULE,
};
});
},
};
};
}

function getNextMidnight() {
return moment().add(1, 'd').startOf('d').toDate();
}
16 changes: 9 additions & 7 deletions x-pack/plugins/alerting/server/usage/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
*/

import { Logger, CoreSetup } from '@kbn/core/server';
import moment from 'moment';
import {
RunContext,
TaskManagerSetupContract,
TaskManagerStartContract,
IntervalSchedule,
} from '@kbn/task-manager-plugin/server';

import { getFailedAndUnrecognizedTasksPerDay } from './lib/get_telemetry_from_task_manager';
Expand All @@ -23,6 +23,7 @@ import {
export const TELEMETRY_TASK_TYPE = 'alerting_telemetry';

export const TASK_ID = `Alerting-${TELEMETRY_TASK_TYPE}`;
export const SCHEDULE: IntervalSchedule = { interval: '1d' };

export function initializeAlertingTelemetry(
logger: Logger,
Expand Down Expand Up @@ -69,6 +70,7 @@ async function scheduleTasks(logger: Logger, taskManager: TaskManagerStartContra
taskType: TELEMETRY_TASK_TYPE,
state: {},
params: {},
schedule: SCHEDULE,
});
} catch (e) {
logger.debug(`Error scheduling task, received ${e.message}`);
Expand Down Expand Up @@ -189,22 +191,22 @@ export function telemetryTaskRunner(
percentile_num_alerts_by_type_per_day:
dailyExecutionCounts.alertsPercentilesByType,
},
runAt: getNextMidnight(),
// Useful for setting a schedule for the old tasks that don't have one
// or to update the schedule if ever the frequency changes in code
schedule: SCHEDULE,
};
}
)
.catch((errMsg) => {
logger.warn(`Error executing alerting telemetry task: ${errMsg}`);
return {
state: {},
runAt: getNextMidnight(),
// Useful for setting a schedule for the old tasks that don't have one
// or to update the schedule if ever the frequency changes in code
schedule: SCHEDULE,
};
});
},
};
};
}

function getNextMidnight() {
return moment().add(1, 'd').startOf('d').toDate();
}
1 change: 1 addition & 0 deletions x-pack/plugins/task_manager/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type {
EphemeralTask,
TaskRunCreatorFunction,
RunContext,
IntervalSchedule,
} from './task';

export { TaskStatus } from './task';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ export default function createAlertingAndActionsTelemetryTests({ getService }: F
const esTestIndexTool = new ESTestIndexTool(es, retry);
const supertestWithoutAuth = getService('supertestWithoutAuth');

// FLAKY: https://github.com/elastic/kibana/issues/140973
describe.skip('telemetry', () => {
describe('telemetry', () => {
const alwaysFiringRuleId: { [key: string]: string } = {};

beforeEach(async () => {
Expand Down

0 comments on commit 6a16932

Please sign in to comment.