Skip to content

Commit

Permalink
[Reporting] Delete the task from Task Manager when deleting a report (#…
Browse files Browse the repository at this point in the history
…192417)

## Summary

Closes #191676

This PR ensures that when a user deletes a report, if there is a backing
Task Manager task that references the report job, that task is deleted
as well. With these changes, the report job will truly be "cancelled"
when a report is deleted.

The risk of not cancelling the Task Manager task when deleting a report
is evident in server logs and in task monitoring, when automated tests
are run on the reporting features.

### How to test
Primarily, the API Integration test for the Reporting Datastream
exercises the changes in this PR.

### Checklist

- [x] Ensure the API/Functional tests are using the delete report
endpoint rather than directly deleting reports from the ES store.
* **To prevent this PR from becoming very large, this assurance is only
needed for serverless tests, and for tests that do not wait for the
report job to be completed before deleting the report document.**
* Those constrictions narrow the changes down to just the test on the
Reporting Datastream
- [x] Handle scenario of report document deleted before task begins or
completes
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
tsullivan and kibanamachine authored Oct 2, 2024
1 parent b2d821a commit dfee928
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export const commonJobsRouteHandlerFactory = (
const reportingSetup = reporting.getPluginSetupDeps();
const logger = reportingSetup.logger.get('delete-report');

logger.debug(`Deleting report ${docId}`);

// An "error" event is emitted if an error is
// passed to the `stream.end` callback from
// the _final method of the ContentStream.
Expand All @@ -121,6 +123,48 @@ export const commonJobsRouteHandlerFactory = (
logger.error(err);
});

// 1. Look for a task in task manager associated with the report job
try {
let taskId: string | undefined;
const { taskManager } = await reporting.getPluginStartDeps();
const result = await taskManager.fetch({
query: {
term: {
'task.taskType': { value: 'report:execute' },
},
},
size: 1000, // NOTE: this is an arbitrary size that is likely to include all running and pending reporting tasks in most deployments
});

if (result.docs.length > 0) {
// The task params are stored as a string of JSON. In order to find the task that corresponds to
// the report to delete, we need to check each task's params, look for the report id, and see if it
// matches our docId to delete.
for (const task of result.docs) {
const { params } = task;
if (params.id === docId) {
// found the matching task
taskId = task.id;
logger.debug(
`Found a Task Manager task associated with the report being deleted: ${taskId}. Task status: ${task.status}.`
);
break;
}
}
if (taskId) {
// remove the task that was found
await taskManager.remove(taskId);
logger.debug(`Deleted Task Manager task ${taskId}.`);
}
}
} catch (error) {
logger.error(
'Encountered an error in finding a task associated with the report being deleted'
);
logger.error(error);
}

// 2. Remove the report document
try {
// Overwriting existing content with an
// empty buffer to remove all the chunks.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ export default function ({ getService }: FtrProviderContext) {
};

describe('Data Stream', function () {
const generatedReports = new Set<string>();
before(async () => {
roleAuthc = await svlUserManager.createM2mApiKeyWithRoleScope('admin');
internalReqHeader = svlCommonApi.getInternalRequestHeader();

await esArchiver.load(archives.ecommerce.data);
await kibanaServer.importExport.load(archives.ecommerce.savedObjects);

// for this test, we don't need to wait for the job to finish or verify the result
await reportingAPI.createReportJobInternal(
const { job } = await reportingAPI.createReportJobInternal(
'csv_searchsource',
{
browserTimezone: 'UTC',
Expand All @@ -51,10 +51,15 @@ export default function ({ getService }: FtrProviderContext) {
roleAuthc,
internalReqHeader
);

generatedReports.add(job.id);
});

after(async () => {
await reportingAPI.deleteAllReports(roleAuthc, internalReqHeader);
for (const reportId of generatedReports) {
await reportingAPI.deleteReport(reportId, roleAuthc, internalReqHeader);
}

await esArchiver.unload(archives.ecommerce.data);
await kibanaServer.importExport.unload(archives.ecommerce.savedObjects);
await svlUserManager.invalidateM2mApiKeyWithRoleScope(roleAuthc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ export default function ({ getService }: FtrProviderContext) {
});

after(async () => {
await reportingAPI.deleteAllReports(roleAuthc, internalReqHeader);
await esArchiver.unload(archives.ecommerce.data);
await kibanaServer.importExport.unload(archives.ecommerce.savedObjects);
});
Expand Down
29 changes: 17 additions & 12 deletions x-pack/test_serverless/shared/services/svl_reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import expect from '@kbn/expect';
import { INTERNAL_ROUTES } from '@kbn/reporting-common';
import type { ReportingJobResponse } from '@kbn/reporting-plugin/server/types';
import { REPORTING_DATA_STREAM_WILDCARD_WITH_LEGACY } from '@kbn/reporting-server';
import rison from '@kbn/rison';
import { FtrProviderContext } from '../../functional/ftr_provider_context';
import { RoleCredentials } from '.';
Expand Down Expand Up @@ -111,17 +110,23 @@ export function SvlReportingServiceProvider({ getService }: FtrProviderContext)
.set(roleAuthc.apiKeyHeader);
return response.text as unknown;
},
async deleteAllReports(roleAuthc: RoleCredentials, internalReqHeader: InternalRequestHeader) {
log.debug('ReportingAPI.deleteAllReports');

// ignores 409 errs and keeps retrying
await retry.tryForTime(5000, async () => {
await supertestWithoutAuth
.post(`/${REPORTING_DATA_STREAM_WILDCARD_WITH_LEGACY}/_delete_by_query`)
.set(internalReqHeader)
.set(roleAuthc.apiKeyHeader)
.send({ query: { match_all: {} } });
});

/*
* Ensures reports are cleaned up through the delete report API
*/
async deleteReport(
reportId: string,
roleAuthc: RoleCredentials,
internalReqHeader: InternalRequestHeader
) {
log.debug(`ReportingAPI.deleteReport ${INTERNAL_ROUTES.JOBS.DELETE_PREFIX}/${reportId}`);
const response = await supertestWithoutAuth
.delete(INTERNAL_ROUTES.JOBS.DELETE_PREFIX + `/${reportId}`)
.set(internalReqHeader)
.set(roleAuthc.apiKeyHeader)
.set('kbn-xsrf', 'xxx')
.expect(200);
return response.text as unknown;
},
};
}
1 change: 0 additions & 1 deletion x-pack/test_serverless/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
"@kbn/dataset-quality-plugin",
"@kbn/alerting-comparators",
"@kbn/search-types",
"@kbn/reporting-server",
"@kbn/config-schema",
"@kbn/features-plugin",
"@kbn/observability-ai-assistant-plugin",
Expand Down

0 comments on commit dfee928

Please sign in to comment.