autometrics-rs icon indicating copy to clipboard operation
autometrics-rs copied to clipboard

Enable `Objective` details to be loaded from environment variables

Open emschwartz opened this issue 2 years ago • 5 comments

A number of people have described use cases where the details of an SLO may be different for different instances of the same service. For example, a service that has specific instances for each geographical region, where each has its own SLO.

The main thing we would need to change in order to support this would be to make the Objective details use Strings instead of &'static strs and the methods would need to be non-const (this would be a breaking change). Then, you'd be able to define an objective like this:

use std::{env, cell::LazyCell};

static OBJECTIVE: LazyCell<Objective> = LazyCell::new(|| {
    Objective::new(env::var("OBJECTIVE_NAME").unwrap())
        .success_rate(env::var("OBJECTIVE_PERCENTILE").unwrap())
});

We could potentially add helper functions for loading the values from environment variables, or we could just show how to do this in the docs.

emschwartz avatar Jun 20 '23 07:06 emschwartz

Actually, this is going to be trickier than I thought.

You can set up the Objective like this:

use autometrics::{autometrics, objectives::*};
use once_cell::sync::Lazy;
use std::env;

static OBJECTIVE_NAME: Lazy<String> = Lazy::new(|| env::var("OBJECTIVE_NAME").unwrap_or("my-objective".to_string()));
static OBJECTIVE_PERCENTILE: Lazy<ObjectivePercentile> = Lazy::new(|| match env::var("OBJECTIVE_PERCENTILE").as_deref() {
  Ok("P90") => ObjectivePercentile::P90,
  Ok("P95") => ObjectivePercentile::P95,
  Ok("P99") => ObjectivePercentile::P99,
  Ok("P99_9") => ObjectivePercentile::P99_9,
  _ => ObjectivePercentile::P95,
});
static OBJECTIVE: Lazy<Objective> = Lazy::new(|| Objective::new(&OBJECTIVE_NAME)
    .success_rate(*OBJECTIVE_PERCENTILE));

#[autometrics(objective = OBJECTIVE)]
pub fn api_handler() {
   // ...
}

The problem is that the type that we're expecting is an Objective, not a Lazy<Objective>. We could switch the type it expects to use something that Derefs into an Objective or pass *Objective. However, in this code, we're using the Objective to create a static and Lazy doesn't implement ~const Deref (as I just learned from the compiler).

I don't know if there's an easy way to get around this...

emschwartz avatar Jul 13 '23 09:07 emschwartz

This problem I described above is the same as described in https://github.com/matklad/once_cell/issues/244

emschwartz avatar Aug 01 '23 08:08 emschwartz

Would it be an option to extend Objective with success_rate_env_var() and latency_env_var() methods? Then it is up to the macro using the objective to resolve the env vars to actual values, which feels a little less clean, but might do the trick? Not sure if you'd run into const issues in another place though...

arendjr avatar Aug 01 '23 09:08 arendjr

Yeah I think something like that might be a good solution.

We could even have a specific const method that returns some values and just returns empty things when you've declared the Objective using those _env_var functions...

emschwartz avatar Aug 01 '23 09:08 emschwartz

Please be mindful with those env variables.

I've just filled an issue because the crate doesn't compile due to another host env variable that is inaccessible from a hermetic build. I don't know the best path forward fro this issue, but in all fairness some constants combined with the option type and custom constructors may yield a deterministic result regardless of env variables.

marvin-hansen avatar May 10 '24 08:05 marvin-hansen