Skip to content

Commit

Permalink
update graphql probe for new tracing modules
Browse files Browse the repository at this point in the history
  • Loading branch information
zvkemp committed May 5, 2023
1 parent a41caf0 commit 3b629e1
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 9 deletions.
50 changes: 49 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile.additional
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions gemfiles/graphql-2.0.17/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
source "https://rubygems.org"

gemspec path: "../.."

gem "rails", "~> 6.1"
gem "sqlite3"

gem "graphql", "2.0.17.2"
55 changes: 51 additions & 4 deletions lib/skylight/probes/graphql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/skylight/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions lib/tasks/workflow.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }]
Expand Down Expand Up @@ -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" },
Expand Down
2 changes: 0 additions & 2 deletions spec/integration/graphql_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3b629e1

Please sign in to comment.