-
Notifications
You must be signed in to change notification settings - Fork 25
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
Instrument sequel gem #414
Changes from 11 commits
90da423
01df966
7c487bd
bceee82
419c7c2
1800d66
c95e8e5
9eacd24
7a9ec8c
d4fa3e1
75da33a
e233a33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# This file was generated by Appraisal | ||
|
||
# (c) Copyright IBM Corp. 2024 | ||
|
||
source "https://rubygems.org" | ||
|
||
gem "minitest-reporters" | ||
gem "webmock" | ||
gem "puma" | ||
gem "rack-test" | ||
gem "simplecov", "~> 0.21.2" | ||
gem "sequel", ">= 5.0" | ||
gem "sqlite3", "~> 1.4" | ||
gem "mysql2", "0.5.5" | ||
|
||
gemspec path: "../" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# (c) Copyright IBM Corp. 2024 | ||
|
||
module Instana | ||
module Activators | ||
class Sequel < Activator | ||
def can_instrument? | ||
defined?(::Sequel::Database) | ||
end | ||
|
||
def instrument | ||
require 'instana/instrumentation/sequel' | ||
|
||
::Sequel::Database | ||
.prepend(Instana::Instrumentation::Sequel) | ||
|
||
true | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,7 @@ def initialize(logger: ::Instana.logger, agent_host: ENV['INSTANA_AGENT_HOST'], | |
@config[:'resque-client'] = { :enabled => true, :propagate => true } | ||
@config[:'resque-worker'] = { :enabled => true, :'setup-fork' => true } | ||
@config[:'rest-client'] = { :enabled => true } | ||
@config[:sequel] = { :enabled => true } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we align the indentation here? |
||
@config[:'sidekiq-client'] = { :enabled => true } | ||
@config[:'sidekiq-worker'] = { :enabled => true } | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# (c) Copyright IBM Corp. 2024 | ||
|
||
module Instana | ||
module Instrumentation | ||
module Sequel | ||
IGNORED_SQL = %w[BEGIN COMMIT SET].freeze | ||
VERSION_SELECT_STATEMENT = "SELECT VERSION()".freeze | ||
SANITIZE_REGEXP = /('[\s\S][^']*'|\d*\.\d+|\d+|NULL)/i.freeze | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This regex is present in active record. So i copied the same. We sanitise sql so that the ppl viewing the logs might not get the information like PII etc etc. So NULL can be used to filter data. Hence i think we are sanitising NULL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am familiar with the purpose of sanitation. My point is that the string |
||
|
||
def log_connection_yield(sql, conn, *args) | ||
call_payload = { | ||
sequel: { | ||
adapter: opts[:adapter], | ||
host: opts[:host], | ||
username: opts[:user], | ||
db: opts[:database], | ||
sql: maybe_sanitize(sql) | ||
} | ||
} | ||
maybe_trace(call_payload) { super(sql, conn, *args) } | ||
end | ||
|
||
private | ||
|
||
def maybe_sanitize(sql) | ||
::Instana.config[:sanitize_sql] ? sql.gsub(SANITIZE_REGEXP, '?') : sql | ||
end | ||
|
||
def maybe_trace(call_payload, &blk) | ||
if ::Instana.tracer.tracing? && !ignored?(call_payload) | ||
::Instana.tracer.trace(:sequel, call_payload, &blk) | ||
else | ||
yield | ||
end | ||
end | ||
|
||
def ignored?(call_payload) | ||
IGNORED_SQL.any? { |s| call_payload[:sequel][:sql].upcase.start_with?(s) } || call_payload[:sequel][:sql].upcase == VERSION_SELECT_STATEMENT | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
# (c) Copyright IBM Corp. 2024 | ||
|
||
require 'test_helper' | ||
require 'sequel' | ||
|
||
class SequelTest < Minitest::Test | ||
def setup | ||
skip unless ENV['DATABASE_URL'] | ||
db_url = ENV['DATABASE_URL'].sub("sqlite3", "sqlite") | ||
@db = Sequel.connect(db_url) | ||
@db.create_table!(:blocks) do | ||
String :name | ||
String :color | ||
end | ||
@model = @db[:blocks] | ||
end | ||
|
||
def teardown | ||
@db.drop_table(:blocks) | ||
@db.disconnect | ||
end | ||
|
||
def test_config_defaults | ||
assert ::Instana.config[:sanitize_sql] == true | ||
assert ::Instana.config[:sequel].is_a?(Hash) | ||
assert ::Instana.config[:sequel].key?(:enabled) | ||
assert_equal true, ::Instana.config[:sequel][:enabled] | ||
end | ||
|
||
def test_create | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@model.insert(name: 'core', color: 'blue') | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
data = span[:data][:sequel] | ||
assert data[:sql].start_with?('INSERT INTO') | ||
end | ||
|
||
def test_read | ||
@model.insert(name: 'core', color: 'blue') | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@model.where(name: 'core').first | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
data = span[:data][:sequel] | ||
assert data[:sql].start_with?('SELECT') | ||
assert_nil span[:ec] | ||
end | ||
|
||
def test_update | ||
@model.insert(name: 'core', color: 'blue') | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@model.where(name: 'core').update(color: 'red') | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
data = span[:data][:sequel] | ||
assert data[:sql].start_with?('UPDATE') | ||
assert_nil span[:ec] | ||
assert_equal 'red', @model.where(name: 'core').first[:color] | ||
end | ||
|
||
def test_delete | ||
@model.insert(name: 'core', color: 'blue') | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@model.where(name: 'core').delete | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
data = span[:data][:sequel] | ||
assert data[:sql].start_with?('DELETE') | ||
assert_nil span[:ec] | ||
assert_nil @model.where(name: 'core').first | ||
end | ||
|
||
def test_raw | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@db.run('SELECT 1') | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
data = span[:data][:sequel] | ||
assert 'SELECT 1', data[:sql] | ||
assert_nil span[:ec] | ||
end | ||
|
||
def test_raw_error | ||
assert_raises Sequel::DatabaseError do | ||
Instana::Tracer.start_or_continue_trace(:sequel_test, {}) do | ||
@db.run('INVALID') | ||
end | ||
end | ||
spans = ::Instana.processor.queued_spans | ||
assert_equal 2, spans.length | ||
span = find_first_span_by_name(spans, :sequel) | ||
assert_equal 1, span[:ec] | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, tha added option in
config.rb
, would be used here already. In fact I don't see anywhere in production code that you readconfig[:sequel]
.