Merge pull request #170 from solarwinds/NH-94970

NH-94970: dbo integration for psql

.github/workflows/ci-markdown-link.yml

@@ -9,10 +9,10 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      # act -j markdown-link-check --container-architecture linux/arm64
-      - name: "Markdown Link Check"
-        uses: gaurav-nelson/github-action-markdown-link-check@v1
+      # equivalent cli: linkspector check
+      - name: Run linkspector
+        uses: umbrelladocs/action-linkspector@v1
         with:
-          config-file: '.markdown-link-check.json'
-          use-quiet-mode: 'yes'
-          use-verbose-mode: 'yes'
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          reporter: github-pr-review
+          fail_on_error: true

CONFIGURATION.md

@@ -165,24 +165,19 @@ 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:
+> [!NOTE]  
+> This feature currently does not support prepared statements. Active Record by default enables prepared statements for the PostgreSQL adapter (`postgresql`), to use this feature you can explicitly disable it as shown below. Please evaluate the impact of disabling prepared statements on your system before proceeding.
 
-```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
 

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):

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
 

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

lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module SolarWindsAPM
+  module Patch
+    module TagSql
+      module SWODboUtils
+        def self.annotate_span_and_sql(sql)
+          return sql if sql.to_s.empty?
+
+          current_span = ::OpenTelemetry::Trace.current_span
+
+          annotated_sql = ''
+          if current_span.context.trace_flags.sampled?
+            traceparent = SolarWindsAPM::Utils.traceparent_from_context(current_span.context)
+            annotated_traceparent = "/*traceparent='#{traceparent}'*/"
+            current_span.add_attributes({ 'sw.query_tag' => annotated_traceparent })
+            annotated_sql = "#{sql} #{annotated_traceparent}"
+          else
+            annotated_sql = sql
+          end
+
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] annotated_sql: #{annotated_sql}" }
+          annotated_sql
+        rescue StandardError => e
+          SolarWindsAPM.logger.error { "[#{self.class}/#{__method__}] Failed to annotated sql. Error: #{e.message}" }
+          sql
+        end
+      end
+    end
+  end
+end

lib/solarwinds_apm/patch/tag_sql/sw_mysql2_patch.rb

@@ -11,18 +11,7 @@ module Patch
     module TagSql
       module SWOMysql2Patch
         def query(sql, options = {})
-          current_span = ::OpenTelemetry::Trace.current_span
-
-          annotated_sql = ''
-          if current_span.context.trace_flags.sampled?
-            traceparent = SolarWindsAPM::Utils.traceparent_from_context(current_span.context)
-            annotated_traceparent = "/*traceparent='#{traceparent}'*/"
-            current_span.add_attributes({ 'sw.query_tag' => annotated_traceparent })
-            annotated_sql = "#{sql} #{annotated_traceparent}"
-          else
-            annotated_sql = sql
-          end
-
+          annotated_sql = ::SolarWindsAPM::Patch::TagSql::SWODboUtils.annotate_span_and_sql(sql)
           super(annotated_sql, options)
         end
       end

lib/solarwinds_apm/patch/tag_sql/sw_pg_patch.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module SolarWindsAPM
+  module Patch
+    module TagSql
+      module SWOPgPatch
+        # We target operations covered by the upstream pg instrumentation.
+        # These are all alike in that they will have a SQL
+        # statement as the first parameter, and they are all
+        # non-prepared statement execute.
+        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|
+            annotated_sql = ::SolarWindsAPM::Patch::TagSql::SWODboUtils.annotate_span_and_sql(args[0])
+            args[0] = annotated_sql
+            super(*args)
+          end
+        end
+      end
+    end
+  end
+end
+
+# need to prepend before pg instrumentation patch itself
+# upstream instrumentation -> our patch -> original function
+PG::Connection.prepend(SolarWindsAPM::Patch::TagSql::SWOPgPatch) if defined?(PG::Connection)

lib/solarwinds_apm/patch/tag_sql_patch.rb

@@ -6,4 +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'