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

Use correct ActiveRecord connection for sql.active_record events #383

Merged
merged 2 commits into from
Mar 27, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Mar 23, 2018

Our previous implementation of connection configuration was always getting a default adapter configuration and using that to populate host, port and database tags for sql.active_record events.

This pull request adds functionality to retrieve more accurate information when the connection object ID is available, which is supplied in the payload for sql.active_record. With this change, we use that connection to populate the tags for ActiveRecord traces.

Such a change should represent a step forward in supporting multi-DB setups although we have yet to add support for configuring different DB connections as different services.

Example

For a request that creates the following records into different databases with sharding via active_record_shards:

screen shot 2018-03-26 at 11 34 51 am

The resulting trace which tags different DB connections per query. Notice the database name of highlighted span (which correlates to Article 2):

screen shot 2018-03-26 at 11 31 40 am

The same trace, different span (corresponding to Article 3) using other shard:

screen shot 2018-03-26 at 11 31 52 am

@delner delner added bug Involves a bug integrations Involves tracing integrations labels Mar 23, 2018
@delner delner self-assigned this Mar 23, 2018
@delner
Copy link
Contributor Author

delner commented Mar 23, 2018

Side note: this effectively undoes the #304 bugfix. This should pass the regression test put in place there, so I'm trusting that means the Resque issue shouldn't resurface.

Copy link
Contributor

@palazzem palazzem left a comment

Choose a reason for hiding this comment

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

Great, let's test it during the week with our release candidate.

@delner
Copy link
Contributor Author

delner commented Mar 26, 2018

Tested this with a sample app implementing active_record_shards to switch connections. Seems to be working fine.

@delner delner merged commit 6465242 into master Mar 27, 2018
delner added a commit that referenced this pull request Mar 27, 2018
…_connection

Use correct ActiveRecord connection for `sql.active_record` events
delner added a commit that referenced this pull request Mar 27, 2018
…_connection

Use correct ActiveRecord connection for `sql.active_record` events
@delner delner modified the milestones: 0.12.0, 0.11.4 Mar 28, 2018
@palazzem palazzem deleted the bugfix/use_correct_active_record_connection branch March 29, 2018 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug integrations Involves tracing integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants