dd-trace-rb icon indicating copy to clipboard operation
dd-trace-rb copied to clipboard

GraphQL tracing with Rails 7.0

Open liaden opened this issue 2 years ago • 3 comments

Current behaviour Rails commands fail due to the error uninitialized constant MyAppSchema

Expected behaviour Application starts and monitors just like it did on Rails 6.1.

Steps to reproduce

  • Take working 6.1 app with graphql instrumented and attempt to upgrade app

Environment

  • ddtrace version: 1.18.0
  • Configuration block (Datadog.configure ...): c.tracing.instrument :graphql, schemas: [MyAppSchema]
  • Ruby version: 3.1.3
  • Operating system:
  • Relevant library versions:

liaden avatar Jan 02 '24 17:01 liaden

👋 @liaden , I noticed that Rails 7 change to Zeitwerk autoloading, could this be the reason of this error?

The error looks like could be solved by requiring your schema file, have you tried that?

TonyCTHsu avatar Jan 03 '24 15:01 TonyCTHsu

@TonyCTHsu Yeah, however, we'd have to go through and load all of the constants underneath app/graphql which for our small app is only 15 files, but for our larger monolith is 667 ruby files.

This seems similar to https://github.com/DataDog/dd-trace-rb/issues/2709 and our configuration would align with Autoloading and Reloading Use Case 3 where we should specify the class as a string that uses constantize to resolve it to the class later?

liaden avatar Jan 03 '24 17:01 liaden

👋 @liaden , Thanks for bringing this up.

I have looked into the exception_controller option from https://github.com/DataDog/dd-trace-rb/issues/2709 . This was solved because I found out that the option is useless. It is deprecated and will be removed. Hence, I have not addressed the zeitwerk autoloading problem yet.

From Autoloading and Reloading Use Case 3, passing a string instead of a class seems reasonable to me. However, it failed with NameError from Object.get_const(schema_string), because the specified schema class has not yet been loaded.

I agree that our configuration should work smoother with zeitwerk, but it is going to require a lot of fundamental changes.

Currently, only the specified schemas are being instrumented, however, I think it would be better to instrument all schemas when no specification. This could help circumvent the trouble for zeitwerk autoloading for some cases (I supposed most of our users are instrumenting all the schemas). What do yo think?

For now, the workaround I could provide would be adding this configuration in your Rails app (assuming your schema is sitting under such path).

config.autoload_once_paths << "#{root}/app/graphql"

TonyCTHsu avatar Jan 17 '24 11:01 TonyCTHsu