Skip to content

Commit

Permalink
[Reporting] Track retryAt time on the task instance (#174409)
Browse files Browse the repository at this point in the history
## Summary

Initial part of #131852

This PR moves towards auto-calculating the maximum timeouts calculated
based on the timing context provided by the running task instance.

### Other changes
* Added an optional logger parameter to the `getScreenshots` function.
When a logger is provided, the logs created by the screenshot plugin
will have the contextual tags added by the calling code.
  * Before
<img width="1198" alt="image"
src="https://github.com/elastic/kibana/assets/908371/f68a102e-6af2-4863-aedb-52f1e4a099d8">
  * After
<img width="1200" alt="image"
src="https://github.com/elastic/kibana/assets/908371/2dd4c947-ffa6-4cb3-b8a2-22893f49ddb7">

* Fixed an unreported bug where browser timezone was not utilized in PNG
reports.

### Checklist

Delete any items that are not applicable to this PR.

- [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
  • Loading branch information
tsullivan authored Jan 16, 2024
1 parent f288919 commit decec37
Show file tree
Hide file tree
Showing 23 changed files with 282 additions and 107 deletions.
25 changes: 24 additions & 1 deletion packages/kbn-generate-csv/src/generate_csv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import { IScopedSearchClient } from '@kbn/data-plugin/server';
import { dataPluginMock } from '@kbn/data-plugin/server/mocks';
import { FieldFormatsRegistry } from '@kbn/field-formats-plugin/common';
import { CancellationToken } from '@kbn/reporting-common';
import type { ReportingConfigType } from '@kbn/reporting-server';
import { JobParamsCSV } from '@kbn/reporting-export-types-csv-common';
import type { ReportingConfigType } from '@kbn/reporting-server';
import {
UI_SETTINGS_CSV_QUOTE_VALUES,
UI_SETTINGS_CSV_SEPARATOR,
Expand All @@ -37,6 +37,7 @@ import { CsvGenerator } from './generate_csv';
const createMockJob = (baseObj: any = {}): JobParamsCSV => ({
...baseObj,
});
const mockTaskInstanceFields = { startedAt: null, retryAt: null };

describe('CsvGenerator', () => {
let mockEsClient: IScopedClusterClient;
Expand Down Expand Up @@ -145,6 +146,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -180,6 +182,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -219,6 +222,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -269,6 +273,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -328,6 +333,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -400,6 +406,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ searchSource: {}, columns: [] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -440,6 +447,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ searchSource: {}, columns: ['_id', 'sku'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -487,6 +495,7 @@ describe('CsvGenerator', () => {
columns: ['_id', '_index', 'date', 'message'],
}),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -541,6 +550,7 @@ describe('CsvGenerator', () => {
},
}),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -583,6 +593,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ searchSource: {}, columns: ['product', 'category'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -621,6 +632,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ searchSource: {}, columns: ['_id', '_index', 'product', 'category'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -659,6 +671,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ searchSource: {}, columns: [] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -699,6 +712,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -737,6 +751,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', TEST_FORMULA] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -784,6 +799,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -817,6 +833,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({}),
mockConfig,
mockTaskInstanceFields,
{ es: mockEsClient, data: mockDataClient, uiSettings: uiSettingsClient },
{
searchSourceStart: mockSearchSourceService,
Expand Down Expand Up @@ -872,6 +889,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -923,6 +941,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -960,6 +979,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -992,6 +1012,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -1041,6 +1062,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down Expand Up @@ -1100,6 +1122,7 @@ describe('CsvGenerator', () => {
const generateCsv = new CsvGenerator(
createMockJob({ columns: ['date', 'ip', 'message'] }),
mockConfig,
mockTaskInstanceFields,
{
es: mockEsClient,
data: mockDataClient,
Expand Down
46 changes: 26 additions & 20 deletions packages/kbn-generate-csv/src/generate_csv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import moment from 'moment';
import { lastValueFrom } from 'rxjs';
import type { Writable } from 'stream';

Expand All @@ -31,7 +32,7 @@ import {
ReportingError,
byteSizeValueToNumber,
} from '@kbn/reporting-common';
import type { TaskRunResult } from '@kbn/reporting-common/types';
import type { TaskInstanceFields, TaskRunResult } from '@kbn/reporting-common/types';
import type { ReportingConfigType } from '@kbn/reporting-server';

import { CONTENT_TYPE_CSV } from './constants';
Expand Down Expand Up @@ -59,6 +60,7 @@ export class CsvGenerator {
constructor(
private job: Omit<JobParamsCSV, 'version'>,
private config: ReportingConfigType['csv'],
private taskInstanceFields: TaskInstanceFields,
private clients: Clients,
private dependencies: Dependencies,
private cancellationToken: CancellationToken,
Expand Down Expand Up @@ -346,16 +348,19 @@ export class CsvGenerator {
}

public async generateData(): Promise<TaskRunResult> {
const logger = this.logger;
const [settings, searchSource] = await Promise.all([
getExportSettings(
this.clients.uiSettings,
this.config,
this.job.browserTimezone,
this.logger
),
getExportSettings(this.clients.uiSettings, this.config, this.job.browserTimezone, logger),
this.dependencies.searchSourceStart.create(this.job.searchSource),
]);
let reportingError: undefined | ReportingError;

const { startedAt, retryAt } = this.taskInstanceFields;
if (startedAt) {
this.logger.debug(
`Task started at: ${startedAt && moment(startedAt).format()}.` +
` Can run until: ${retryAt && moment(retryAt).format()}`
);
}

const index = searchSource.getField('index');

Expand All @@ -372,12 +377,13 @@ export class CsvGenerator {
let totalRecords: number | undefined;
let searchAfter: estypes.SortResults | undefined;

let reportingError: undefined | ReportingError;
let pitId = await this.openPointInTime(indexPatternTitle, settings);

// apply timezone from the job to all date field formatters
try {
index.fields.getByType('date').forEach(({ name }) => {
this.logger.debug(`Setting timezone on ${name}`);
logger.debug(`Setting timezone on ${name}`);
const format: FieldFormatConfig = {
...index.fieldFormatMap[name],
id: index.fieldFormatMap[name]?.id || 'date', // allow id: date_nanos
Expand All @@ -389,7 +395,7 @@ export class CsvGenerator {
index.setFieldFormat(name, format);
});
} catch (err) {
this.logger.error(err);
logger.error(err);
}

const columns = new Set<string>(this.job.columns ?? []);
Expand All @@ -403,7 +409,7 @@ export class CsvGenerator {

const results = await this.doSearch(searchSource, settings, searchAfter);
if (!results) {
this.logger.warn(`Search results are undefined!`);
logger.warn(`Search results are undefined!`);
break;
}

Expand All @@ -423,22 +429,22 @@ export class CsvGenerator {
// Update last sort results for next query. PIT is used, so the sort results
// automatically include _shard_doc as a tiebreaker
searchAfter = hits[hits.length - 1]?.sort as estypes.SortResults | undefined;
this.logger.debug(`Received search_after: [${searchAfter}]`);
logger.debug(`Received search_after: [${searchAfter}]`);

// check for shard failures, log them and add a warning if found
const { _shards: shards } = results;
if (shards.failures) {
shards.failures.forEach(({ reason }) => {
warnings.push(`Shard failure: ${JSON.stringify(reason)}`);
this.logger.warn(JSON.stringify(reason));
logger.warn(JSON.stringify(reason));
});
}

let table: Datatable | undefined;
try {
table = tabifyDocs(results, index, { shallow: true, includeIgnoredValues: true });
} catch (err) {
this.logger.error(err);
logger.error(err);
warnings.push(i18nTexts.unknownError(err?.message ?? err));
}

Expand Down Expand Up @@ -472,7 +478,7 @@ export class CsvGenerator {
warnings.push(i18nTexts.escapedFormulaValuesMessage);
}
} catch (err) {
this.logger.error(err);
logger.error(err);
if (err instanceof esErrors.ResponseError) {
if ([401, 403].includes(err.statusCode ?? 0)) {
reportingError = new AuthenticationExpiredError();
Expand All @@ -487,20 +493,20 @@ export class CsvGenerator {

try {
if (pitId) {
this.logger.debug(`Closing PIT ${this.formatPit(pitId)}`);
logger.debug(`Closing PIT ${this.formatPit(pitId)}`);
await this.clients.es.asCurrentUser.closePointInTime({ body: { id: pitId } });
} else {
this.logger.warn(`No PIT ID to clear!`);
logger.warn(`No PIT ID to clear!`);
}
} catch (err) {
this.logger.error(err);
logger.error(err);
warnings.push(i18nTexts.csvUnableToClosePit());
}

this.logger.info(`Finished generating. Row count: ${this.csvRowCount}.`);
logger.info(`Finished generating. Row count: ${this.csvRowCount}.`);

if (!this.maxSizeReached && this.csvRowCount !== totalRecords) {
this.logger.warn(
logger.warn(
`ES scroll returned fewer total hits than expected! ` +
`Search result total hits: ${totalRecords}. Row count: ${this.csvRowCount}`
);
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-reporting/common/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@
"@kbn/utility-types",
"@kbn/screenshotting-plugin",
"@kbn/i18n",
"@kbn/task-manager-plugin",
]
}
6 changes: 6 additions & 0 deletions packages/kbn-reporting/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import type { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server';
import type {
LayoutParams,
PerformanceMetrics as ScreenshotMetrics,
Expand Down Expand Up @@ -82,6 +83,11 @@ export interface BasePayload extends BaseParams {
isDeprecated?: boolean;
}

/**
* Timestamp metrics about the task lifecycle
*/
export type TaskInstanceFields = Pick<ConcreteTaskInstance, 'startedAt' | 'retryAt'>;

export type JobId = string;

/**
Expand Down
Loading

0 comments on commit decec37

Please sign in to comment.