Skip to content

Commit 069f7fa

Browse files
chrisronlinechrisronline
committed
PR feedback
1 parent 4567cce commit 069f7fa

11 files changed

Lines changed: 45 additions & 45 deletions

‎docs/settings/task-manager-settings.asciidoc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ Task Manager runs background tasks by polling for work on an interval. You can
2828
| `xpack.task_manager.max_workers`
2929
| The maximum number of tasks that this Kibana instance will run simultaneously. Defaults to 10.
3030
Starting in 8.0, it will not be possible to set the value greater than 100.
31+
32+
| `xpack.task_manager.monitored_stats_warn_delayed_task_start_in_seconds`
33+
| The amount of seconds we allow a task to delay before print a warning server log. Defaults to 60.
3134
|===
3235

3336
[float]

‎x-pack/plugins/task_manager/server/config.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe('config validation', () => {
2020
"monitored_aggregated_stats_refresh_rate": 60000,
2121
"monitored_stats_required_freshness": 4000,
2222
"monitored_stats_running_average_window": 50,
23-
"monitored_stats_warn_drift_in_seconds": 60,
23+
"monitored_stats_warn_delayed_task_start_in_seconds": 60,
2424
"monitored_task_execution_thresholds": Object {
2525
"custom": Object {},
2626
"default": Object {
@@ -69,7 +69,7 @@ describe('config validation', () => {
6969
"monitored_aggregated_stats_refresh_rate": 60000,
7070
"monitored_stats_required_freshness": 4000,
7171
"monitored_stats_running_average_window": 50,
72-
"monitored_stats_warn_drift_in_seconds": 60,
72+
"monitored_stats_warn_delayed_task_start_in_seconds": 60,
7373
"monitored_task_execution_thresholds": Object {
7474
"custom": Object {},
7575
"default": Object {
@@ -105,7 +105,7 @@ describe('config validation', () => {
105105
"monitored_aggregated_stats_refresh_rate": 60000,
106106
"monitored_stats_required_freshness": 4000,
107107
"monitored_stats_running_average_window": 50,
108-
"monitored_stats_warn_drift_in_seconds": 60,
108+
"monitored_stats_warn_delayed_task_start_in_seconds": 60,
109109
"monitored_task_execution_thresholds": Object {
110110
"custom": Object {
111111
"alerting:always-fires": Object {

‎x-pack/plugins/task_manager/server/config.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export const DEFAULT_VERSION_CONFLICT_THRESHOLD = 80;
1818
// Refresh aggregated monitored stats at a default rate of once a minute
1919
export const DEFAULT_MONITORING_REFRESH_RATE = 60 * 1000;
2020
export const DEFAULT_MONITORING_STATS_RUNNING_AVERGAE_WINDOW = 50;
21-
export const DEFAULT_MONITORING_STATS_WARN_DRIFT_IN_SECONDS = 60;
21+
export const DEFAULT_MONITORING_STATS_WARN_DELAYED_TASK_START_IN_SECONDS = 60;
2222

2323
export const taskExecutionFailureThresholdSchema = schema.object(
2424
{
@@ -110,8 +110,9 @@ export const configSchema = schema.object(
110110
defaultValue: {},
111111
}),
112112
}),
113-
monitored_stats_warn_drift_in_seconds: schema.number({
114-
defaultValue: DEFAULT_MONITORING_STATS_WARN_DRIFT_IN_SECONDS,
113+
/* The amount of seconds we allow a task to delay before print a warning server log */
114+
monitored_stats_warn_delayed_task_start_in_seconds: schema.number({
115+
defaultValue: DEFAULT_MONITORING_STATS_WARN_DELAYED_TASK_START_IN_SECONDS,
115116
}),
116117
},
117118
{

‎x-pack/plugins/task_manager/server/integration_tests/managed_configuration.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('managed configuration', () => {
3737
version_conflict_threshold: 80,
3838
max_poll_inactivity_cycles: 10,
3939
monitored_aggregated_stats_refresh_rate: 60000,
40-
monitored_stats_warn_drift_in_seconds: 60,
40+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
4141
monitored_stats_required_freshness: 4000,
4242
monitored_stats_running_average_window: 50,
4343
request_capacity: 1000,

‎x-pack/plugins/task_manager/server/lib/log_health_metrics.test.ts‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import { logHealthMetrics } from './log_health_metrics';
1414
import { Logger } from '../../../../../src/core/server';
1515

1616
describe('logHealthMetrics', () => {
17-
it('should log as debug if there the status is OK', () => {
17+
it('should log as debug if status is OK', () => {
1818
const logger = loggingSystemMock.create().get();
1919
const config = getTaskManagerConfig({
20-
monitored_stats_warn_drift_in_seconds: 60,
20+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
2121
});
2222
const health = getMockMonitoredHealth();
2323

@@ -29,10 +29,10 @@ describe('logHealthMetrics', () => {
2929
expect(firstDebug).toMatchObject(health);
3030
});
3131

32-
it('should log as warn if there the status is Warn', () => {
32+
it('should log as warn if status is Warn', () => {
3333
const logger = loggingSystemMock.create().get();
3434
const config = getTaskManagerConfig({
35-
monitored_stats_warn_drift_in_seconds: 60,
35+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
3636
});
3737
const health = getMockMonitoredHealth({
3838
status: HealthStatus.Warning,
@@ -42,17 +42,17 @@ describe('logHealthMetrics', () => {
4242

4343
const logMessage = JSON.parse(
4444
((logger as jest.Mocked<Logger>).warn.mock.calls[0][0] as string).replace(
45-
'Latest Monitored Stats (warning status): ',
45+
'Latest Monitored Stats: ',
4646
''
4747
)
4848
);
4949
expect(logMessage).toMatchObject(health);
5050
});
5151

52-
it('should log as error if there the status is Error', () => {
52+
it('should log as error if status is Error', () => {
5353
const logger = loggingSystemMock.create().get();
5454
const config = getTaskManagerConfig({
55-
monitored_stats_warn_drift_in_seconds: 60,
55+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
5656
});
5757
const health = getMockMonitoredHealth({
5858
status: HealthStatus.Error,
@@ -62,17 +62,17 @@ describe('logHealthMetrics', () => {
6262

6363
const logMessage = JSON.parse(
6464
((logger as jest.Mocked<Logger>).error.mock.calls[0][0] as string).replace(
65-
'Latest Monitored Stats (error status): ',
65+
'Latest Monitored Stats: ',
6666
''
6767
)
6868
);
6969
expect(logMessage).toMatchObject(health);
7070
});
7171

72-
it('should log as warn if there the drift exceeds the threshold', () => {
72+
it('should log as warn if drift exceeds the threshold', () => {
7373
const logger = loggingSystemMock.create().get();
7474
const config = getTaskManagerConfig({
75-
monitored_stats_warn_drift_in_seconds: 60,
75+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
7676
});
7777
const health = getMockMonitoredHealth({
7878
stats: {
@@ -88,19 +88,23 @@ describe('logHealthMetrics', () => {
8888

8989
logHealthMetrics(health, logger, config);
9090

91-
const logMessage = JSON.parse(
92-
((logger as jest.Mocked<Logger>).warn.mock.calls[0][0] as string).replace(
93-
`Latest Monitored Stats (Detected drift of 60s): `,
91+
expect((logger as jest.Mocked<Logger>).warn.mock.calls[0][0] as string).toBe(
92+
`Detected delay task start of 60s (which exceeds configured value of 60s)`
93+
);
94+
95+
const secondMessage = JSON.parse(
96+
((logger as jest.Mocked<Logger>).warn.mock.calls[1][0] as string).replace(
97+
`Latest Monitored Stats: `,
9498
''
9599
)
96100
);
97-
expect(logMessage).toMatchObject(health);
101+
expect(secondMessage).toMatchObject(health);
98102
});
99103

100104
it('should log as debug if there are no stats', () => {
101105
const logger = loggingSystemMock.create().get();
102106
const config = getTaskManagerConfig({
103-
monitored_stats_warn_drift_in_seconds: 60,
107+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
104108
});
105109
const health = {
106110
id: '1',

‎x-pack/plugins/task_manager/server/lib/log_health_metrics.ts‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,22 @@ export function logHealthMetrics(
1616
logger: Logger,
1717
config: TaskManagerConfig
1818
) {
19-
let contextMessage;
20-
2119
let logAsWarn = monitoredHealth.status === HealthStatus.Warning;
2220
const logAsError =
2321
monitoredHealth.status === HealthStatus.Error && !isEmpty(monitoredHealth.stats);
2422
const driftInSeconds = (monitoredHealth.stats.runtime?.value.drift.p99 ?? 0) / 1000;
2523

26-
if (driftInSeconds >= config.monitored_stats_warn_drift_in_seconds) {
27-
contextMessage = `Detected drift of ${driftInSeconds}s`;
24+
if (driftInSeconds >= config.monitored_stats_warn_delayed_task_start_in_seconds) {
25+
logger.warn(
26+
`Detected delay task start of ${driftInSeconds}s (which exceeds configured value of ${config.monitored_stats_warn_delayed_task_start_in_seconds}s)`
27+
);
2828
logAsWarn = true;
2929
}
3030

3131
if (logAsError) {
32-
logger.error(
33-
`Latest Monitored Stats (${contextMessage ?? `error status`}): ${JSON.stringify(
34-
monitoredHealth
35-
)}`
36-
);
32+
logger.error(`Latest Monitored Stats: ${JSON.stringify(monitoredHealth)}`);
3733
} else if (logAsWarn) {
38-
logger.warn(
39-
`Latest Monitored Stats (${contextMessage ?? `warning status`}): ${JSON.stringify(
40-
monitoredHealth
41-
)}`
42-
);
34+
logger.warn(`Latest Monitored Stats: ${JSON.stringify(monitoredHealth)}`);
4335
} else {
4436
logger.debug(`Latest Monitored Stats: ${JSON.stringify(monitoredHealth)}`);
4537
}

‎x-pack/plugins/task_manager/server/monitoring/configuration_statistics.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('Configuration Statistics Aggregator', () => {
2323
max_poll_inactivity_cycles: 10,
2424
request_capacity: 1000,
2525
monitored_aggregated_stats_refresh_rate: 5000,
26-
monitored_stats_warn_drift_in_seconds: 60,
26+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
2727
monitored_stats_running_average_window: 50,
2828
monitored_task_execution_thresholds: {
2929
default: {

‎x-pack/plugins/task_manager/server/monitoring/monitoring_stats_stream.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ describe('createMonitoringStatsStream', () => {
2727
max_poll_inactivity_cycles: 10,
2828
request_capacity: 1000,
2929
monitored_aggregated_stats_refresh_rate: 5000,
30-
monitored_stats_warn_drift_in_seconds: 60,
30+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
3131
monitored_stats_running_average_window: 50,
3232
monitored_task_execution_thresholds: {
3333
default: {

‎x-pack/plugins/task_manager/server/plugin.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('TaskManagerPlugin', () => {
2525
max_poll_inactivity_cycles: 10,
2626
request_capacity: 1000,
2727
monitored_aggregated_stats_refresh_rate: 5000,
28-
monitored_stats_warn_drift_in_seconds: 60,
28+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
2929
monitored_stats_required_freshness: 5000,
3030
monitored_stats_running_average_window: 50,
3131
monitored_task_execution_thresholds: {
@@ -56,7 +56,7 @@ describe('TaskManagerPlugin', () => {
5656
max_poll_inactivity_cycles: 10,
5757
request_capacity: 1000,
5858
monitored_aggregated_stats_refresh_rate: 5000,
59-
monitored_stats_warn_drift_in_seconds: 60,
59+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
6060
monitored_stats_required_freshness: 5000,
6161
monitored_stats_running_average_window: 50,
6262
monitored_task_execution_thresholds: {

‎x-pack/plugins/task_manager/server/polling_lifecycle.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ describe('TaskPollingLifecycle', () => {
4545
max_poll_inactivity_cycles: 10,
4646
request_capacity: 1000,
4747
monitored_aggregated_stats_refresh_rate: 5000,
48-
monitored_stats_warn_drift_in_seconds: 60,
48+
monitored_stats_warn_delayed_task_start_in_seconds: 60,
4949
monitored_stats_required_freshness: 5000,
5050
monitored_stats_running_average_window: 50,
5151
monitored_task_execution_thresholds: {

0 commit comments

Comments
 (0)