search

rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

"NoMethodError: undefined method `instrumenter' for Skylight:Module" #1833

Closed AndrewRayCode closed 6 years ago

AndrewRayCode commented 6 years ago

We're starting to see this error in our logs for GraphQL queries:

File "/usr/local/rvm/gems/ruby-2.5.1@bundler/gems/graphql-pro-4dedb4349ae2/lib/graphql/pro/monitoring/skylight_platform.rb" line 32 in before_query
...
File "./app/controllers/graphql_controller.rb", line 43 in execute_query

We see this stack trace when performing normal GraphQL queries.

The line it's coming from in the graphql-pro gem is:

inst = Skylight.instrumenter

To get around a critical bug in the Skylight gem (long story), we started running our Rails application with this configuration flag set:

SKYLIGHT_LAZY_START=false

This flag is as internal API setting for Skylight, but we worked with their support team to identify Skylight's critical bug, and they signed off that this flag setting should address our issue.

The docs (https://www.rubydoc.info/gems/skylight/0.8.1/Skylight/Instrumenter) for the Skylight::Instrumenter class say it's a private API, so it's unclear to me if this is a problem with our use of the internal API, or your use of the internal API (or both, or neither).

Are you able to reproduce this, setting the above env var, with the below versions?

Skylight gem version 2.0.2 Graphql pro version 1.7.8

rmosolgo commented 6 years ago

🤔 Interesting! In the 2.0.2 docs, Skylight::Instrumenter is no longer marked as private.

About a year ago, I replaced graphql-pro's monitoring with an open-source "tracing" system that covers more parts of the GraphQL query lifecycle, it has support for Skylight: http://graphql-ruby.org/queries/tracing.html#skylight

It only calls Skylight.instrumenter if a certain configuration is set:

https://github.com/rmosolgo/graphql-ruby/blob/master/lib/graphql/tracing/skylight_tracing.rb#L30-L33

Do you want to give that new tracer a try?

AndrewRayCode commented 6 years ago

Nevermind, this was a red herring, we had an engineer yoloing our test env and downgrading the skylight gem. I assumed this error was from the most recent version of the gem (it's not)

rmosolgo commented 6 years ago

😅 Glad you got to the bottom of it!