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

Integrate with ActiveRecord::QueryLogs (old marginalia) #4946

Open
mrcasals opened this issue May 7, 2024 · 4 comments
Open

Integrate with ActiveRecord::QueryLogs (old marginalia) #4946

mrcasals opened this issue May 7, 2024 · 4 comments

Comments

@mrcasals
Copy link

mrcasals commented May 7, 2024

Is your feature request related to a problem? Please describe.

ActiveRecord::QueryLogs (the old marginalia gem, now bundled in Rails 7) allow us to add comments on SQL queries to detect where is the query triggered from (what controller and action). But when using GraphQL, the comment becomes useless because it's always the same controller and action, so we become blind.

Describe the solution you'd like

ActiveRecord::QueryLogs allow for customization so we could add the current GraphQL query being resolved.

Describe alternatives you've considered

None.

Additional context

None.

@rmosolgo
Copy link
Owner

rmosolgo commented May 7, 2024

Hey, thanks for the suggestion! I think that's a great idea. Here's how you could add that in your own app in the meantime:

# In your Application setup: 
config.active_record.query_log_tags = [ 
  # Rails default keys: 
  :application, :controller, :action, :job,
  # Tell QueryLogs about this new key: 
  { graphql_query: -> { Current.graphql_query_names } }
]

# Assuming `class Current < ActiveSupport::CurrentAttributes` as in https://api.rubyonrails.org/classes/ActiveSupport/CurrentAttributes.html
# Substitute your own global state object as-needed
module ActiveRecordQueryLogsTrace
  def execute_multiplex(multiplex:)
    Current.graphql_query_names = multiplex.queries.map { |q| q.selected_operation&.name || "anonymous" }
  end 
end 

I think that would tag ActiveRecord queries with the currently running operation(s). Is that what you had in mind?

A deeper integration is also possible, for example, tagging SQL with the currently-running Dataloader::Source or GraphQL field.

@mrcasals
Copy link
Author

mrcasals commented May 8, 2024

This looks good, I'm trying to adapt it to my code because I'm using an old version of the gem, v2.0.14 (sorry, forgot to mention it!). I'm guessing the ActiveRecordQueryLogsTrace module would be used with trace_with(ActiveRecordQueryLogsTrace) in the latest version, so I should use tracer(ActiveRecordQueryLogsTrace) there and adapt it, I'll get back to you!

Thanks!

@rmosolgo
Copy link
Owner

rmosolgo commented May 8, 2024

Here's an approach that would work in 2.0.x:

# Assuming `class Current < ActiveSupport::CurrentAttributes` as in https://api.rubyonrails.org/classes/ActiveSupport/CurrentAttributes.html
# Substitute your own global state object as-needed
module ActiveRecordQueryLogsInstrumenter
  def self.begin_multiplex(multiplex)
    Current.graphql_query_names = multiplex.queries.map { |q| q.selected_operation&.name || "anonymous" }
  end 

  def self.end_multiplex(_multiplex); end 
end 


# ... 

class MySchema < GraphQL::Schema 
  instrument(:multiplex, ActiveRecordQueryLogsInstrumenter 
end 

(The instrument(...) API was merged into trace_with in recent versions.)

@mrcasals
Copy link
Author

mrcasals commented May 9, 2024

Awesome, it works! Thanks! I just had to change the method names, they should be before_multiplex and after_multiplex, but apart from that everything works fine! Thank you so much ❤️

I'll leave the issue open in case you want to track it to automatically add intergration with QueryLogs, but feel free to close it if needed!

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

No branches or pull requests

2 participants