grafana / agent

Vendor-neutral programmable observability pipelines.
https://grafana.com/docs/agent/
Apache License 2.0
1.59k stars 486 forks source link

Proposal: Deprecation plan for server settings #1336

Closed rfratto closed 2 years ago

rfratto commented 2 years ago

Background

581 suggests that the implementation of /-/reload can be made simpler by not supporting dynamic reloading for certain settings. There are two ways of doing this:

  1. Break guarantees that the entire YAML file will always be reloadable and choose to make a subset of settings static. Changing the value for these static settings would cause the reload call to fail.
  2. Keep guarantees that the entire YAML file will always be reloadable and move the subset of static settings out of the YAML file and into configuration flags.

I would prefer option 2 as it is easier for the user to understand.

There are two pieces of code which introduce the most complexity with the /-/reload implementation:

  1. The server block
  2. The scraping service

The proposed metrics-next rewrite does not support dynamically changing cluster settings and avoids this problem. Since metrics-next will eventually deprecate the scraping service, it will be considered out of scope for this proposal.

This issue proposes a general deprecation plan to allow us to remove support for dynamically reloading all of the server settings.

Goals

  1. Remove need to support dynamically reloading the entire HTTP server
  2. Implement a gradual deprecation strategy

Proposal

The deprecation will be done in chunks, where each chunk of work is split up across every 2 minor releases:

  1. Next minor release (v0.23.0)
    1. Temporarily disallow all dynamic changes to the server block. This is done as a stopgap to facilitate the development of #1140, which is complicated by the possibility of the gRPC server being changeable.
  2. +2 minor releases (v0.25.0)
    1. Deprecate fields from the server block that we consider static.
    2. Introduce flags for just static server options (i.e., dynamic settings shouldn't have a flag equivalent)
    3. Change all of our example configuration to prefer the flags over the deprecated YAML options
  3. +2 minor releases (v0.27.0)
    1. Remove deprecated static YAML options so just the flags remain
    2. Re-allow dynamic updating of the remaining non-deprecated YAML settings. This may require breaking away from the weaveworks/common dependency and using a similar implementation where specific fields can be updated.

We typically do one release per month, meaning this deprecation plan will finally complete in ~5 months by July 2022.

List of fields

The following is the full list of current options split up into a proposed set of "static" and "dynamic."

Static settings:

Dynamic settings:

rfratto commented 2 years ago

My immediate concern with this is how the operator will be impacted. I'm thinking that maybe we should combine step 1 and 2 and have the following steps instead:

  1. First minor release
    1. Temporarily disallow all dynamic changes to the server block.
    2. Deprecate fields from the server block that we consider static.
    3. Introduce flags for static server options
    4. Change operator to set relevant fields as flags instead of in the config file
    5. Change all of our example configuration to prefer the flags over deprecated YAML options
  2. +2 minor releases
    1. Remove deprecated static YAML options so just the flags remain
    2. Re-enable dynamic updating of the leftover non-deprecated YAML settings.