[agents] definition of ACTIVE/DISABLED_INSTRUMENTATION

[axw]

This is an offshoot of #76.

There are some questions about whether active and disableInstrumentations config should be reloadable, or whether they should be static.

Currently, in the Node.js agent, starting the agent with active: false would skip instrumentation entirely. This means that it is not possible to dynamically "activate" the agent.

Elsewhere, users of the Java agent have assumed that setting active: false would completely disable the agent: elastic/apm-agent-java#639. Given the config's name, this is pretty a fair assumption. In that case it was due to metrics which we can fix, but still leaves a question about remote config polling in the future.

So, open questions:

• What should active: false do? Should this just disable event generation, or should it disable all code paths, including remote config polling?
  • If active: false disables remote config polling, should we have some other config that allows the agent to run "dormant" - polling remote config, but not generating events?
  • If active: false disables only event generation, should we have some other config which disables everything, including remote config polling?
• Should we have ternary state for disabling instrumentation? On, off (but possible to turn on), skipped entirely (never instrumented in the first place - cannot turn on).

[SergeyKleyman]

I would prefer to have two separate Boolean configuration settings:

• enabled (or any other suitable name) - this setting is read by the agent only during the startup and if it is set to false the agent should do as little as possible in order to simulate the state as if the agent is not there at all. In particular this would mean not communicating with APM server (we can discuss if it also means not writing any logs)
• active - unlike enabled the agent should support changing this configuration setting after the startup. If it's set to false the agent should "shrink" its footprint to the "minimum" as long as the first part of this requirement is satisfied (so agent should continue polling remote config and it should be able to go back and forth between "shrunk to minimum" and "fully functioning" states). The goal in minimizing the footprint is the same as in enabled - to simulate the state as if the agent is not there at all but there might be additional development cost, performance, etc. constraints affecting how small is this minimal footprint can be so we can leave it for each agent team to decide.

[axw]

@SergeyKleyman I was thinking the same. The main issue I have with this is the subtle difference in meaning between "enabled" and "active" - I've not had any better ideas for an alternative name to "enabled". I suppose these are not things that people will want to do every day, so maybe that's not a big issue anyway.

[SergeyKleyman]

In addition to introduction of enabled we can also rename active setting to stress the distinction between them - maybe paused? The additional advantage of having completely new configuration settings would be easier migration for agents that had different semantics for active.

[mikker]

I like @SergeyKleyman's suggestion and I agree we could probably come up with more telling names.

[mikker]

Didn't think too long but how about keeping enabled for completely turned off or not, and recording for sending data or not.

[SergeyKleyman]

Yes, I agree - recording is better than paused because it's aligned with enabled in that they are both "positive" :) (both set to true is the default, fully functioning state).

[watson]

I like the names enabled and recording 👍 By not giving active a new meaning it also leaves us with the option to simply alias it to enabled, deprecate it, and remove it in a major or two.

Just to ensure there are no misunderstandings, the recording flag is not to be confused with the disableSend flag that a few of the agents have. Setting disableSend: false has a bit more overhead than recording: false will have because the latter will be as close to a no-op as possible.

Regarding disableInstrumentations, I'd still like this config option to behave in a way that's non-reversible - similar to the new enabled flag we're discussing.

[felixbarny]

Will recording: false also disable metrics?

[watson]

@felixbarny I hope so. In my mind, this is as close as we can get to enabled: false just with the ability to turn it on via remote config later. So it has to still poll /config, but that's it.

[Qard]

Personally I think we should just rely on more focused config options like remoteConfig: false. We're coming into this issue because of the gradually increasing complexity of adding more and more new features. As new features are added, the meaning of our configs may change subtly, if they are not more specific. I'm sure we'd eventually find another case where enabled or recording is not nuanced enough to fully describe what it's trying to do and needs to be split up again. 🤷‍♂

[SergeyKleyman]

@Qard I agree that it's easier to understand and support configuration options that are as orthogonal/independent in their semantics as possible. OTOH use case for making agent's presence as less felt as possible without restarting the application is IMHO so widespread that making an exception for it and introducing an "aggregate" configuration option (instead of having to remember each one of the "atomic" configuration options it's composed of) is worth it.