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

[Question] No env and prod converges to the same value. Invitation to human error? #25

Open
linearregression opened this issue Jan 29, 2016 · 5 comments
Labels

Comments

@linearregression
Copy link

When people (or mistake in the script) forget to put value for env, it will output the same
value as if "prod" has been put in. It looks like it hides a mistake, and makes is a hard to find error.

Suggest to make env a required settings, not optional, during init for least surprises.

File: elixometer/lib/elixometer.ex

def name_to_exometer(metric_type, name) when is_bitstring(name) do
    config = Application.get_all_env(:elixometer)
    prefix = config[:metric_prefix] || "elixometer"
    base_name = case config[:env] do
                  nil -> "#{prefix}.#{metric_type}.#{name}"    <<<<<<<<<<<<<<<<<<<<<<<
                  :prod -> "#{prefix}.#{metric_type}.#{name}"   <<<<<<<<<<<<<<<<<<<<<<
                  env -> "#{prefix}.#{env}.#{metric_type}.#{name}"
                end
@scohen
Copy link
Collaborator

scohen commented Jan 29, 2016

I think it would be bad if your application failed to boot because your metrics package wasn't configured correctly; especially if you left out the env.
This default is reasonable; it leave out an environment, and the worst possible outcome is you taint a metric with non-prod data.

@linearregression
Copy link
Author

It can go both ways. It is about what do you mean by a good state.
If it fails to boot, it will be right from the start of the whole app, not in the middle of it.
Whoever deployed it will be forced to fix it.

Right now the process owns the ets. So risk is small.
Same argument goes with if you want a default, why no a :dev or :test env. Why :prod which is more important to make sure the state is as valid and clean as you can get

@scohen
Copy link
Collaborator

scohen commented Jan 29, 2016

I think prod is more valid because that's the env where I really don't want stuff to fail.

Imagine this: deploy goes out, servers don't boot and I'm running at reduced capacity until I can roll back. Not a great scenario.

@linearregression
Copy link
Author

Then at least log a warning or something, a default is being used.
It sounds like you are referring deploy it all at once but not issue if it is under canary deployment.

@zorbash
Copy link
Contributor

zorbash commented Jun 30, 2017

I think it makes sense at least to log a warning when config[:env] is missing.
Other alternatives would be to try to guess the env from Mix.env or use something harmless like "default_env".

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

No branches or pull requests

4 participants