From 3b629e15549fdd2a912a1c36d9fbbaa376e4085f Mon Sep 17 00:00:00 2001 From: zvkemp Date: Thu, 4 May 2023 12:03:18 -0700 Subject: [PATCH] update graphql probe for new tracing modules --- .github/workflows/build.yml | 50 ++++++++++++++++++++++++++++- CHANGELOG.md | 3 ++ gemfiles/Gemfile.additional | 2 +- gemfiles/graphql-2.0.17/Gemfile | 8 +++++ lib/skylight/probes/graphql.rb | 55 +++++++++++++++++++++++++++++--- lib/skylight/version.rb | 2 +- lib/tasks/workflow.rake | 6 ++++ spec/integration/graphql_spec.rb | 2 -- 8 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 gemfiles/graphql-2.0.17/Gemfile diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2648e0db..51cec739 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,7 +11,7 @@ env: RAILS_ENV: development EMBEDDED_HTTP_SERVER_TIMEOUT: '30' WORKER_SPAWN_TIMEOUT: '15' - CONFIG_DIGEST: 717893199685d608b3344488549d189c17b8e76c07f2b31bb81ffafda8e5dca8 + CONFIG_DIGEST: de849583dc16173af4bee6fd30e354d66b559cbc32d5730b998c9f5d42c76f70 'on': push: branches: @@ -323,6 +323,53 @@ jobs: run: bundle exec rake needs: - ruby-3-2-default + ruby-3-2-graphql-2-0-17: + name: ruby 3.2, graphql-2.0.17 + runs-on: ubuntu-latest + if: github.ref == 'refs/heads/master' || contains(github.event.pull_request.labels.*.name, + 'full-ci') || contains(github.event.pull_request.labels.*.name, 'graphql-2.0.17') + services: + redis: + image: redis + ports: + - 6379:6379 + options: "--entrypoint redis-server" + env: + BUNDLE_GEMFILE: gemfiles/graphql-2.0.17/Gemfile + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Setup ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.2' + - name: Install APT dependencies + run: | + sudo apt-get update + sudo apt-get install -yq sqlite libsqlite3-dev + - name: Setup cache (bundler) + uses: actions/cache@v3 + with: + path: "${{ github.workspace }}/vendor/bundle" + key: "${{ runner.os }}-gems-3.2-graphql-2.0.17-${{ hashFiles('gemfiles/graphql-2.0.17/Gemfile.lock') + }}" + restore-keys: |- + ${{ runner.os }}-gems-3.2-graphql-2.0.17- + ${{ runner.os }}-gems-3.2- + - name: bundle install + run: | + gem install bundler + bundle install + - name: Run tests + run: | + bundle exec rake workflow:verify[$CONFIG_DIGEST] + bundle exec rake + - name: Run tests (agent disabled) + env: + SKYLIGHT_DISABLE_AGENT: 'true' + run: bundle exec rake + needs: + - ruby-3-2-default ruby-2-7-rails-5-2-x: name: ruby 2.7, rails-5.2.x runs-on: ubuntu-latest @@ -1199,6 +1246,7 @@ jobs: - ruby-3-2-elasticsearch-elasticsearch - ruby-2-7-sidekiq-5-x - ruby-2-7-graphql-1-9-x + - ruby-3-2-graphql-2-0-17 - ruby-2-7-rails-5-2-x - ruby-3-2-rails-6-0-x - ruby-3-2-rails-6-1-x diff --git a/CHANGELOG.md b/CHANGELOG.md index 49bef9fb..018ff8d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## 6.0.0-beta2 (prerelease) +- [IMPROVEMENT] Better support for GraphQL versions >= 2.0.18. + ## 6.0.0-beta (prerelease) - [BREAKING] End support for Ruby 2.6 - The following libraries are no longer tested and are not guaranteed to work with Skylight 6: diff --git a/gemfiles/Gemfile.additional b/gemfiles/Gemfile.additional index 1ee31f4d..68bc36fb 100644 --- a/gemfiles/Gemfile.additional +++ b/gemfiles/Gemfile.additional @@ -4,7 +4,7 @@ gem "delayed_job_active_record", require: false gem "excon" gem "faraday" gem "graphiti", git: "https://github.com/graphiti-api/graphiti.git" -gem "graphql", "~> 2" +gem "graphql", "~> 2.0.18" gem "httpclient" gem "rexml" gem "sequel", "> 0" diff --git a/gemfiles/graphql-2.0.17/Gemfile b/gemfiles/graphql-2.0.17/Gemfile new file mode 100644 index 00000000..109428f4 --- /dev/null +++ b/gemfiles/graphql-2.0.17/Gemfile @@ -0,0 +1,8 @@ +source "https://rubygems.org" + +gemspec path: "../.." + +gem "rails", "~> 6.1" +gem "sqlite3" + +gem "graphql", "2.0.17.2" diff --git a/lib/skylight/probes/graphql.rb b/lib/skylight/probes/graphql.rb index 134d9c76..436fb4ca 100644 --- a/lib/skylight/probes/graphql.rb +++ b/lib/skylight/probes/graphql.rb @@ -11,20 +11,67 @@ def initialize(*, **) return unless defined?(@tracers) + # This is the legacy tracing used in graphql =< 2.0.17 unless @tracers.include?(::GraphQL::Tracing::ActiveSupportNotificationsTracing) @tracers << ::GraphQL::Tracing::ActiveSupportNotificationsTracing end end end + module InstrumentationV2 + def self.included(base) + base.singleton_class.prepend ClassMethods + end + + # GraphQL versions 2.0.18 - 2.0.21 (or higher?) were missing this notification + module ExecuteMultiplexNotification + def execute_multiplex(**metadata, &blk) + if @notifications_engine + @notifications_engine.instrument("execute_multiplex.graphql", metadata, &blk) + else + # safety fallback in case graphql's authors unexpectedly rename @notifications_engine + super + end + end + end + + module ClassMethods + def new_trace(*, **) + unless @__sk_instrumentation_installed + trace_with(::GraphQL::Tracing::ActiveSupportNotificationsTrace) + + unless ::GraphQL::Tracing::ActiveSupportNotificationsTrace.instance_methods.include?(:execute_multiplex) + trace_with(ExecuteMultiplexNotification) + end + + @__sk_instrumentation_installed = true + end + + super + end + end + end + class Probe def install - tracing_klass_name = "::GraphQL::Tracing::ActiveSupportNotificationsTracing" - klasses_to_probe = %w[::GraphQL::Execution::Multiplex ::GraphQL::Query] + new_tracing = false + begin + require "graphql/tracing/active_support_notifications_trace" + new_tracing = true + rescue LoadError # rubocop:disable Lint/SuppressedException + end + + if new_tracing + # GraphQL >= 2.0.18 + ::GraphQL::Schema.include(InstrumentationV2) + else + tracing_klass_name = "::GraphQL::Tracing::ActiveSupportNotificationsTracing" + klasses_to_probe = %w[::GraphQL::Execution::Multiplex ::GraphQL::Query] - return unless ([tracing_klass_name] + klasses_to_probe).all?(&method(:safe_constantize)) + return unless ([tracing_klass_name] + klasses_to_probe).all?(&method(:safe_constantize)) - klasses_to_probe.each { |klass_name| safe_constantize(klass_name).prepend(Instrumentation) } + klasses_to_probe.each { |klass_name| safe_constantize(klass_name).prepend(Instrumentation) } + end end def safe_constantize(klass_name) diff --git a/lib/skylight/version.rb b/lib/skylight/version.rb index 6dae49ac..4135dd15 100644 --- a/lib/skylight/version.rb +++ b/lib/skylight/version.rb @@ -3,5 +3,5 @@ module Skylight # for compatibility with semver when it is parsed by the rust agent. # This string will be transformed in the gemspec to "5.0.0.alpha" # to conform with rubygems. - VERSION = "6.0.0-beta".freeze + VERSION = "6.0.0-beta2".freeze end diff --git a/lib/tasks/workflow.rake b/lib/tasks/workflow.rake index 1b4c771b..2073ecc1 100644 --- a/lib/tasks/workflow.rake +++ b/lib/tasks/workflow.rake @@ -60,6 +60,10 @@ module CITasks allow: [{ "dependency-name": "graphql" }], ignore: [{ "dependency-name": "graphql", versions: [">= 1.10"] }] }, + "graphql-2.0.17" => { + allow: [{ "dependency-name": "graphql" }], + ignore: [{ "dependency-name": "graphql", versions: [">= 2.0.18"] }] + }, "mongoid-6.x" => { allow: [{ "dependency-name": "mongoid" }], ignore: [{ "dependency-name": "mongoid", versions: [">= 7"] }] @@ -162,6 +166,8 @@ module CITasks # GraphQL 1.11 is tested as part of our default additional gems # TODO: We should test 1.12+ + # Latest version of graphql with legacy instrumentation + { ruby_version: NEWEST_RUBY, gemfile: "graphql-2.0.17" }, { gemfile: "rails-5.2.x", ruby_version: OLDEST_RUBY, always_run: true }, { always_run: true, ruby_version: NEWEST_RUBY, gemfile: "rails-6.0.x" }, { always_run: true, ruby_version: NEWEST_RUBY, gemfile: "rails-6.1.x" }, diff --git a/spec/integration/graphql_spec.rb b/spec/integration/graphql_spec.rb index d06e8d3e..b02e0ecb 100644 --- a/spec/integration/graphql_spec.rb +++ b/spec/integration/graphql_spec.rb @@ -174,8 +174,6 @@ class Types::MutationType < Types::BaseObject end class TestAppSchema < GraphQL::Schema - # tracer(GraphQL::Tracing::ActiveSupportNotificationsTracing) - mutation(Types::MutationType) query(Types::QueryType) end