envoyproxy / envoyproxy/envoy

Static registration of runtime flags

Open
#10,916 2 comments 0 reactions 0 assignees View on GitHub
area/runtime design proposal help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

Currently runtime flags are checked by passing an arbitrary strings to the runtime snapshot in code, which makes discovering runtime values hard, usually relying on documentation that might not be easily discoverable (or non-existent).

If we provided a static registration mechanism for runtime flags, we could provide a CLI option that would dump all runtime flags, making it easy to search through available runtime flags. This could also potentially be sent up as part of the discovery request for RTDS, informing the server what flags are available.

This could look something like this

```
Runtime::RegisterFlag lightstep_min_flush_spans_{"tracing.lightstep.min_flush_spans", DefaultMinFlushSpans};
```

with evaluation looking like
```
uint64_t value = lightstep_min_flush_spans_.evaluate(runtime_.snapshot());
```

For keys that are parametric, we could register them similarly:

```
Runtime::RegisterParametricFlag parametric_fault_delay_duration_{"fault.http.{}.delay.fixed_duration_ms"};
```

with the difference being that upon evaluation the real name has to be resolved. A few options here:

Direct parametric expansion:
```
// Calls fmt::format with the original string + provided args.
parametric_fault_delay_duration_.evaluateWithFormatArgs(runtime_.snapshot(), downstream_cluster_);
```

Provide the evaluated string: this avoids the performance cost of re-evaluating the format string on every invocation, but we lose out on the guarantee that the flags match the registered format string:
```
parametric_fault_delay_duration_.evaluateWithKey(runtime_.snapshot(), fault_delay_duration_key_);
```

A third option would be to be able to expand a registered parametric flag into a regular flag:
```
cluster_fault_duration_ = parametric_fault_delay_duration_.expandParameters(cluster_);
```
which seems to be the best of both worlds, only sacrificing some potential memory of having to allocate a few more objects.

To further add self-documentation, these flags could require a description to be registered:
```
Runtime::RegisterParametricFlag parametric_fault_delay_duration_{"fault.http.{}.delay.fixed_duration_ms", "Allows overriding the fixed duration injected by the fault filter for a specific cluster."};
```

Default flag values could be handled in two ways (enforced through overloads and various flag types): either given statically during registration, or provided during evaluation.

With all this, I imagine being able to invoke envoy with `--runtime-flags` and see something like

```
fault.http.{}.delay.fixed_duration_ms | Defaults to a dynamic value. | Overrides the fixed duration injected by the fault filter for a specific cluster.
tracing.lightstep.min_flush_spans | Defaults to 200. | Controls the minimum number of spans before triggering a flush.

```

There's a balance to be struck here between ease of use (to make it easy for people to add flags) and powering flag discoverability, so I'm open for suggestions here. I can imagine further categorization of deprecation flags, feature toggles, parameter knobs, etc., which could be powered by this same registration mechanism.

@mattklein123 @alyssawilk

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.