From 2eb54f6972c1f49c3df9fb272ac2a8ade493c3fc Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 12 Dec 2024 12:12:00 -0500 Subject: [PATCH] NH-94970: remove marginalia code and add psql patch for dbo --- CONFIGURATION.md | 24 +- CONTRIBUTING.md | 4 - Rakefile | 6 +- gemfiles/rails_6x.gemfile | 9 - .../templates/solarwinds_apm_initializer.rb | 4 - .../patch/tag_sql/sw_mysql2_patch.rb | 2 - .../patch/tag_sql/sw_pg_patch.rb | 2 - lib/solarwinds_apm/patch/tag_sql_patch.rb | 1 + .../support/swomarginalia/LICENSE | 20 -- .../support/swomarginalia/README.md | 46 ---- .../support/swomarginalia/comment.rb | 206 ------------------ .../support/swomarginalia/formatter.rb | 20 -- .../swomarginalia/load_swomarginalia.rb | 55 ----- .../support/swomarginalia/railtie.rb | 24 -- .../support/swomarginalia/swomarginalia.rb | 89 -------- test/minitest_helper.rb | 29 ++- test/patch/sw_mysql2_patch_integrate_test.rb | 4 +- test/patch/sw_mysql2_patch_test.rb | 2 +- test/patch/sw_pg_patch_integrate_test.rb | 91 ++++++++ test/patch/sw_pg_patch_test.rb | 24 ++ test/run_tests.sh | 3 +- .../swomarginalia/swomarginalia_test.rb | 120 ---------- 22 files changed, 158 insertions(+), 627 deletions(-) delete mode 100644 gemfiles/rails_6x.gemfile delete mode 100644 lib/solarwinds_apm/support/swomarginalia/LICENSE delete mode 100644 lib/solarwinds_apm/support/swomarginalia/README.md delete mode 100644 lib/solarwinds_apm/support/swomarginalia/comment.rb delete mode 100644 lib/solarwinds_apm/support/swomarginalia/formatter.rb delete mode 100644 lib/solarwinds_apm/support/swomarginalia/load_swomarginalia.rb delete mode 100644 lib/solarwinds_apm/support/swomarginalia/railtie.rb delete mode 100644 lib/solarwinds_apm/support/swomarginalia/swomarginalia.rb create mode 100644 test/patch/sw_pg_patch_integrate_test.rb create mode 100644 test/patch/sw_pg_patch_test.rb delete mode 100644 test/support/swomarginalia/swomarginalia_test.rb diff --git a/CONFIGURATION.md b/CONFIGURATION.md index ced75c51..23722d7c 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -165,24 +165,18 @@ SELECT * FROM SAMPLE_TABLE WHERE user_id = 1; SELECT * FROM SAMPLE_TABLE WHERE user_id = 1; /* traceparent=7435a9fe510ae4533414d425dadf4e18-49e60702469db05f-01 */ ``` -For Rails < 7 and non-Rails applications, we use a port of [Marginalia](https://github.com/basecamp/marginalia) that patches ActiveRecord to inject the comment. +#### Limitation -For Rails >= 7, Marginalia is [integrated into ActiveRecord](https://api.rubyonrails.org/classes/ActiveRecord/QueryLogs.html) so enabling this feature should be done through Rails [configuration](https://guides.rubyonrails.org/v7.0/configuring.html#config-active-record-query-log-tags-enabled). For example: +Currently, tag queries don't support prepared statements. Active Record uses the PostgreSQL adapter (pg) by default to prepare statements. So, when you choose to use PostgreSQL and wish to enable trace context, please set `prepared_statements` to false." -```ruby -class Application < Rails::Application - config.active_record.query_log_tags_enabled = true - config.active_record.query_log_tags = [ - { - traceparent: -> { - SolarWindsAPM::SWOMarginalia::Comment.traceparent - } - } - ] -end -``` +e.g. -Note that with Rails >= 7.1 the comment format can be specified via the `config.active_record.query_log_tags_format` option. SolarWinds Observability functionality depends on the default `:sqlcommenter` format, it is not recommended to change this value. +```yaml +development: + adapter: postgresql + # ... + prepared_statements: false +``` ### Background Jobs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0e732b59..db7599b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -192,10 +192,6 @@ BUNDLE_GEMFILE=gemfiles/unit.gemfile bundle update BUNDLE_GEMFILE=gemfiles/unit.gemfile bundle exec ruby -I test test/opentelemetry/solarwinds_exporter_test.rb BUNDLE_GEMFILE=gemfiles/unit.gemfile bundle exec ruby -I test test/opentelemetry/solarwinds_propagator_test.rb - -# marginalia tests require the rails_6x.gemfile dependencies -BUNDLE_GEMFILE=gemfiles/rails_6x.gemfile bundle install -BUNDLE_GEMFILE=gemfiles/rails_6x.gemfile bundle exec ruby -I test test/support/swomarginalia/swomarginalia_test.rb ``` To run a specific test (that requires unit.gemfile): diff --git a/Rakefile b/Rakefile index c183912a..0bb265a1 100755 --- a/Rakefile +++ b/Rakefile @@ -25,17 +25,13 @@ Rake::TestTask.new do |t| gem_file = ENV['BUNDLE_GEMFILE']&.split('/')&.last case gem_file - when 'rails_6x.gemfile' - t.test_files = FileList['test/support/swomarginalia/*_test.rb'] - when 'unit.gemfile' t.test_files = FileList['test/api/*_test.rb'] + FileList['test/solarwinds_apm/*_test.rb'] + FileList['test/opentelemetry/*_test.rb'] + FileList['test/noop/*_test.rb'] + FileList['test/ext/*_test.rb'] + - FileList['test/support/*_test.rb'] - - FileList['test/support/swomarginalia/*_test.rb'] + FileList['test/support/*_test.rb'] end end diff --git a/gemfiles/rails_6x.gemfile b/gemfiles/rails_6x.gemfile deleted file mode 100644 index 6929f8f6..00000000 --- a/gemfiles/rails_6x.gemfile +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -source 'https://rubygems.org' - -eval(File.read(File.join(File.dirname(__FILE__), 'test_gems.gemfile'))) # rubocop:disable Security/Eval - -gem 'rails', '~> 6.0.0.0' -gem 'sidekiq', '~> 7.1.1' -gem 'sqlite3', '~> 1.4.3' diff --git a/lib/rails/generators/solarwinds_apm/templates/solarwinds_apm_initializer.rb b/lib/rails/generators/solarwinds_apm/templates/solarwinds_apm_initializer.rb index 5c838a6c..c850965b 100644 --- a/lib/rails/generators/solarwinds_apm/templates/solarwinds_apm_initializer.rb +++ b/lib/rails/generators/solarwinds_apm/templates/solarwinds_apm_initializer.rb @@ -169,9 +169,5 @@ # Example: # SELECT `posts`.* FROM `posts` /*traceparent=00-a448f096d441e167d12ebd32a927c1a5-a29655a47e430119-01*/ # - # This option can add a small overhead for prepared statements since the traceparent value is unique per execution. - # This feature uses marginalia, see its caveat and possible workaround - # https://github.com/basecamp/marginalia/blob/master/README.md#prepared-statements - # SolarWindsAPM::Config[:tag_sql] = false end diff --git a/lib/solarwinds_apm/patch/tag_sql/sw_mysql2_patch.rb b/lib/solarwinds_apm/patch/tag_sql/sw_mysql2_patch.rb index fcef0156..88e2bd8c 100644 --- a/lib/solarwinds_apm/patch/tag_sql/sw_mysql2_patch.rb +++ b/lib/solarwinds_apm/patch/tag_sql/sw_mysql2_patch.rb @@ -6,8 +6,6 @@ # # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. -require_relative 'swo_dbo_utils' - module SolarWindsAPM module Patch module TagSql diff --git a/lib/solarwinds_apm/patch/tag_sql/sw_pg_patch.rb b/lib/solarwinds_apm/patch/tag_sql/sw_pg_patch.rb index 58b55844..6d29505b 100644 --- a/lib/solarwinds_apm/patch/tag_sql/sw_pg_patch.rb +++ b/lib/solarwinds_apm/patch/tag_sql/sw_pg_patch.rb @@ -4,8 +4,6 @@ # # SPDX-License-Identifier: Apache-2.0 -require_relative 'swo_dbo_utils' - module SolarWindsAPM module Patch module TagSql diff --git a/lib/solarwinds_apm/patch/tag_sql_patch.rb b/lib/solarwinds_apm/patch/tag_sql_patch.rb index ef80b5c8..c19b4ded 100644 --- a/lib/solarwinds_apm/patch/tag_sql_patch.rb +++ b/lib/solarwinds_apm/patch/tag_sql_patch.rb @@ -6,5 +6,6 @@ # # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. +require_relative 'tag_sql/sw_dbo_utils' require_relative 'tag_sql/sw_mysql2_patch' require_relative 'tag_sql/sw_pg_patch' diff --git a/lib/solarwinds_apm/support/swomarginalia/LICENSE b/lib/solarwinds_apm/support/swomarginalia/LICENSE deleted file mode 100644 index 1ecad31f..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/LICENSE +++ /dev/null @@ -1,20 +0,0 @@ -# Copyright (c) 2012 37signals, LLC -# -# Permission is hereby granted, free of charge, to any person obtaining -# a copy of this software and associated documentation files (the -# "Software"), to deal in the Software without restriction, including -# without limitation the rights to use, copy, modify, merge, publish, -# distribute, sublicense, and/or sell copies of the Software, and to -# permit persons to whom the Software is furnished to do so, subject to -# the following conditions: -# -# The above copyright notice and this permission notice shall be -# included in all copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/lib/solarwinds_apm/support/swomarginalia/README.md b/lib/solarwinds_apm/support/swomarginalia/README.md deleted file mode 100644 index c4a9dfa1..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/README.md +++ /dev/null @@ -1,46 +0,0 @@ -# swomarginalia - -This folder contains the code that is copied from original [marginalia](https://github.com/basecamp/marginalia) - -## File structure - -```console -|-- swomarginalia -| |-- LICENSE -| |-- README.md -| |-- comment.rb -| |-- railtie.rb -| |-- load_swomarginalia.rb -| `-- swomarginalia.rb -``` - -## Modification - -### railitie.rb - -1. Moved prepend logic into load_swomarginalia.rb in case that non-rails app using activerecord - -### swomarginlia.rb - -1. Removed the alias_method to achieve function override; instead, we use prepend. -2. Added step of cleaning-up previous transparent comments - -### comment.rb - -1. Added the traceparent comment (trace string constructed based on opentelemetry) - -### load_swomarginalia.rb - -1. (new file) prepend the ActiveRecordInstrumentation to activerecord adapter - -## Example - -### Sample output of rails application - -```console -Post Load (1.1ms) SELECT `posts`.* FROM `posts` /*application=SqlcommenterRailsDemo,controller=posts,action=index,traceparent=00-a448f096d441e167d12ebd32a927c1a5-a29655a47e430119-01*/ -``` - -## License - -This project is licensed under the [MIT License](https://github.com/solarwinds/apm-ruby/blob/main/lib/solarwinds_apm/support/swomarginalia/LICENSE). diff --git a/lib/solarwinds_apm/support/swomarginalia/comment.rb b/lib/solarwinds_apm/support/swomarginalia/comment.rb deleted file mode 100644 index 299ebdfd..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/comment.rb +++ /dev/null @@ -1,206 +0,0 @@ -# frozen_string_literal: true - -require 'socket' -require 'opentelemetry-api' - -module SolarWindsAPM - module SWOMarginalia - module Comment - mattr_accessor :components, :lines_to_ignore, :prepend_comment - SWOMarginalia::Comment.components ||= [:traceparent] - # To add new components: - # Create file and load after loading solarwinds_apm, and add following: - # SolarWindsAPM::SWOMarginalia::Comment.component = [:user_defined] - - def self.update!(controller = nil) - self.marginalia_controller = controller - end - - def self.update_job!(job) - self.marginalia_job = job - end - - def self.update_adapter!(adapter) - self.marginalia_adapter = adapter - end - - def self.construct_comment - ret = +'' - components.each do |c| - component_value = send(c) - ret << "#{c}='#{component_value}'," if component_value.present? - end - ret.chop! - escape_sql_comment(ret) - end - - def self.construct_inline_comment - return nil if inline_annotations.none? - - escape_sql_comment(inline_annotations.join) - end - - def self.escape_sql_comment(str) - str = str.gsub('/*', '').gsub('*/', '') while str.include?('/*') || str.include?('*/') - str - end - - def self.clear! - self.marginalia_controller = nil - end - - def self.clear_job! - self.marginalia_job = nil - end - - def self.marginalia_controller=(controller) - Thread.current[:marginalia_controller] = controller - end - - def self.marginalia_controller - Thread.current[:marginalia_controller] - end - - def self.marginalia_job=(job) - Thread.current[:marginalia_job] = job - end - - def self.marginalia_job - Thread.current[:marginalia_job] - end - - def self.marginalia_adapter=(adapter) - Thread.current[:marginalia_adapter] = adapter - end - - def self.marginalia_adapter - Thread.current[:marginalia_adapter] - end - - def self.application - SWOMarginalia.application_name ||= if defined?(::Rails.application) - ::Rails.application.class.name.split('::').first - else - 'rails' - end - - SWOMarginalia.application_name - end - - def self.job - marginalia_job&.class&.name - end - - def self.controller - marginalia_controller.controller_name if marginalia_controller.respond_to? :controller_name - end - - def self.controller_with_namespace - marginalia_controller&.class&.name - end - - def self.action - marginalia_controller.action_name if marginalia_controller.respond_to? :action_name - end - - def self.sidekiq_job - marginalia_job['class'] if marginalia_job.respond_to?(:[]) - end - - DEFAULT_LINES_TO_IGNORE_REGEX = %r{\.rvm|/ruby/gems/|vendor/|marginalia|rbenv|monitor\.rb.*mon_synchronize} - - def self.line - SWOMarginalia::Comment.lines_to_ignore ||= DEFAULT_LINES_TO_IGNORE_REGEX - - last_line = caller.detect do |line| - line !~ SWOMarginalia::Comment.lines_to_ignore - end - return unless last_line - - root = if defined?(Rails) && Rails.respond_to?(:root) - Rails.root.to_s - elsif defined?(RAILS_ROOT) - RAILS_ROOT - else - '' - end - last_line = last_line[root.length..] if last_line.start_with? root - last_line - end - - def self.hostname - @hostname ||= Socket.gethostname - end - - def self.pid - Process.pid - end - - def self.request_id - return unless marginalia_controller.respond_to?(:request) && marginalia_controller.request.respond_to?(:uuid) - - marginalia_controller.request.uuid - end - - def self.socket - return unless connection_config.present? - - connection_config[:socket] - end - - def self.db_host - return unless connection_config.present? - - connection_config[:host] - end - - def self.database - return unless connection_config.present? - - connection_config[:database] - end - - ## - # Insert trace string as traceparent to sql statement - # Not insert if: - # there is no valid current trace context - # current trace context is not sampled - # - def self.traceparent - span_context = ::OpenTelemetry::Trace.current_span.context - return '' if span_context == ::OpenTelemetry::Trace::SpanContext::INVALID - - trace_flag = span_context.trace_flags.sampled? ? '01' : '00' - return '' if trace_flag == '00' - - format( - '00-%s-%s-%.2d', - trace_id: span_context.hex_trace_id, - span_id: span_context.hex_span_id, - trace_flags: trace_flag - ) - rescue NameError => e - SolarWindsAPM.logger.error { "[#{name}/#{__method__}] Couldn't find OpenTelemetry. Error: #{e.message}" } - end - - if Gem::Version.new(ActiveRecord::VERSION::STRING) < Gem::Version.new('6.1') - def self.connection_config - return if marginalia_adapter.pool.nil? - - marginalia_adapter.pool.spec.config - end - else - def self.connection_config - # `pool` might be a NullPool which has no db_config - return unless marginalia_adapter.pool.respond_to?(:db_config) - - marginalia_adapter.pool.db_config.configuration_hash - end - end - - def self.inline_annotations - Thread.current[:marginalia_inline_annotations] ||= [] - end - end - end -end diff --git a/lib/solarwinds_apm/support/swomarginalia/formatter.rb b/lib/solarwinds_apm/support/swomarginalia/formatter.rb deleted file mode 100644 index bc8a31b3..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/formatter.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -module SolarWindsAPM - module SWOMarginalia - module Formatter - def tag_content - comment = super - comment.tr!(':', '=') - comment - end - end - end -end - -# Prepend the Formatter module to the singleton class of ActiveRecord::QueryLogs -if defined?(ActiveRecord::QueryLogs) - class << ActiveRecord::QueryLogs - prepend SolarWindsAPM::SWOMarginalia::Formatter - end -end diff --git a/lib/solarwinds_apm/support/swomarginalia/load_swomarginalia.rb b/lib/solarwinds_apm/support/swomarginalia/load_swomarginalia.rb deleted file mode 100644 index 214f7dc8..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/load_swomarginalia.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -# © 2023 SolarWinds Worldwide, LLC. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - -require_relative 'swomarginalia' - -module SolarWindsAPM - module SWOMarginalia - module LoadSWOMarginalia - def self.insert - insert_into_active_record - insert_into_action_controller - insert_into_active_job - end - - def self.insert_into_active_job - return unless defined? ::ActiveJob::Base - - ::ActiveJob::Base.class_eval do - around_perform do |job, block| - SWOMarginalia::Comment.update_job! job - block.call - ensure - SWOMarginalia::Comment.clear_job! - end - end - end - - def self.insert_into_action_controller - return unless defined? ::ActionController::Base - - ::ActionController::Base.include SWOMarginalia::ActionControllerInstrumentation - - return unless defined? ::ActionController::API - - ::ActionController::API.include SWOMarginalia::ActionControllerInstrumentation - end - - def self.insert_into_active_record - ActiveRecord::ConnectionAdapters::Mysql2Adapter.prepend(SWOMarginalia::ActiveRecordInstrumentation) if defined? ActiveRecord::ConnectionAdapters::Mysql2Adapter - ActiveRecord::ConnectionAdapters::MysqlAdapter.prepend(SWOMarginalia::ActiveRecordInstrumentation) if defined? ActiveRecord::ConnectionAdapters::MysqlAdapter - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(SWOMarginalia::ActiveRecordInstrumentation) if defined? ActiveRecord::ConnectionAdapters::PostgreSQLAdapter - return unless defined? ActiveRecord::ConnectionAdapters::SQLite3Adapter - - return unless defined? ActiveRecord::ConnectionAdapters::SQLite3Adapter - - ActiveRecord::ConnectionAdapters::SQLite3Adapter.prepend(SWOMarginalia::ActiveRecordInstrumentation) - end - end - end -end diff --git a/lib/solarwinds_apm/support/swomarginalia/railtie.rb b/lib/solarwinds_apm/support/swomarginalia/railtie.rb deleted file mode 100644 index 65d1457b..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/railtie.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -require 'rails/railtie' -require_relative 'load_swomarginalia' - -module SolarWindsAPM - module SWOMarginalia - class Railtie < Rails::Railtie - initializer 'swomarginalia.insert' do - ActiveSupport.on_load :active_record do - ::SolarWindsAPM::SWOMarginalia::LoadSWOMarginalia.insert_into_active_record - end - - ActiveSupport.on_load :action_controller do - ::SolarWindsAPM::SWOMarginalia::LoadSWOMarginalia.insert_into_action_controller - end - - ActiveSupport.on_load :active_job do - ::SolarWindsAPM::SWOMarginalia::LoadSWOMarginalia.insert_into_active_job - end - end - end - end -end diff --git a/lib/solarwinds_apm/support/swomarginalia/swomarginalia.rb b/lib/solarwinds_apm/support/swomarginalia/swomarginalia.rb deleted file mode 100644 index 2b76275a..00000000 --- a/lib/solarwinds_apm/support/swomarginalia/swomarginalia.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -require_relative 'comment' - -module SolarWindsAPM - module SWOMarginalia - mattr_accessor :application_name - - # This ActiveRecordInstrumentation should only work for activerecord < 7.0 since after rails 7 - # this module won't be prepend to activerecord - module ActiveRecordInstrumentation - def execute(sql, *args, **options) - super(annotate_sql(sql), *args, **options) - end - - # only for postgresql adapter - def execute_and_clear(sql, *args, **options) - super(annotate_sql(sql), *args, **options) - end - - def exec_query(sql, *args, **options) - super(annotate_sql(sql), *args, **options) - end - - def exec_delete(sql, *args) - super(annotate_sql(sql), *args) - end - - def exec_update(sql, *args) - super(annotate_sql(sql), *args) - end - - def annotate_sql(sql) - SWOMarginalia::Comment.update_adapter!(self) # switch to current sql adapter - comment = SWOMarginalia::Comment.construct_comment # comment will include traceparent - if comment.present? && !sql.include?(comment) - sql = if SWOMarginalia::Comment.prepend_comment - "/*#{comment}*/ #{sql}" - else - "#{sql} /*#{comment}*/" - end - end - - inline_comment = SWOMarginalia::Comment.construct_inline_comment # this is for customized_swo_inline_annotations (user-defined value) - if inline_comment.present? && !sql.include?(inline_comment) - sql = if SWOMarginalia::Comment.prepend_comment - "/*#{inline_comment}*/ #{sql}" - else - "#{sql} /*#{inline_comment}*/" - end - end - - sql - end - - # We don't want to trace framework caches. - # Only instrument SQL that directly hits the database. - def ignore_payload?(name) - %w[SCHEMA EXPLAIN CACHE].include?(name.to_s) - end - end - - module ActionControllerInstrumentation - def self.included(instrumented_class) - instrumented_class.class_eval do - if respond_to?(:around_action) - around_action :record_query_comment - else - around_filter :record_query_comment - end - end - end - - def record_query_comment - SWOMarginalia::Comment.update!(self) - yield - ensure - SWOMarginalia::Comment.clear! - end - end - - def self.with_annotation(comment, &block) - SWOMarginalia::Comment.inline_annotations.push(comment) - yield if block.present? - ensure - SWOMarginalia::Comment.inline_annotations.pop - end - end -end diff --git a/test/minitest_helper.rb b/test/minitest_helper.rb index 68df77f3..12688979 100644 --- a/test/minitest_helper.rb +++ b/test/minitest_helper.rb @@ -54,7 +54,7 @@ def $stdout.write(string) end # Print out a headline in with the settings used in the test run -puts "\n\033[1m=== TEST RUN: #{RUBY_VERSION} #{File.basename(ENV.fetch('BUNDLE_GEMFILE', nil))} #{ENV.fetch('DBTYPE', nil)} #{ENV.fetch('TEST_PREPARED_STATEMENT', nil)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" +puts "\n\033[1m=== TEST RUN: #{RUBY_VERSION} #{File.basename(ENV.fetch('BUNDLE_GEMFILE', nil))} #{ENV.fetch('DBTYPE', nil)} #{ENV.fetch('TEST_PREPARED_STATEMENT', nil)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" unless ENV['DBO_PATCH_TEST'] ENV['RACK_ENV'] = 'test' Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new @@ -413,3 +413,30 @@ def query(sql, _options = {}) end end end + +module PG + class Connection + attr_reader :db, :user, :host, :conninfo_hash + + def initialize + @conninfo_hash = {} + end + + EXEC_ISH_METHODS = %i[ + exec + query + sync_exec + async_exec + exec_params + async_exec_params + sync_exec_params + ].freeze + + EXEC_ISH_METHODS.each do |method| + define_method method do |*args| + args[0] + end + end + end + VERSION = '999.0.0' +end diff --git a/test/patch/sw_mysql2_patch_integrate_test.rb b/test/patch/sw_mysql2_patch_integrate_test.rb index 97178c81..3b100d42 100644 --- a/test/patch/sw_mysql2_patch_integrate_test.rb +++ b/test/patch/sw_mysql2_patch_integrate_test.rb @@ -12,7 +12,7 @@ require './lib/solarwinds_apm/support' require './lib/solarwinds_apm/constants' require './lib/solarwinds_apm/oboe_init_options' -require './lib/solarwinds_apm/patch/tag_sql/swo_dbo_utils' +require './lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils' # rubocop:disable Naming/MethodName module SolarWindsAPM @@ -23,7 +23,7 @@ def self.createSpan(trans_name, domain, span_time, has_error); end # rubocop:enable Naming/MethodName describe 'mysql2 patch integrate test' do - puts "\n\033[1m=== TEST RUN: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" + puts "\n\033[1m=== TEST RUN MYSQL2 PATCH TEST: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" let(:sdk) { OpenTelemetry::SDK } let(:exporter) { sdk::Trace::Export::InMemorySpanExporter.new } diff --git a/test/patch/sw_mysql2_patch_test.rb b/test/patch/sw_mysql2_patch_test.rb index eb344405..ccfb76de 100644 --- a/test/patch/sw_mysql2_patch_test.rb +++ b/test/patch/sw_mysql2_patch_test.rb @@ -10,7 +10,7 @@ require './lib/solarwinds_apm/otel_config' describe 'mysql2 patch test' do - puts "\n\033[1m=== TEST RUN: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" + puts "\n\033[1m=== TEST RUN MYSQL2 PATCH TEST: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" it 'mysql_patch_order_test_when_tag_sql_is_false' do SolarWindsAPM::Config[:tag_sql] = false diff --git a/test/patch/sw_pg_patch_integrate_test.rb b/test/patch/sw_pg_patch_integrate_test.rb new file mode 100644 index 00000000..14f92367 --- /dev/null +++ b/test/patch/sw_pg_patch_integrate_test.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +# Copyright (c) 2023 SolarWinds, LLC. +# All rights reserved. + +require 'minitest_helper' +require './lib/solarwinds_apm/config' +require './lib/solarwinds_apm/opentelemetry' +require './lib/solarwinds_apm/support/txn_name_manager' +require './lib/solarwinds_apm/otel_config' +require './lib/solarwinds_apm/api' +require './lib/solarwinds_apm/support' +require './lib/solarwinds_apm/constants' +require './lib/solarwinds_apm/oboe_init_options' +require './lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils' + +# rubocop:disable Naming/MethodName +module SolarWindsAPM + module Span + def self.createSpan(trans_name, domain, span_time, has_error); end + end +end +# rubocop:enable Naming/MethodName + +def pg_dbo_integration_verification(sql, finished_spans) + pattern = %r{/\*traceparent='[\da-f-]+'*\*/$} + assert_match pattern, finished_spans[0].attributes['sw.query_tag'], "Doesn't match sw.query_tag" + + pattern = %r{^SELECT \* FROM ABC;\s+/\*traceparent='[\da-f-]+'*\*/$} + assert_match pattern, sql, "Sql doesn't contain traceparent" +end + +describe 'pg patch integrate test' do + puts "\n\033[1m=== TEST RUN PG PATCH TEST: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" + + let(:sdk) { OpenTelemetry::SDK } + let(:exporter) { sdk::Trace::Export::InMemorySpanExporter.new } + let(:span_processor) { sdk::Trace::Export::SimpleSpanProcessor.new(exporter) } + + it 'tag_sql_pg_integrate_test' do + require './lib/solarwinds_apm/patch/tag_sql/sw_pg_patch' + + OpenTelemetry::SDK.configure(&:use_all) + OpenTelemetry.tracer_provider.add_span_processor(span_processor) + + client_ancestors = PG::Connection.ancestors + _(client_ancestors[0]).must_equal OpenTelemetry::Instrumentation::PG::Patches::Connection + _(client_ancestors[1]).must_equal SolarWindsAPM::Patch::TagSql::SWOPgPatch + _(client_ancestors[2]).must_equal PG::Connection + + pg_client = PG::Connection.new + + args = ['SELECT * FROM ABC;', 'a1'] + + sql = pg_client.query(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + sql = pg_client.exec(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + sql = pg_client.sync_exec(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + sql = pg_client.async_exec(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + args = ['SELECT * FROM ABC;', [1]] + sql = pg_client.exec_params(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + sql = pg_client.async_exec_params(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + + sql = pg_client.sync_exec_params(*args) + finished_spans = exporter.finished_spans + pg_dbo_integration_verification(sql, finished_spans) + exporter.reset + end +end diff --git a/test/patch/sw_pg_patch_test.rb b/test/patch/sw_pg_patch_test.rb new file mode 100644 index 00000000..f7c34937 --- /dev/null +++ b/test/patch/sw_pg_patch_test.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Copyright (c) 2023 SolarWinds, LLC. +# All rights reserved. + +require 'minitest_helper' +require './lib/solarwinds_apm/config' +require './lib/solarwinds_apm/opentelemetry' +require './lib/solarwinds_apm/support/txn_name_manager' +require './lib/solarwinds_apm/otel_config' + +describe 'pg patch test' do + puts "\n\033[1m=== TEST RUN PG PATCH TEST: #{RUBY_VERSION} #{File.basename(__FILE__)} #{Time.now.strftime('%Y-%m-%d %H:%M')} ===\033[0m\n" + + it 'pg_patch_order_test_when_tag_sql_is_false' do + SolarWindsAPM::Config[:tag_sql] = false + SolarWindsAPM::OTelConfig.initialize + + client_ancestors = PG::Connection.ancestors + puts "client_ancestors: #{client_ancestors.inspect}" + _(client_ancestors[0]).must_equal OpenTelemetry::Instrumentation::PG::Patches::Connection + _(client_ancestors[1]).must_equal PG::Connection + end +end diff --git a/test/run_tests.sh b/test/run_tests.sh index d397caa3..6b6af102 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -19,7 +19,6 @@ check_status() { gemfiles=( "gemfiles/unit.gemfile" - "gemfiles/rails_6x.gemfile" ) ## @@ -82,7 +81,7 @@ fi PATCH_TEST_FILE=$(find test/patch/*_test.rb -type f) for file in $PATCH_TEST_FILE; do check_file_name=$file - BUNDLE_GEMFILE=gemfiles/test_gems.gemfile bundle exec ruby -I test $file + BUNDLE_GEMFILE=gemfiles/test_gems.gemfile DBO_PATCH_TEST=1 bundle exec ruby -I test $file check_status done diff --git a/test/support/swomarginalia/swomarginalia_test.rb b/test/support/swomarginalia/swomarginalia_test.rb deleted file mode 100644 index df4158e2..00000000 --- a/test/support/swomarginalia/swomarginalia_test.rb +++ /dev/null @@ -1,120 +0,0 @@ -# frozen_string_literal: true - -require 'minitest/autorun' -require 'mocha/minitest' -require 'rails' -require 'logger' -require 'active_record' -require 'action_controller' -require 'active_job' -require 'sidekiq' -require 'sidekiq/testing' -require 'sqlite3' -require 'action_dispatch/middleware/request_id' - -require 'minitest_helper' - -require_relative '../../../lib/solarwinds_apm/support/swomarginalia/load_swomarginalia' - -puts "Current rails version: #{Rails.version}" - -ActiveRecord::Base.establish_connection({ - adapter: 'sqlite3', - database: 'database.db' - }) - -ActiveRecord::Base.connection.execute('CREATE TABLE IF NOT EXISTS posts( first_name TEXT NOT NULL, id TEXT NOT NULL)') -ActiveRecord::Base.connection.execute('INSERT OR IGNORE INTO posts(first_name, id) VALUES("fake_name", 456)') - -class Post < ActiveRecord::Base -end - -class PostsController < ActionController::Base - def driver_only - ActiveRecord::Base.connection.execute 'select id from posts' - render body: nil - end -end - -class PostsJob < ActiveJob::Base - def perform - Post.first - end -end - -class PostsSidekiqJob - include Sidekiq::Worker - def perform - Post.first - end -end - -# has to override the traceparent for testing purpose -module SolarWindsAPM - module SWOMarginalia - module Comment - def self.traceparent - format( - '00-%s-%s-%.2d', - trace_id: '85e9b1a685e9b1a685e9b1a685e9b1a6', - span_id: '85e9b1a685e9b1a6', - trace_flags: '01' - ) - end - end - end -end - -# Has to insert after ActiveRecord defined -SolarWindsAPM::SWOMarginalia::LoadSWOMarginalia.insert - -describe 'SWOMarginaliaTestForRails6' do - before do - SolarWindsAPM::SWOMarginalia.application_name = 'rails' - @queries = [] - ActiveSupport::Notifications.subscribe 'sql.active_record' do |*args| - @queries << args.last[:sql] - end - @env = Rack::MockRequest.env_for('/') - ActiveJob::Base.queue_adapter = :inline - end - - after do - SolarWindsAPM::SWOMarginalia.application_name = nil - SolarWindsAPM::SWOMarginalia::Comment.lines_to_ignore = nil - SolarWindsAPM::SWOMarginalia::Comment.components = [:traceparent] - ActiveSupport::Notifications.unsubscribe 'sql.active_record' - end - - it 'test_query_commenting_on_sqlite3_driver_with_application_function' do - SolarWindsAPM::SWOMarginalia::Comment.components = %i[application traceparent] - Post.where(first_name: 'fake_name') - _(@queries.first).must_equal "PRAGMA table_info(\"posts\") /*application='rails',traceparent='00-85e9b1a685e9b1a685e9b1a685e9b1a6-85e9b1a685e9b1a6-01'*/" - end - - # Only ActiveRecord::Base.connection.raw_connection.prepare can do the prepare statement (the native connection) - it 'test_query_commenting_on_sqlite3_driver_with_random_chars' do - ActiveRecord::Base.connection.execute 'select id from posts /* random_char */' - _(@queries.first).must_equal "select id from posts /* random_char */ /*traceparent='00-85e9b1a685e9b1a685e9b1a685e9b1a6-85e9b1a685e9b1a6-01'*/" - end - - it 'test_query_commenting_on_sqlite3_driver_with_action' do - PostsController.action(:driver_only).call(@env) - _(@queries.first).must_equal "select id from posts /*traceparent='00-85e9b1a685e9b1a685e9b1a685e9b1a6-85e9b1a685e9b1a6-01'*/" - end - - it 'test_query_commenting_on_sqlite3_driver_with_nothing' do - SolarWindsAPM::SWOMarginalia::Comment.components = [] - ActiveRecord::Base.connection.execute 'select id from posts' - _(@queries.last).must_equal 'select id from posts' - end - - it 'test_proc_function_traceparent_for_rails_7' do - traceparent = SolarWindsAPM::SWOMarginalia::Comment.traceparent - traceparent = traceparent.split('-') - _(traceparent[0]).must_equal '00' - _(traceparent[1].size).must_equal 32 - _(traceparent[2].size).must_equal 16 - _(traceparent[3].size).must_equal 2 - end -end