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

Implement ActiveRecord integration configuration #451

Merged
merged 7 commits into from
Jul 11, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Jun 11, 2018

Following from #450 , this pull request implements the integration configuration core for ActiveRecord, and leverages its multiplexing feature to implement the API for multi-database configuration (based on #404), permitting multiple databases to be configured like so:

Datadog.configure do |c|
  # Where 'gadget_db' is a key in the `database.yml` configuration
   c.use :active_record, describes: :gadget_db do |gadget_db|
    gadget_db.tracer = tracer
    gadget_db.service_name = gadget_db_service_name
  end

  # You can also provide connection strings as a key
  c.use :active_record, describes: 'mysql2://root@127.0.0.1:3306/mysql' do |backup_db|
    backup_db.service_name = 'backup-db'
  end
end

@delner delner added integrations Involves tracing integrations feature Involves a product feature labels Jun 11, 2018
@delner delner self-assigned this Jun 11, 2018
@delner delner force-pushed the feature/integration_configuration_settings branch from 887e679 to afe57f3 Compare June 11, 2018 21:47
@delner delner force-pushed the refactor/active_record_integration_configuration branch from 01dc89d to 879b111 Compare June 11, 2018 21:54
@delner delner added this to the 0.14.0 milestone Jun 15, 2018
@delner delner force-pushed the feature/integration_configuration_settings branch from afe57f3 to c412d1c Compare June 25, 2018 18:01
@delner delner force-pushed the refactor/active_record_integration_configuration branch from 879b111 to c94bfb7 Compare June 26, 2018 17:41
@delner delner force-pushed the refactor/active_record_integration_configuration branch from c94bfb7 to c9873f8 Compare June 29, 2018 19:53
@pawelchcki pawelchcki changed the base branch from feature/integration_configuration_settings to 0.14-dev July 11, 2018 14:38
@@ -0,0 +1,301 @@
require 'uri'

# NOTE: This code is copied directly from ActiveRecord.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about putting this file in a folder named vendor? Similarily how gems are put in ROOT/vendor but here it would be something like:

lib/ddtrace/contrib/active_record/vendor/configuration/connection_specification.rb

Its a small and cosmetic change, but it would make me feel much better about including files from different projects verbatim.

We could actually insert this file without any modifications too, and add small note in the vendor folder, and exclude the whole prefix in the Rubocop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I like that idea. Would make the Rubocop and other stuff much simpler to deal with too. I'll try this.

@delner delner force-pushed the refactor/active_record_integration_configuration branch from cc96863 to 91e19ad Compare July 11, 2018 18:06
@delner delner merged commit f1d8eac into 0.14-dev Jul 11, 2018
@delner delner deleted the refactor/active_record_integration_configuration branch August 14, 2018 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Involves a product feature integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants