Merge pull request #819 from appsignal/mongo-sanitization

Improve MongoDB query sanitization

CHANGELOG.md

@@ -1,5 +1,14 @@
 # AppSignal for Ruby gem Changelog
 
+## 3.0.21.alpha.1
+
+### Changed
+
+- [f19d9dcc](https://github.com/appsignal/appsignal-ruby/commit/f19d9dcc1c00103f5dc92951481becf4d4ade39e) patch - The MongoDB query sanitization now shows all the attributes in the query at all levels.
+  Only the actual values are filtered with a `?` character. Less MongoDB queries are now marked
+  as N+1 queries when they weren't the exact same query. This increases the number of unique events
+  AppSignal tracks for MongoDB queries.
+
 ## 3.0.20
 
 ### Added

lib/appsignal/event_formatter/mongo_ruby_driver/query_formatter.rb

@@ -21,28 +21,28 @@ class QueryFormatter
           },
           "insert" => {
             "insert"    => :allow,
-            "documents" => :deny_array,
+            "documents" => :sanitize_document,
             "ordered"   => :allow
           },
           "update" => {
             "update"  => :allow,
-            "updates" => :sanitize_bulk,
+            "updates" => :sanitize_document,
             "ordered" => :allow
           },
           "findandmodify" => {
             "findandmodify" => :allow,
             "query"         => :sanitize_document,
-            "update"        => :deny_array,
+            "update"        => :sanitize_document,
             "new"           => :allow
           },
           "delete" => {
             "delete" => :allow,
-            "deletes" => :sanitize_bulk,
+            "deletes" => :sanitize_document,
             "ordered" => :allow
           },
           "bulk" => {
             "q"      => :sanitize_document,
-            "u"      => :deny_array,
+            "u"      => :sanitize_document,
             "limit"  => :allow,
             "multi"  => :allow,
             "upsert" => :allow
@@ -68,20 +68,9 @@ def self.format(strategy, command)
         # Applies strategy on hash values based on keys
         def self.apply_strategy(strategy, val)
           case strategy
-          when :allow      then val
-          when :deny       then "?"
-          when :deny_array then "[?]"
+          when :allow then val
           when :sanitize_document
-            Appsignal::Utils::QueryParamsSanitizer.sanitize(val, true, :mongodb)
-          when :sanitize_bulk
-            if val.length > 1
-              [
-                format(:bulk, val.first),
-                "[...]"
-              ]
-            else
-              val.map { |v| format(:bulk, v) }
-            end
+            Appsignal::Utils::QueryParamsSanitizer.sanitize(val, false, :mongodb)
           else "?"
           end
         end

lib/appsignal/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Appsignal
-  VERSION = "3.0.20".freeze
+  VERSION = "3.0.21.alpha.1".freeze
 end

spec/lib/appsignal/event_formatter/mongo_ruby_driver/query_formatter_spec.rb

@@ -51,57 +51,27 @@
       end
     end
 
-    context "when strategy is deny" do
-      let(:strategy) { :deny }
-      let(:value)    { { "_id" => 1 } }
-
-      it "should return a '?'" do
-        expect(formatter.apply_strategy(strategy, value)).to eql("?")
-      end
-    end
-
-    context "when strategy is deny_array" do
-      let(:strategy) { :deny_array }
-      let(:value)    { { "_id" => 1 } }
-
-      it "should return a sanitized array string" do
-        expect(formatter.apply_strategy(strategy, value)).to eql("[?]")
-      end
-    end
-
     context "when strategy is sanitize_document" do
       let(:strategy) { :sanitize_document }
-      let(:value)    { { "_id" => 1 } }
-
-      it "should return a sanitized document" do
-        expect(formatter.apply_strategy(strategy, value)).to eql("_id" => "?")
-      end
-    end
-
-    context "when strategy is sanitize_bulk" do
-      let(:strategy) { :sanitize_bulk }
-      let(:value)    { [{ "q" => { "_id" => 1 }, "u" => [{ "foo" => "bar" }] }] }
-
-      it "should return an array of sanitized bulk documents" do
-        expect(formatter.apply_strategy(strategy, value)).to eql([
-          { "q" => { "_id" => "?" }, "u" => "[?]" }
-        ])
+      let(:value) do
+        {
+          "_id" => 1,
+          "authors" => [
+            { "name" => "BarBaz" },
+            { "name" => "FooBar" },
+            { "name" => "BarFoo", "surname" => "Baz" }
+          ]
+        }
       end
 
-      context "when bulk has more than one update" do
-        let(:value) do
-          [
-            { "q" => { "_id" => 1 }, "u" => [{ "foo" => "bar" }] },
-            { "q" => { "_id" => 2 }, "u" => [{ "foo" => "baz" }] }
+      it "should return a sanitized document" do
+        expect(formatter.apply_strategy(strategy, value)).to eql(
+          "_id" => "?",
+          "authors" => [
+            { "name" => "?" },
+            { "name" => "?", "surname" => "?" }
           ]
-        end
-
-        it "should return only the first value of sanitized bulk documents" do
-          expect(formatter.apply_strategy(strategy, value)).to eql([
-            { "q" => { "_id" => "?" }, "u" => "[?]" },
-            "[...]"
-          ])
-        end
+        )
       end
     end
 

spec/lib/appsignal/utils/query_params_sanitizer_spec.rb

@@ -117,15 +117,15 @@
       context "when value is an array" do
         let(:value) { %w[foo bar] }
 
-        it "should sanitize all hash values with a single questionmark" do
+        it "should sanitize all hash values with a single question mark" do
           expect(subject).to eq(["?"])
         end
       end
 
       context "when value is a mixed array" do
         let(:value) { [nil, "foo", "bar"] }
 
-        it "should sanitize all hash values with a single questionmark" do
+        it "should sanitize all hash values with a single question mark" do
           expect(subject).to eq(["?"])
         end
       end