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

Type coercion behavior inconsistencies and flaws between configuration formats and between APM and CSEC #2852

Open
fallwith opened this issue Sep 16, 2024 · 1 comment
Labels

Comments

@fallwith
Copy link
Contributor

Existing type coercion behavior is implemented pretty well in the environment variable handling logic, given that all environment variable values are strings on input. But sourcing parameters from elsewhere - especially the YAML configuration file - will bypass this type coercion, leading to the following outstanding issues:

  • Behavior from environment variable input (example: '123' becomes 123) can reasonably be expected from other configuration sources, leading to unexpected behavior (or more specifically a lack of coercion behavior in this case).
  • The CSEC agent - which needs to deal with numeric parameter values more than the APM agent does - now does interger coercion that the APM agent do. Users may reasonably expect consistency here.
  • The lack of integer coercion is particularly problematic because in one way the agent tries to do the right thing by not succeeding with a string value at all (rather than converting a string unexpectedly to 0 or whatnot), but unfortunately has the potential to permit a fatal exception to lay hidden by not performing any type or bounds check on some integers until the time that they are actively used. This lack of config parse time checking in addition to the lack of type coercion can be particularly problematic (buggy).
  • Existing non-coercion behavior for the YAML input seems to be pretty well rooted in New Relic's history of only offering a Ruby agent back when the company first started. In those days, knowledge of Ruby and YAML were pretty safely assumed. Nowadays with New Relic offering multiple agents that take advantage of multiple configuration file formats, knowledge of YAML-to-Ruby behavior should not assumed when reasonable coercion can be performed - especially when this coercion already takes place for environment variables. This will be especially important to customers that leverage multiple agents to observe applications written in multiple languages.

While bullet point 4 might make for a good "feature request" candidate, the first 3 bullet points pertaining to inconsistency and potential unexpected behavior swayed us into favoring the "bug" category.

Definition of Done

  • For all configuration parameters (defined in default_source.rb) that make use of a :type attribute, ensure that user supplied values are coerced to that type in a way that is consistent between YAML and environment variables and between the Ruby APM and CSEC agents.
  • To address bullet point 3, any problematic invalid values should be identified at agent start time and not at the time the agent goes to use the values that were previously blindly trusted.
@fallwith fallwith added the bug label Sep 16, 2024
@workato-integration
Copy link

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

No branches or pull requests

1 participant