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

Allow-list Task Manager configuration in cloud #56478

Closed
gmmorris opened this issue Jan 31, 2020 · 9 comments
Closed

Allow-list Task Manager configuration in cloud #56478

gmmorris opened this issue Jan 31, 2020 · 9 comments
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@gmmorris
Copy link
Contributor

gmmorris commented Jan 31, 2020

In order to support tweaking Task Manager's behaviour in cloud, we need to allow some config values.

We definitely need to allow-list:

  1. task_manager.max_workers

We should consider allow-listing:

  1. poll_interval
@gmmorris gmmorris added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0 labels Jan 31, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@pmuellr
Copy link
Member

pmuellr commented Feb 1, 2020

It probably makes sense to set hard-coded limits on these, in a separate PR. Reason: whitelisting these will be advertising them to some extent, and without limits, I feel like we're shipping a foot-gun.

Just opened issue #56573 for that, didn't immediately find an existing open issue.

@gmmorris
Copy link
Contributor Author

gmmorris commented Feb 3, 2020

Yup, I was going to do it as part of this issue (2 PRs in one issue) but fair enough, as I didn't actually document that 😬

@mikecote
Copy link
Contributor

mikecote commented May 6, 2020

Task should wait and see if new configuration variables are created by #65552.

@kobelb kobelb changed the title Whitelist Task Manager configuration in cloud Allowlist Task Manager configuration in cloud Oct 21, 2020
@kobelb kobelb changed the title Allowlist Task Manager configuration in cloud Allow-list Task Manager configuration in cloud Oct 21, 2020
@mikecote mikecote mentioned this issue Oct 26, 2020
36 tasks
@ymao1 ymao1 self-assigned this Oct 27, 2020
@ymao1
Copy link
Contributor

ymao1 commented Oct 28, 2020

@elastic/kibana-alerting-services

It was mentioned that there might be additional task manager configs that we would want exposed in the cloud. Do we have an idea of what they are so they can be rolled into this issue?

@mikecote
Copy link
Contributor

++ there's no list of what other configs should be included but it's worth taking a look through our plugins (alerts, actions, event log, task manager, etc) to see if there's anything else you think should be allow-listed on cloud. You can summarize that list in this thread and if you're not sure on one of them, you can add it with a question mark or something.

@ymao1
Copy link
Contributor

ymao1 commented Oct 28, 2020

These are the configurations I've found in our plugins:
xpack.actions.enabled
xpack.actions.allowedHosts --> already in cloud config
xpack.actions.enabledActionTypes --> already in cloud config
xpack.actions.preconfigured
xpack.actions.proxyUrl
xpack.actions.proxyHeaders
xpack.actions.proxyRejectUnauthorizedCertificates
xpack.actions.rejectUnauthorized
xpack.event_log.enabled
xpack.event_log.logEntries
xpack.event_log.indexEntries
xpack.task_manager.enabled
xpack.task_manager.max_attempts
xpack.task_manager.poll_interval --> adding to cloud config in this issue
xpack.task_manager.max_poll_inactivity_cycles
xpack.task_manager.request_capacity
xpack.task_manager.index
xpack.task_manager.max_workers --> adding to cloud config in this issue
xpack.task_manager.monitored_stats_required_freshness
xpack.task_manager.monitored_aggregated_stats_refresh_rate
xpack.task_manager.monitored_stats_running_average_window
xpack.task_manager.monitored_task_execution_thresholds

According to @gmmorris comment here, we may want to expose xpack.task_manager.monitored_task_execution_thresholds

Do we need any of the xpack.actions.proxy* configs in the cloud.

Thoughts?

@pmuellr
Copy link
Member

pmuellr commented Oct 28, 2020

I don't think any of the xpack.event_log.* configs make sense to enable for cloud. At this point, they're really dev-time settings.

So alerts has no config setting!? (I looked, didn't see any) Awesome!

No opinion on TM settings, other than I feel like maybe exposing the bare minimum right now would be good - poll_interval, max_workers and probably the monitored_* ones. Per #56478 (comment), I feel like we want to make sure we have some max's set for the TM ones, which would be here:

export const configSchema = schema.object(
{
enabled: schema.boolean({ defaultValue: true }),
/* The maximum number of times a task will be attempted before being abandoned as failed */
max_attempts: schema.number({
defaultValue: 3,
min: 1,
}),
/* How often, in milliseconds, the task manager will look for more work. */
poll_interval: schema.number({
defaultValue: DEFAULT_POLL_INTERVAL,
min: 100,
}),
/* How many poll interval cycles can work take before it's timed out. */
max_poll_inactivity_cycles: schema.number({
defaultValue: DEFAULT_MAX_POLL_INACTIVITY_CYCLES,
min: 1,
}),
/* How many requests can Task Manager buffer before it rejects new requests. */
request_capacity: schema.number({
// a nice round contrived number, feel free to change as we learn how it behaves
defaultValue: 1000,
min: 1,
}),
/* The name of the index used to store task information. */
index: schema.string({
defaultValue: '.kibana_task_manager',
validate: (val) => {
if (val.toLowerCase() === '.tasks') {
return `"${val}" is an invalid Kibana Task Manager index, as it is already in use by the ElasticSearch Tasks Manager`;
}
},
}),
/* The maximum number of tasks that this Kibana instance will run simultaneously. */
max_workers: schema.number({
defaultValue: DEFAULT_MAX_WORKERS,
// disable the task manager rather than trying to specify it with 0 workers
min: 1,
}),
/* The rate at which we emit fresh monitored stats. By default we'll use the poll_interval (+ a slight buffer) */
monitored_stats_required_freshness: schema.number({
defaultValue: (config?: unknown) =>
((config as { poll_interval: number })?.poll_interval ?? DEFAULT_POLL_INTERVAL) + 1000,
min: 100,
}),
/* The rate at which we refresh monitored stats that require aggregation queries against ES. */
monitored_aggregated_stats_refresh_rate: schema.number({
defaultValue: DEFAULT_MONITORING_REFRESH_RATE,
/* don't run monitored stat aggregations any faster than once every 5 seconds */
min: 5000,
}),
/* The size of the running average window for monitored stats. */
monitored_stats_running_average_window: schema.number({
defaultValue: DEFAULT_MONITORING_STATS_RUNNING_AVERGAE_WINDOW,
max: 100,
min: 10,
}),
/* Task Execution result warn & error thresholds. */
monitored_task_execution_thresholds: schema.object({
default: taskExecutionFailureThresholdSchema,
custom: schema.recordOf(schema.string(), taskExecutionFailureThresholdSchema, {
defaultValue: {},
}),
}),
},

Not opposed to doing it in this PR, since I think we can just add the additional schema options, but we will have to make a decision on the numbers!

For actions, I think the following:

  • xpack.actions.proxyUrl
  • xpack.actions.proxyHeaders
  • xpack.actions.proxyRejectUnauthorizedCertificates
  • xpack.actions.rejectUnauthorized

I'd love to allowList xpack.actions.preconfigured (see #73073), but I believe @peterschretlen has been thinking these preconfigured actions are more like "system installed" actions, hence hiding some of the settings and what-not. I know the previous concern I had was not showing config for these in the UI, and I can still live with that, but wondering what you think (Peter) about allowListing these for cloud?

@ymao1
Copy link
Contributor

ymao1 commented Oct 30, 2020

To summarize the comments I've seen so far, I will be adding:

xpack.task_manager.monitored_task_execution_thresholds
xpack.actions.proxyUrl
xpack.actions.proxyHeaders
xpack.actions.proxyRejectUnauthorizedCertificates
xpack.actions.rejectUnauthorized

@mikecote mikecote removed the blocked label Nov 2, 2020
@ymao1 ymao1 closed this as completed Nov 10, 2020
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

No branches or pull requests

6 participants