DEBUG-2334 duplicate mutable values when serializing for dynamic inst…

lib/datadog/di/serializer.rb

@@ -26,6 +26,16 @@ module DI
     # All serialization methods take the names of the variables being
     # serialized in order to be able to redact values.
     #
+    # The result of serialization should not reference parameter values when
+    # the values are mutable (currently, this only applies to string values).
+    # Serializer will duplicate such mutable values, so that if method
+    # arguments are captured at entry and then modified during method execution,
+    # the serialized values from entry are correctly preserved.
+    # Alternatively, we could pass a parameter to the serialization methods
+    # which would control whether values are duplicated. This may be more
+    # efficient but there would be additional overhead from passing this
+    # parameter all the time and the API would get more complex.
+    #
     # @api private
     class Serializer
       def initialize(settings, redactor)
@@ -92,12 +102,24 @@ def serialize_value(value, name: nil, depth: settings.dynamic_instrumentation.ma
         when Integer, Float, TrueClass, FalseClass
           serialized.update(value: value.to_s)
         when String, Symbol
-          value = value.to_s
+          need_dup = false
+          value = if String === value
+            # This is the only place where we duplicate the value, currently.
+            # All other values are immutable primitives (e.g. numbers).
+            # However, do not duplicate if the string is frozen, or if
+            # it is later truncated.
+            need_dup = !value.frozen?
+            value
+          else
+            value.to_s
+          end
           max = settings.dynamic_instrumentation.max_capture_string_length
           if value.length > max
             serialized.update(truncated: true, size: value.length)
             value = value[0...max]
+            need_dup = false
           end
+          value = value.dup if need_dup
           serialized.update(value: value)
         when Array
           if depth < 0

spec/datadog/di/serializer_spec.rb

@@ -320,5 +320,79 @@ def self.define_cases(cases)
         end
       end
     end
+
+    context 'when positional arg is mutated' do
+      let(:args) do
+        ['hello', 'world']
+      end
+
+      let(:kwargs) { {} }
+
+      it 'preserves original value' do
+        serialized
+
+        args.first.gsub!('hello', 'bye')
+
+        expect(serialized).to eq(
+          arg1: {type: 'String', value: 'hello'},
+          arg2: {type: 'String', value: 'world'},
+        )
+      end
+    end
+
+    context 'when keyword arg is mutated' do
+      let(:args) do
+        []
+      end
+
+      let(:kwargs) do
+        {foo: 'bar'}
+      end
+
+      it 'preserves original value' do
+        serialized
+
+        kwargs[:foo].gsub!('bar', 'bye')
+
+        expect(serialized).to eq(
+          foo: {type: 'String', value: 'bar'},
+        )
+      end
+    end
+
+    context 'when positional arg is frozen' do
+      let(:frozen_string) { 'hello'.freeze }
+
+      let(:args) do
+        [frozen_string, 'world']
+      end
+
+      let(:kwargs) { {} }
+
+      it 'serializes without duplication' do
+        expect(serialized).to eq(
+          arg1: {type: 'String', value: 'hello'},
+          arg2: {type: 'String', value: 'world'},
+        )
+
+        expect(serialized[:arg1][:value]).to be frozen_string
+      end
+    end
+
+    context 'when keyword arg is frozen' do
+      let(:frozen_string) { 'hello'.freeze }
+
+      let(:args) { [] }
+
+      let(:kwargs) { {foo: frozen_string} }
+
+      it 'serializes without duplication' do
+        expect(serialized).to eq(
+          foo: {type: 'String', value: 'hello'},
+        )
+
+        expect(serialized[:foo][:value]).to be frozen_string
+      end
+    end
   end
 end