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

Fix AWS patcher loading wrong constants when AWS is partially loaded #945

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

delner
Copy link
Contributor

@delner delner commented Feb 11, 2020

Resolves #938.

There's a scenario when AWS is partially loaded such that certain constants are not available, const_get will search ancestors and use any matching constant there, which results in the wrong constant being patched.

Additionally, despite providing a false flag to disable ancestor search, Module#const_get seems to ignore this sometimes when Rails autoloading is present (at least the first time), causing it to still unexpectedly return the wrong constant.

e.g.

module Support
  class Client
  end
end

module Aws
  # Not loaded
  # module Support
  #   class Client
  #   end
  # end
end

::Aws.const_get(:Support).const_get(:Client)
# => Support::Client
::Aws.const_get(:Support, false).const_get(:Client, false)
# In Ruby env, => NameError
# In Rails env, can result in Support::Client on first invocation.

In this PR we make sure to cross check the loaded constants against the list, then only collect what's actually available, rather than depending on const_get to resolve any namespace conflicts.

@delner delner added bug Involves a bug integrations Involves tracing integrations community Was opened by a community member labels Feb 11, 2020
@delner delner requested review from marcotc and a team February 11, 2020 23:21
@delner delner self-assigned this Feb 11, 2020
@delner
Copy link
Contributor Author

delner commented Feb 11, 2020

@illdelph you can test this with the following pre-release gem:

source 'http://gems.datadoghq.com/prerelease' do
  gem 'ddtrace', '0.32.0.fix.aws.constant.loading.55008'
end

@illdelph
Copy link

source 'http://gems.datadoghq.com/prerelease' do
gem 'ddtrace', '0.32.0.fix.aws.constant.loading.55008'
end

Yeah, that seems to do the trick

@delner delner merged commit ddccd46 into master Feb 12, 2020
@delner delner deleted the fix/aws_constant_loading branch February 12, 2020 22:39
@delner delner added this to the 0.33.0 milestone Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS patcher loads wrong Client constant
3 participants