Add enabled config option replacing active

[estolfo]

Deprecate the active config option and replace it with enabled.

#623

[bmorelli25]

This is a dynamic config, right? If so, let's add a dynamic badge like we've done for the other settings: #760

Suggested change

	[options="header"]
	<<dynamic-configuration, image:./images/dynamic-config.svg[] >>
	[options="header"]

[estolfo]

No, only recording is dynamic.
It says here about the enabled configuration:

This can only be set locally to the agent (e.g. in code, config file, environment variable), and is interpreted once at agent initialisation time; once disabled, disabled for the process's lifetime.

[matschaffer]

Will config.elastic_apm.enabled be supported as an attr writer?

Was thinking it'd be nice to make it dynamic based on the presence of the token & url env vars.

e.g., Safecast/safecastapi#670 (review)

[estolfo]

According to the definitions here only the recording option is dynamic. Can you use that instead?
If you want to enabled/disable the agent based on the presence of token and url env vars, can't you interpret that before starting the agent and set ELASTIC_APM_ENABLED? Why does the enabled setting have to be dynamic?

[matschaffer]

As mentioned in Safecast/safecastapi#670 (comment) having the gem installed without env vars set (eg., for local dev) led to a lot of log noise.

My idea was to have it always installed and in the gemfile but only active if we provide the keys (as we do in deployed envs)

[matschaffer]

I guess we could also move the gem to a production group in the gemfile. Any idea on what’s “usual”?

For new relic we have it available and the token is configured via env var. if that’s nil the newrelic gem doesn’t log any errors. The we can set the key in deployed envs to get reporting. Would be nice to have a similar flow with elastic apm.

[estolfo]

It sounds like what you've proposed-- putting the gem in the production group in the Gemfile-- would resolve this. As mention in this comment it sounds like there's no need to instrument the app in an environment other than production, so that solution seems to make the most sense.

Alternatively, you could set the elastic apm log level to FATAL via an env variable if you want to avoid the error messages when developing locally.

[estolfo]

Hi @matschaffer I just realized that maybe you were asking to change the enabled attribute on the agent config object. Were you asking to do something like the following before the agent is started:

config.enabled = false

or were you looking to change the enabled value while the agent is running?

[matschaffer]

I'm not clear on where agent start happens in the rails lifecycle, but I don't need to switch it dynamically. It'll be either on or off for the life of the process.

I ended up doing Safecast/safecastapi#677 which uses the gemfile in order to avoid using what appears to be a deprecated config setter.

https://github.com/Safecast/safecastapi/pull/677/files#diff-b1fe55db50c712fef0673345e5b9c0d9L13

[matschaffer]

Fwiw, the configuration was a little surprising/confusing compared to newrelic.

With newrelic all the config is always in place (I think it even provides a dev view though I don't use it much). But data is only sent if NEW_RELIC_LICENSE_KEY is exported to the env (https://github.com/Safecast/safecastapi/blob/master/config/newrelic.yml#L15)

I was trying to figure out how to do something like that with elastic APM (only sending data if ELASTIC_APM_SECRET_TOKEN is set, assuming ELASTIC_APM_SERVER_URL will also be set). config.elastic_apm.active = ENV['ELASTIC_APM_SECRET_TOKEN'].present? looked promising, but active= is deprecated here and I didn't see an enabled=...

Though now that I type that maybe enabled= works just fine 🤦

Update: Okay, after digging into the config loading a bit (especially https://github.com/elastic/apm-agent-ruby/blob/master/lib/elastic_apm/railtie.rb#L25-L36) I can see that enabled= should work once this PR is released. So I guess we could have gone with config.elastic_apm.active = ENV['ELASTIC_APM_SECRET_TOKEN'].present? the switched to config.elastic_apm.enabled = ENV['ELASTIC_APM_SECRET_TOKEN'].present?

Probably fine for now, just more of my own confusion about how to get the gem configured in a way that's nice for both prod & dev.

[estolfo]

yes, config.elastic_apm.enabled would work in the next release :)

Thanks for the feedback regarding comparison between new relic and elastic APM. I do think, however, the best and simplest solution is what you've gone with-- using the groups in the Gemfile as there is no point in loading the gem if it is not intended to be used.

[matschaffer]

Agreed. Thanks for helping get me pointed in a good direction.