Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Objective details to be loaded from environment variables #115

Open
emschwartz opened this issue Jun 20, 2023 · 5 comments
Open

Enable Objective details to be loaded from environment variables #115

emschwartz opened this issue Jun 20, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@emschwartz
Copy link
Contributor

emschwartz commented Jun 20, 2023

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 emschwartz added enhancement New feature or request good first issue Good for newcomers labels Jun 20, 2023
@emschwartz
Copy link
Contributor Author

emschwartz commented Jul 13, 2023

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 emschwartz removed the good first issue Good for newcomers label Jul 13, 2023
@emschwartz
Copy link
Contributor Author

This problem I described above is the same as described in matklad/once_cell#244

@arendjr
Copy link

arendjr commented Aug 1, 2023

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...

@emschwartz
Copy link
Contributor Author

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...

@marvin-hansen
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants