divviup / janus

Experimental implementation of the Distributed Aggregation Protocol (DAP) specification.
Mozilla Public License 2.0
53 stars 15 forks source link

`janus_server`: configuration #20

Closed tgeoghegan closed 2 years ago

tgeoghegan commented 2 years ago

We need to decide how we will provide configuration values to janus_server workers.

tgeoghegan commented 2 years ago

In prio-server, workflow-manager and facilitator are driven by command line arguments set in Terraform, and we use the Go stdlib flags library or the clap crate, respectively, to parse arguments.

Relevant design doc section: https://docs.google.com/document/d/13CwR1w6vXFmfpVUX8xcvXripfqRB8JQOm-MiSixV38E/edit#heading=h.r8uf4mihx0d7

branlwyd commented 2 years ago

One problem (IMO) with binaries in prio-server is that there were quite a large number of flags, some of which only applied in certain circumstances, some with logical structure/relation to other flags (that ends up flattened out by the flat nature of the flag namespace) -- this made things very complex. Our flag-parsing code also became quite complex due to this.

I'd suggest we specify a --config flag that specifies a file in a structured ML (eg YAML), containing all of the required configuration. Janus would then parse this file into a structure we define (likely using a derived serde implementation), and we could use that structure & its substructures as inputs into constructors, etc. Practically, for deployments we'd build the YAML structure in some terraform and include it in a volume to make constructing the config easy & make it available to the service.

One downside to this approach is that the flags are less observable; I think we would be OK if we logged the structure after parsing it. Another downside is we don't get some of clap's nice features like mutual-exclusion on certain flags, etc -- we can express some of this via serde validation, but some things (like mutual exclusion) I think would require writing manual validation code. IMO, the upside of the additional structure we get is worth the few lines of validation code.

divergentdave commented 2 years ago

I agree that YAML files would be a good route for configuration. The natural way to mount them in would be via a Kubernetes ConfigMap (or secret). We can create a ConfigMap and provide its contents from Terraform, and use yamlencode() to avoid having to do our own escaping with string templating.

One downside of the environment variable/command line flag approach with prio-server is that the "minimum viable invocation" requires a very long command line. Configuration files would be easier to save and reuse, and make for better developer ergonomics.

Using configuration files will also unblock live reloading of configuration, if we ever wish to pursue it. Kubernetes does the right thing with ConfigMap/secret volumes, the kubelet polls a watch endpoint, and when there's an update to a mounted ConfigMap/secret, it atomically swaps it in with a rename. That would just leave it to the application code to "poll" the file or use inotify.

tgeoghegan commented 2 years ago

I don't know about YAML. The language is too rich and flexible and that leads to a lot of surprises when parsing it (see https://noyaml.com/ for a good summary). My favourite, besides the infamous Norway == false thing, is that any JSON document is also valid YAML. Why?! Those parsing ambiguities get even harder to deal with if we intend to render YAML from HCL.

JSON is safer but it's nice to have comments in config files, so I vote for TOML.

branlwyd commented 2 years ago

I'm definitely not married to YAML, but I think one thing we should keep in mind is that we want to generate these config files somehow based on Terraform configuration. Both JSON & YAML are well-supported in Terraform (with the jsonencode, yamlencode built-in functions).

That doesn't mean we can't use another format, but we should factor in the cost of generating the configs. Sadly, TOML isn't supported by Terraform natively & I can't find any kind of third-party support, either. :-/