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

"active" config not being respected when the agent is started with -r or top level import #4112

Open
2 of 3 tasks
csnate opened this issue Jun 28, 2024 · 1 comment · May be fixed by #4119
Open
2 of 3 tasks

"active" config not being respected when the agent is started with -r or top level import #4112

csnate opened this issue Jun 28, 2024 · 1 comment · May be fixed by #4119

Comments

@csnate
Copy link

csnate commented Jun 28, 2024

Describe the bug

The active config value is not being respected when it's set to "false" if I start the agent with the require start module or the -r opt and I use the agent config file to load the configuration. All of the other options are correctly being used (data is getting to the apm server, service name and environment are set, etc.) but if I set active: false, Elastic APM is still recording data.

If I use the require and start way to start the agent, then it looks like the active flag is working as expected.

To Reproduce

Steps to reproduce the behavior:

  1. Use this config
    elastic-apm-node.js
module.exports = {
  active: false,
  serverUrl: "https://apm.my-server.com",
  serviceName: "my-service"
  environment: "development"
};
  1. Then call node -r elastic-apm-node/start.js app.js
  2. Then do some actions that would send data to Elastic APM
  3. See that data is still getting reported to Elastic APM

Expected behavior

No data is sent to Elastic APM when active: false when started with "require start module" or "-r opt"

Environment (please complete the following information)

  • OS: Linux Alpine/Docker
  • Node.js version: 20.15.0
  • APM Server version: 8.10.2
  • Agent version: 4.7.0 or 8.10.2 if you mean the Elastic Server Agent?

How are you starting the agent? (please tick one of the boxes)

  • Calling agent.start() directly (e.g. require('elastic-apm-node').start(...))
  • Requiring elastic-apm-node/start from within the source code
  • Starting node with -r elastic-apm-node/start

Additional context

Agent config options:

Click to expand
{
  active: false,
  serverUrl: "https://apm.my-server.com",
  serviceName: "my-service"
  environment: "development"
}

package.json dependencies:

Click to expand
"elastic-apm-node": "4.7.0"
@csnate
Copy link
Author

csnate commented Jun 28, 2024

I think I've found the issue - https://github.com/elastic/apm-agent-nodejs/blob/main/lib/config/schema.js#L879

  1. Before this line 862 the options are read in from the file, so fileOpts['active'] = false;
  2. Line 879, the check isn't going to pass because fileOpts[def.name] would evaluate to false, and then def.fileValue does not get set to false
  3. Then when the Config is constructed, the default value for active will "win out" since it is set and the fileOpts.active value is not set.
  4. Also, the reason it DOES work when passing the config into apm.start({...}) is because that config object gets assigned to Config after the defaults, so the value would be active: false correctly.

The fix would be to change that check on line 879 to be typeof fileOpts[def.name] !== "undefined". Wouldn't hurt to do the same thing on line 876 in case an environment variable was set to false as well.

If I get some time next week I can fork and submit a PR

csnate pushed a commit to csnate/apm-agent-nodejs that referenced this issue Jul 1, 2024
This allows a "false" boolean value to override a "true" default value when loading the options via the elastic-apm-node.js config file

Fixes: elastic#4112
Closes: elastic#4112
csnate added a commit to csnate/apm-agent-nodejs that referenced this issue Jul 2, 2024
This allows a "false" boolean value to override a "true" default value when loading the options via the elastic-apm-node.js config file

Fixes: elastic#4112
Closes: elastic#4112
@csnate csnate linked a pull request Jul 2, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant