remove global runtime: rebase runtime features on ABSL_flags

[alyssawilk]

So as part of
envoyproxy/envoy-mobile#2003 we need to fix the runtime singleton problem.

Actual runtime is no problem but having runtimeFeatureEnabled point at an actual runtime is basically impossible to fix.

I propose replacing the runtime guards with ABSL flags: still setting them via RTDS but being able to access them directly as flags. This solves both a bunch of test issues where flags were accessed before/after the envoy server comes up and down, and allows for removing the singleton issue for envoy mobile.

E-M won't support different values for runtime across different instances, but we can't do that without wiring some server-specific handle to all the locations where we use runtime which would make adding new values prohibitively cumbersome anyway.

[alyssawilk]

cc @envoyproxy/envoy-maintainers for thoughts

[jmarantz]

@krajshiva FYI

[yanavlasov]

How is this going to work together with TCLAP?

[alyssawilk]

will test to ensure no change but I don't intend to use the command line parsing utilities, just the in-memory classes/macros.

[mattklein123]

Aren't gflags process singletons? How does that fix the problem? Or it's OK to have the feature enabled stuff be global?

[yanavlasov]

I see, if we do not intend to set them through command line, then there are no concerns. I think one other concern is that ABSL flags use synchronization. And I think Envoy's runtime has per worker snapshots. This may impact performance if flag is accessed in a hot path. Even latching it per connection may lead to contention.

[alyssawilk]

@mattklein123 they're functionally thread-safe globals. the problem isn't that they're global (we can't avoid that if you want to allow runtime access from any point in the code) the problem is they're globals with lifetime issues. that if you have Envoy A and Envoy B, and the "single singleton pointer" for runtime points at A's runtime, and then A gets deleted just as B is doing a runtime flag lookup, you crash.
If we actually make our globals global, then you cant have 2 instances with different values, but you avoid lifetime and thread safety issues.

[mattklein123]

Yup OK that seems fine for those flags. What will be the semantic if the values are set from multiple runtimes? I assume last one wins? This is an edge case and I think anything is fine but we should decide/document the behavior.

[alyssawilk]

exactly that. I planned on adding docs to E-M for running concurrent Envoy's. Could also doc up in runtime but I can't imagine it being a good idea for anyone to run multiple server side proxies simultaneously so I'd rather not mention the possibility upstream :-P

[jmarantz]

You've probably explored this already, but could we plumb access to the instance-specific Runtime object everywhere that's needed via the already-laid conduit that is Api::Api?

[alyssawilk]

nope. most of the value of runtime is folks can effortlessly guard refactors. If they have to plumb through 20 layers of API, they're not going to do it, and then folks start making high risk functional changes in the clear.
If you look at where runtime is used, API is often not available, which is why the decision was made to allow it to be global several years back.

[alyssawilk]

Ok I have a draft up at #19880 for the curious. Still only draft as I have to tear down the singleton access and figure out what to do with getInteger. I'm inclined to special case the existing cases, see if we can deprecate them entirely, and say it shouldn't be used.
Also I'm wondering if folks have opinions on if we should encourage folks to use flag directly in future to save the hash map look up

[alyssawilk]

Turns out flags was the least annoying part of this.
So as Matt predicted, once we had the singleton folks used it in unexpected ways. We've got some stats on deprecated config (removing: #19906) global connection enforcement (plumbing runtime: #19907), quic enablement (will plumb), and then regex size stats (see below)

I'm not sure what to do with regex: I'm inclined to remove the stats, and then deprecate the config (to see if anyone is using it, and if they are leave it as an int-flag) but would appreciate others' thoughts for if their intuition is that folks need the stats and I need to plumb runtime to everywhere we create regex @mattklein123 @htuch
source here: https://github.com/envoyproxy/envoy/blob/main/source/common/common/regex.cc#L24

[jmarantz]

Hi -- I can't remember what specifically I was involved in with regard to the runtime tweak and histogram tracking regex program size in regex.cc, but I was at least aware of it :)

TL;DR I think it'd be fine to just rip all that out and leave in the runtime int setting, because I think no one is looking at the histogram, and its access of rootScope() may complicate things for you.

Is the access to rootScope() the thing that you were not expecting?

[htuch]

The notion of getRootScope() in the runtime seems like mission creep.

[mattklein123]

Yeah I would just rip out the regex stuff.