From 0615e9f65d930f618f9098e1fb154482dbbd8dcf Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:04:44 -0500 Subject: [PATCH 01/18] DEBUG-2334 custom ActiveRecord model serialization --- lib/datadog/di/contrib/active_record.rb | 9 + lib/datadog/di/serializer.rb | 238 ++++++++++++++--------- spec/datadog/di/serializer_helper.rb | 57 ++++++ spec/datadog/di/serializer_rails_spec.rb | 101 ++++++++++ spec/datadog/di/serializer_spec.rb | 101 ++++++---- 5 files changed, 370 insertions(+), 136 deletions(-) create mode 100644 lib/datadog/di/contrib/active_record.rb create mode 100644 spec/datadog/di/serializer_helper.rb create mode 100644 spec/datadog/di/serializer_rails_spec.rb diff --git a/lib/datadog/di/contrib/active_record.rb b/lib/datadog/di/contrib/active_record.rb new file mode 100644 index 00000000000..db817d343c7 --- /dev/null +++ b/lib/datadog/di/contrib/active_record.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value }) \ +do |serializer, value, name:, depth:| + value_to_serialize = { + attributes: value.attributes, + } + serializer.serialize_value(value_to_serialize, depth: depth ? depth - 1 : nil, type: value.class) +end diff --git a/lib/datadog/di/serializer.rb b/lib/datadog/di/serializer.rb index 2fefa0d91eb..617fde5210d 100644 --- a/lib/datadog/di/serializer.rb +++ b/lib/datadog/di/serializer.rb @@ -38,13 +38,42 @@ module DI # # @api private class Serializer - def initialize(settings, redactor) + # Third-party library integration / custom serializers. + # + # Dynamic instrumentation has limited payload sizes, and for efficiency + # reasons it is not desirable to transmit data to Datadog that will + # never contain useful information. Additionally, due to depth limits, + # desired data may not even be included in payloads when serialized + # with the default, naive serializer. Therefore, custom objects like + # ActiveRecord model instances may need custom serializers. + # + # CUSTOMER NOTE: The API for defining custom serializers is not yet + # finalized. Please create an issue at + # https://github.com/datadog/dd-trace-rb/issues describing the + # object(s) you wish to serialize so that we can ensure your use case + # will be supported as the library evolves. + # + # Note that the current implementation does not permit defining a + # serializer for a particular class, which is the simplest use case. + # This is because the library itself does not need this functionality + # yet, and it won't help for ActiveRecord models (that derive from + # a common base class but are all of different classes) or for Mongoid + # models (that do not have a common base class at all but include a + # standard Mongoid module). + @@flat_registry = [] + def self.register(condition: nil, &block) + @@flat_registry << {condition: condition, proc: block} + end + + def initialize(settings, redactor, telemetry: nil) @settings = settings @redactor = redactor + @telemetry = telemetry end attr_reader :settings attr_reader :redactor + attr_reader :telemetry # Serializes positional and keyword arguments to a method, # as obtained by a method probe. @@ -86,116 +115,135 @@ def serialize_vars(vars) # (integers, strings, arrays, hashes). # # Respects string length, collection size and traversal depth limits. - def serialize_value(value, name: nil, depth: settings.dynamic_instrumentation.max_capture_depth) - if redactor.redact_type?(value) - return {type: class_name(value.class), notCapturedReason: "redactedType"} - end + def serialize_value(value, name: nil, depth: settings.dynamic_instrumentation.max_capture_depth, type: nil) + cls = type || value.class + begin + if redactor.redact_type?(value) + return {type: class_name(cls), notCapturedReason: "redactedType"} + end - if name && redactor.redact_identifier?(name) - return {type: class_name(value.class), notCapturedReason: "redactedIdent"} - end + if name && redactor.redact_identifier?(name) + return {type: class_name(cls), notCapturedReason: "redactedIdent"} + end - serialized = {type: class_name(value.class)} - case value - when NilClass - serialized.update(isNull: true) - when Integer, Float, TrueClass, FalseClass - serialized.update(value: value.to_s) - when String, Symbol - 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 + @@flat_registry.each do |entry| + if condition = entry[:condition] and condition.call(value) + serializer_proc = entry.fetch(:proc) + return serializer_proc.call(self, value, name: nil, depth: depth) + end end - max = settings.dynamic_instrumentation.max_capture_string_length - if value.length > max - serialized.update(truncated: true, size: value.length) - value = value[0...max] + + serialized = {type: class_name(cls)} + case value + when NilClass + serialized.update(isNull: true) + when Integer, Float, TrueClass, FalseClass + serialized.update(value: value.to_s) + when Time + # This path also handles DateTime values although they do not need + # to be explicitly added to the case statement. + serialized.update(value: value.iso8601) + when Date + serialized.update(value: value.to_s) + when String, Symbol need_dup = false - end - value = value.dup if need_dup - serialized.update(value: value) - when Array - if depth < 0 - serialized.update(notCapturedReason: "depth") - else - max = settings.dynamic_instrumentation.max_capture_collection_size - if max != 0 && value.length > max - serialized.update(notCapturedReason: "collectionSize", size: value.length) - # same steep failure with array slices. - # https://github.com/soutaro/steep/issues/1219 - value = value[0...max] || [] + 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 - entries = value.map do |elt| - serialize_value(elt, depth: depth - 1) + 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 - serialized.update(elements: entries) - end - when Hash - if depth < 0 - serialized.update(notCapturedReason: "depth") - else - max = settings.dynamic_instrumentation.max_capture_collection_size - cur = 0 - entries = [] - value.each do |k, v| - if max != 0 && cur >= max + value = value.dup if need_dup + serialized.update(value: value) + when Array + if depth < 0 + serialized.update(notCapturedReason: "depth") + else + max = settings.dynamic_instrumentation.max_capture_collection_size + if max != 0 && value.length > max serialized.update(notCapturedReason: "collectionSize", size: value.length) - break + # same steep failure with array slices. + # https://github.com/soutaro/steep/issues/1219 + value = value[0...max] || [] + end + entries = value.map do |elt| + serialize_value(elt, depth: depth - 1) end - cur += 1 - entries << [serialize_value(k, depth: depth - 1), serialize_value(v, name: k, depth: depth - 1)] + serialized.update(elements: entries) + end + when Hash + if depth < 0 + serialized.update(notCapturedReason: "depth") + else + max = settings.dynamic_instrumentation.max_capture_collection_size + cur = 0 + entries = [] + value.each do |k, v| + if max != 0 && cur >= max + serialized.update(notCapturedReason: "collectionSize", size: value.length) + break + end + cur += 1 + entries << [serialize_value(k, depth: depth - 1), serialize_value(v, name: k, depth: depth - 1)] + end + serialized.update(entries: entries) end - serialized.update(entries: entries) - end - else - if depth < 0 - serialized.update(notCapturedReason: "depth") else - fields = {} - max = settings.dynamic_instrumentation.max_capture_attribute_count - cur = 0 + if depth < 0 + serialized.update(notCapturedReason: "depth") + else + fields = {} + max = settings.dynamic_instrumentation.max_capture_attribute_count + cur = 0 - # MRI and JRuby 9.4.5+ preserve instance variable definition - # order when calling #instance_variables. Previous JRuby versions - # did not preserve order and returned the variables in arbitrary - # order. - # - # The arbitrary order is problematic because 1) when there are - # fewer instance variables than capture limit, the order in which - # the variables are shown in UI will change from one capture to - # the next and generally will be arbitrary to the user, and - # 2) when there are more instance variables than capture limit, - # *which* variables are captured will also change meaning user - # looking at the UI may have "new" instance variables appear and - # existing ones disappear as they are looking at multiple captures. - # - # For consistency, we should have some kind of stable order of - # instance variables on all supported Ruby runtimes, so that the UI - # stays consistent. Given that initial implementation of Ruby DI - # does not support JRuby, we don't handle JRuby's lack of ordering - # of #instance_variables here, but if JRuby is supported in the - # future this may need to be addressed. - ivars = value.instance_variables + # MRI and JRuby 9.4.5+ preserve instance variable definition + # order when calling #instance_variables. Previous JRuby versions + # did not preserve order and returned the variables in arbitrary + # order. + # + # The arbitrary order is problematic because 1) when there are + # fewer instance variables than capture limit, the order in which + # the variables are shown in UI will change from one capture to + # the next and generally will be arbitrary to the user, and + # 2) when there are more instance variables than capture limit, + # *which* variables are captured will also change meaning user + # looking at the UI may have "new" instance variables appear and + # existing ones disappear as they are looking at multiple captures. + # + # For consistency, we should have some kind of stable order of + # instance variables on all supported Ruby runtimes, so that the UI + # stays consistent. Given that initial implementation of Ruby DI + # does not support JRuby, we don't handle JRuby's lack of ordering + # of #instance_variables here, but if JRuby is supported in the + # future this may need to be addressed. + ivars = value.instance_variables - ivars.each do |ivar| - if cur >= max - serialized.update(notCapturedReason: "fieldCount", fields: fields) - break + ivars.each do |ivar| + if cur >= max + serialized.update(notCapturedReason: "fieldCount", fields: fields) + break + end + cur += 1 + fields[ivar] = serialize_value(value.instance_variable_get(ivar), name: ivar, depth: depth - 1) end - cur += 1 - fields[ivar] = serialize_value(value.instance_variable_get(ivar), name: ivar, depth: depth - 1) + serialized.update(fields: fields) end - serialized.update(fields: fields) end + serialized + rescue => exc + telemetry&.report(exc, description: "Error serializing") + {type: class_name(cls), notSerializedReason: "#{exc}"} end - serialized end private diff --git a/spec/datadog/di/serializer_helper.rb b/spec/datadog/di/serializer_helper.rb new file mode 100644 index 00000000000..9a184c8a0d4 --- /dev/null +++ b/spec/datadog/di/serializer_helper.rb @@ -0,0 +1,57 @@ +module SerializerHelper + def define_serialize_value_cases(cases) + cases.each do |c| + value = c.fetch(:input) + var_name = c[:var_name] + + context c.fetch(:name) do + let(:value) do + if Proc === value + value.call + else + value + end + end + + let(:options) do + {name: var_name} + end + + if expected_matches = c[:expected_matches] + it "serialization matches expectation" do + expect(serialized).to match(expected_matches) + end + else + expected = c.fetch(:expected) + it "serializes exactly as specified" do + expect(serialized).to eq(expected) + end + end + end + end + end + + def default_settings + let(:settings) do + double("settings").tap do |settings| + allow(settings).to receive(:dynamic_instrumentation).and_return(di_settings) + end + end + + let(:di_settings) do + double("di settings").tap do |settings| + allow(settings).to receive(:enabled).and_return(true) + allow(settings).to receive(:propagate_all_exceptions).and_return(false) + allow(settings).to receive(:redacted_identifiers).and_return([]) + allow(settings).to receive(:redacted_type_names).and_return(%w[ + DISerializerSpecSensitiveType DISerializerSpecWildCard* + ]) + allow(settings).to receive(:max_capture_collection_size).and_return(10) + allow(settings).to receive(:max_capture_attribute_count).and_return(10) + # Reduce max capture depth to 2 from default of 3 + allow(settings).to receive(:max_capture_depth).and_return(2) + allow(settings).to receive(:max_capture_string_length).and_return(100) + end + end + end +end diff --git a/spec/datadog/di/serializer_rails_spec.rb b/spec/datadog/di/serializer_rails_spec.rb new file mode 100644 index 00000000000..cac1c831c9e --- /dev/null +++ b/spec/datadog/di/serializer_rails_spec.rb @@ -0,0 +1,101 @@ +if defined?(ActiveRecord::Base) +raise 'ok to run tests' + require "datadog/di/spec_helper" + require "datadog/di/serializer" + require_relative 'serializer_helper' + require 'active_record' + require 'sqlite3' + require "datadog/di/contrib/active_record" + + class SerializerRailsSpecTestEmptyModel < ActiveRecord::Base + end + + class SerializerRailsSpecTestBasicModel < ActiveRecord::Base + end + + RSpec.describe Datadog::DI::Serializer do + di_test + + extend SerializerHelper + + before(:all) do + @original_config = begin + if defined?(::ActiveRecord::Base.connection_db_config) + ::ActiveRecord::Base.connection_db_config + else + ::ActiveRecord::Base.connection_config + end + rescue ActiveRecord::ConnectionNotEstablished + end + + ActiveRecord::Base.establish_connection('sqlite3::memory:') + + ActiveRecord::Schema.define(version: 20161003090450) do + create_table 'serializer_rails_spec_test_empty_models', force: :cascade do |t| + end + + create_table 'serializer_rails_spec_test_basic_models', force: :cascade do |t| + t.string 'title' + t.datetime 'created_at', null: false + t.datetime 'updated_at', null: false + end + end + + end + + after(:all) do + ::ActiveRecord::Base.establish_connection(@original_config) if @original_config + end + + let(:redactor) do + Datadog::DI::Redactor.new(settings) + end + + default_settings + + let(:serializer) do + described_class.new(settings, redactor) + end + + describe "#serialize_value" do + let(:serialized) do + serializer.serialize_value(value, **options) + end + + cases = [ + {name: "AR model with no attributes", + input: -> { SerializerRailsSpecTestEmptyModel.new }, + expected: {type: "SerializerRailsSpecTestEmptyModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [[{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}]]}, + ]]}}, + {name: "AR model with empty attributes", + input: -> { SerializerRailsSpecTestBasicModel.new }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'created_at'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'updated_at'}, {type: 'NilClass', isNull: true}], + ]}, + ]]}}, + {name: "AR model with filled out attributes", + input: -> { SerializerRailsSpecTestBasicModel.new( + title: 'Hello, world!', created_at: Time.utc(2020, 1, 2), updated_at: Time.utc(2020, 1, 3)) }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'String', value: 'Hello, world!'}], + # TODO serialize Time, Date, DateTime types + [{type: 'String', value: 'created_at'}, {type: 'Time', value: '2020-01-02T00:00:00Z'}], + [{type: 'String', value: 'updated_at'}, {type: 'Time', value: '2020-01-03T00:00:00Z'}], + ]}, + ]]}}, + ] + + define_serialize_value_cases(cases) + end + end +end diff --git a/spec/datadog/di/serializer_spec.rb b/spec/datadog/di/serializer_spec.rb index 2e870d9cad3..0e24e50760b 100644 --- a/spec/datadog/di/serializer_spec.rb +++ b/spec/datadog/di/serializer_spec.rb @@ -1,5 +1,6 @@ require "datadog/di/spec_helper" require "datadog/di/serializer" +require_relative 'serializer_helper' class DISerializerSpecSensitiveType end @@ -29,30 +30,14 @@ def initialize # Should have no instance variables class DISerializerSpecTestClass; end +class DISerializerCustomExceptionTestClass; end + RSpec.describe Datadog::DI::Serializer do di_test - let(:settings) do - double("settings").tap do |settings| - allow(settings).to receive(:dynamic_instrumentation).and_return(di_settings) - end - end + extend SerializerHelper - let(:di_settings) do - double("di settings").tap do |settings| - allow(settings).to receive(:enabled).and_return(true) - allow(settings).to receive(:propagate_all_exceptions).and_return(false) - allow(settings).to receive(:redacted_identifiers).and_return([]) - allow(settings).to receive(:redacted_type_names).and_return(%w[ - DISerializerSpecSensitiveType DISerializerSpecWildCard* - ]) - allow(settings).to receive(:max_capture_collection_size).and_return(10) - allow(settings).to receive(:max_capture_attribute_count).and_return(10) - # Reduce max capture depth to 2 from default of 3 - allow(settings).to receive(:max_capture_depth).and_return(2) - allow(settings).to receive(:max_capture_string_length).and_return(100) - end - end + default_settings let(:redactor) do Datadog::DI::Redactor.new(settings) @@ -67,26 +52,6 @@ class DISerializerSpecTestClass; end serializer.serialize_value(value, **options) end - def self.define_cases(cases) - cases.each do |c| - value = c.fetch(:input) - expected = c.fetch(:expected) - var_name = c[:var_name] - - context c.fetch(:name) do - let(:value) { value } - - let(:options) do - {name: var_name} - end - - it "serializes as expected" do - expect(serialized).to eq(expected) - end - end - end - end - cases = [ {name: "nil value", input: nil, expected: {type: "NilClass", isNull: true}}, {name: "true value", input: true, expected: {type: "TrueClass", value: "true"}}, @@ -100,9 +65,19 @@ def self.define_cases(cases) expected: {type: "String", notCapturedReason: "redactedIdent"}}, {name: "variable name given and is not a redacted identifier", input: "123", var_name: "normal", expected: {type: "String", value: "123"}}, + # We can assert exact value when the time zone is UTC, + # since we don't know the local time zone ahead of time. + {name: 'Time value in UTC', input: Time.utc(2020, 1, 2, 3, 4, 5), + expected: {type: 'Time', value: '2020-01-02T03:04:05Z'}}, + {name: 'Time value in local time zone', input: Time.local(2020, 1, 2, 3, 4, 5), + expected_matches: {type: 'Time', value: %r,\A2020-01-02T03:04:05[-+]\d\d:\d\d\z, }}, + {name: 'Date value', input: Date.new(2020, 1, 2), + expected: {type: 'Date', value: '2020-01-02'}}, + {name: 'DateTime value', input: DateTime.new(2020, 1, 2, 3, 4, 5), + expected: {type: 'DateTime', value: '2020-01-02T03:04:05+00:00'}}, ] - define_cases(cases) + define_serialize_value_cases(cases) end describe "#serialize_vars" do @@ -395,4 +370,48 @@ def self.define_cases(cases) end end end + + describe '.register' do + context 'with condition' do + before do + described_class.register(condition: lambda { |value| String === value && value =~ /serializer spec hello/ }) do |serializer, value, name:, depth:| + serializer.serialize_value('replacement value') + end + end + + let(:expected) do + {type: 'String', value: 'replacement value'} + end + + it 'invokes custom serializer' do + serialized = serializer.serialize_value('serializer spec hello world') + expect(serialized).to eq(expected) + end + end + end + + context 'when serialization raises an exception' do + before do + # Register a custom serializer that will raise an exception + Datadog::DI::Serializer.register(condition: lambda { |value| DISerializerCustomExceptionTestClass === value }) do |*args| + raise "Test exception" + end + end + + describe "#serialize_value" do + let(:serialized) do + serializer.serialize_value(value, **options) + end + + cases = [ + {name: "serializes other values", input: {a: DISerializerCustomExceptionTestClass.new, b: 1}, + expected: {type: "Hash", entries: [ + [{type: 'Symbol', value: 'a'}, {type: 'DISerializerCustomExceptionTestClass', notSerializedReason: 'Test exception'}], + [{type: 'Symbol', value: 'b'}, {type: 'Integer', value: '1'}], + ]}}, + ] + + define_serialize_value_cases(cases) + end + end end From e71b9e8d50e9c841242d20509500113882494b75 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:09:18 -0500 Subject: [PATCH 02/18] standard --- spec/datadog/di/serializer_rails_spec.rb | 62 ++++++++++++------------ spec/datadog/di/serializer_spec.rb | 16 +++--- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/spec/datadog/di/serializer_rails_spec.rb b/spec/datadog/di/serializer_rails_spec.rb index cac1c831c9e..6e710d657a8 100644 --- a/spec/datadog/di/serializer_rails_spec.rb +++ b/spec/datadog/di/serializer_rails_spec.rb @@ -1,5 +1,5 @@ if defined?(ActiveRecord::Base) -raise 'ok to run tests' + raise 'ok to run tests' require "datadog/di/spec_helper" require "datadog/di/serializer" require_relative 'serializer_helper' @@ -35,12 +35,11 @@ class SerializerRailsSpecTestBasicModel < ActiveRecord::Base end create_table 'serializer_rails_spec_test_basic_models', force: :cascade do |t| - t.string 'title' + t.string 'title' t.datetime 'created_at', null: false t.datetime 'updated_at', null: false end end - end after(:all) do @@ -64,35 +63,38 @@ class SerializerRailsSpecTestBasicModel < ActiveRecord::Base cases = [ {name: "AR model with no attributes", - input: -> { SerializerRailsSpecTestEmptyModel.new }, - expected: {type: "SerializerRailsSpecTestEmptyModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [[{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}]]}, - ]]}}, + input: -> { SerializerRailsSpecTestEmptyModel.new }, + expected: {type: "SerializerRailsSpecTestEmptyModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [[{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}]]}, + ]]}}, {name: "AR model with empty attributes", - input: -> { SerializerRailsSpecTestBasicModel.new }, - expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [ - [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'title'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'created_at'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'updated_at'}, {type: 'NilClass', isNull: true}], - ]}, - ]]}}, + input: -> { SerializerRailsSpecTestBasicModel.new }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'created_at'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'updated_at'}, {type: 'NilClass', isNull: true}], + ]}, + ]]}}, {name: "AR model with filled out attributes", - input: -> { SerializerRailsSpecTestBasicModel.new( - title: 'Hello, world!', created_at: Time.utc(2020, 1, 2), updated_at: Time.utc(2020, 1, 3)) }, - expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [ - [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'title'}, {type: 'String', value: 'Hello, world!'}], - # TODO serialize Time, Date, DateTime types - [{type: 'String', value: 'created_at'}, {type: 'Time', value: '2020-01-02T00:00:00Z'}], - [{type: 'String', value: 'updated_at'}, {type: 'Time', value: '2020-01-03T00:00:00Z'}], - ]}, - ]]}}, + input: -> { + SerializerRailsSpecTestBasicModel.new( + title: 'Hello, world!', created_at: Time.utc(2020, 1, 2), updated_at: Time.utc(2020, 1, 3) + ) + }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'String', value: 'Hello, world!'}], + # TODO serialize Time, Date, DateTime types + [{type: 'String', value: 'created_at'}, {type: 'Time', value: '2020-01-02T00:00:00Z'}], + [{type: 'String', value: 'updated_at'}, {type: 'Time', value: '2020-01-03T00:00:00Z'}], + ]}, + ]]}}, ] define_serialize_value_cases(cases) diff --git a/spec/datadog/di/serializer_spec.rb b/spec/datadog/di/serializer_spec.rb index 0e24e50760b..d7bce780fe0 100644 --- a/spec/datadog/di/serializer_spec.rb +++ b/spec/datadog/di/serializer_spec.rb @@ -68,13 +68,13 @@ class DISerializerCustomExceptionTestClass; end # We can assert exact value when the time zone is UTC, # since we don't know the local time zone ahead of time. {name: 'Time value in UTC', input: Time.utc(2020, 1, 2, 3, 4, 5), - expected: {type: 'Time', value: '2020-01-02T03:04:05Z'}}, + expected: {type: 'Time', value: '2020-01-02T03:04:05Z'}}, {name: 'Time value in local time zone', input: Time.local(2020, 1, 2, 3, 4, 5), - expected_matches: {type: 'Time', value: %r,\A2020-01-02T03:04:05[-+]\d\d:\d\d\z, }}, + expected_matches: {type: 'Time', value: %r{\A2020-01-02T03:04:05[-+]\d\d:\d\d\z}}}, {name: 'Date value', input: Date.new(2020, 1, 2), - expected: {type: 'Date', value: '2020-01-02'}}, + expected: {type: 'Date', value: '2020-01-02'}}, {name: 'DateTime value', input: DateTime.new(2020, 1, 2, 3, 4, 5), - expected: {type: 'DateTime', value: '2020-01-02T03:04:05+00:00'}}, + expected: {type: 'DateTime', value: '2020-01-02T03:04:05+00:00'}}, ] define_serialize_value_cases(cases) @@ -405,10 +405,10 @@ def self.define_cases(cases) cases = [ {name: "serializes other values", input: {a: DISerializerCustomExceptionTestClass.new, b: 1}, - expected: {type: "Hash", entries: [ - [{type: 'Symbol', value: 'a'}, {type: 'DISerializerCustomExceptionTestClass', notSerializedReason: 'Test exception'}], - [{type: 'Symbol', value: 'b'}, {type: 'Integer', value: '1'}], - ]}}, + expected: {type: "Hash", entries: [ + [{type: 'Symbol', value: 'a'}, {type: 'DISerializerCustomExceptionTestClass', notSerializedReason: 'Test exception'}], + [{type: 'Symbol', value: 'b'}, {type: 'Integer', value: '1'}], + ]}}, ] define_serialize_value_cases(cases) From 3743884b0e27dac587511b55872ca3455b6223de Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:10:21 -0500 Subject: [PATCH 03/18] standard --- lib/datadog/di/serializer.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/datadog/di/serializer.rb b/lib/datadog/di/serializer.rb index 617fde5210d..511061e5dfc 100644 --- a/lib/datadog/di/serializer.rb +++ b/lib/datadog/di/serializer.rb @@ -127,7 +127,7 @@ def serialize_value(value, name: nil, depth: settings.dynamic_instrumentation.ma end @@flat_registry.each do |entry| - if condition = entry[:condition] and condition.call(value) + if (condition = entry[:condition]) && condition.call(value) serializer_proc = entry.fetch(:proc) return serializer_proc.call(self, value, name: nil, depth: depth) end @@ -242,7 +242,7 @@ def serialize_value(value, name: nil, depth: settings.dynamic_instrumentation.ma serialized rescue => exc telemetry&.report(exc, description: "Error serializing") - {type: class_name(cls), notSerializedReason: "#{exc}"} + {type: class_name(cls), notSerializedReason: exc.to_s} end end From 65f1f21fe4a73e234859778d9ecac9b07450427d Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:12:04 -0500 Subject: [PATCH 04/18] standard --- spec/datadog/di/serializer_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/datadog/di/serializer_helper.rb b/spec/datadog/di/serializer_helper.rb index 9a184c8a0d4..0559193e268 100644 --- a/spec/datadog/di/serializer_helper.rb +++ b/spec/datadog/di/serializer_helper.rb @@ -1,3 +1,5 @@ +# rubocop:disable Lint/AssignmentInCondition + module SerializerHelper def define_serialize_value_cases(cases) cases.each do |c| @@ -55,3 +57,5 @@ def default_settings end end end + +# rubocop:enable Lint/AssignmentInCondition From 38470b0dcf586618b99650117e465ca56232f148 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:16:35 -0500 Subject: [PATCH 05/18] types --- lib/datadog/di/contrib/active_record.rb | 2 +- sig/datadog/di/serializer.rbs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/datadog/di/contrib/active_record.rb b/lib/datadog/di/contrib/active_record.rb index db817d343c7..695925c649f 100644 --- a/lib/datadog/di/contrib/active_record.rb +++ b/lib/datadog/di/contrib/active_record.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value }) \ +Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value }) \ # steep:ignore do |serializer, value, name:, depth:| value_to_serialize = { attributes: value.attributes, diff --git a/sig/datadog/di/serializer.rbs b/sig/datadog/di/serializer.rbs index eb428098bc1..e827ae29a45 100644 --- a/sig/datadog/di/serializer.rbs +++ b/sig/datadog/di/serializer.rbs @@ -4,16 +4,22 @@ module Datadog @settings: untyped @redactor: untyped + + @telemetry: Core::Telemetry::Component - def initialize: (untyped settings, untyped redactor) -> void + def initialize: (untyped settings, untyped redactor, ?telemetry: Core::Telemetry::Component) -> void attr_reader settings: Datadog::Core::Configuration::Settings attr_reader redactor: Datadog::DI::Redactor + attr_reader telemetry: Core::Telemetry::Component + def serialize_args: (untyped args, untyped kwargs) -> untyped def serialize_vars: (untyped vars) -> untyped def serialize_value: (untyped value, ?name: String, ?depth: Integer) -> untyped + + def self.register: (?condition: Proc) -> void private def class_name: (untyped cls) -> untyped From 7306f4fdd286c8a8817bd400f6a371da125bbd51 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:20:57 -0500 Subject: [PATCH 06/18] types --- Steepfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Steepfile b/Steepfile index c2baeedd576..c1e8fdc1bc0 100644 --- a/Steepfile +++ b/Steepfile @@ -105,6 +105,7 @@ target :datadog do ignore 'lib/datadog/core/workers/queue.rb' ignore 'lib/datadog/core/workers/runtime_metrics.rb' ignore 'lib/datadog/di/configuration/settings.rb' + ignore 'lib/datadog/di/contrib/active_record.rb' ignore 'lib/datadog/kit/appsec/events.rb' # disabled because of https://github.com/soutaro/steep/issues/701 ignore 'lib/datadog/kit/identity.rb' # disabled because of https://github.com/soutaro/steep/issues/701 ignore 'lib/datadog/opentelemetry.rb' From 7aec268d74a1b9a04f1da6e51a388a243a9dd920 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:31:49 -0500 Subject: [PATCH 07/18] make tests work? --- Rakefile | 5 +- lib/datadog/di/contrib/active_record.rb | 4 +- spec/datadog/di/serializer_rails_spec.rb | 163 +++++++++++------------ 3 files changed, 85 insertions(+), 87 deletions(-) diff --git a/Rakefile b/Rakefile index aa67152a835..5633e23f918 100644 --- a/Rakefile +++ b/Rakefile @@ -94,7 +94,8 @@ namespace :spec do RSpec::Core::RakeTask.new(:main) do |t, args| t.pattern = 'spec/**/*_spec.rb' t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking}/**/*_spec.rb,'\ - ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb' + ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb',\ + 'spec/datadog/di/*_rails_spec.rb' t.rspec_opts = args.to_a.join(' ') end @@ -132,7 +133,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:rails) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rbx,spec/datadog/di/*_rails_spec.rb' t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\ 'semantic_logger}*_spec.rb' t.rspec_opts = args.to_a.join(' ') diff --git a/lib/datadog/di/contrib/active_record.rb b/lib/datadog/di/contrib/active_record.rb index 695925c649f..f13495d8717 100644 --- a/lib/datadog/di/contrib/active_record.rb +++ b/lib/datadog/di/contrib/active_record.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value }) \ # steep:ignore -do |serializer, value, name:, depth:| +Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value } # steep:ignore +) do |serializer, value, name:, depth:| value_to_serialize = { attributes: value.attributes, } diff --git a/spec/datadog/di/serializer_rails_spec.rb b/spec/datadog/di/serializer_rails_spec.rb index 6e710d657a8..b465e7592ca 100644 --- a/spec/datadog/di/serializer_rails_spec.rb +++ b/spec/datadog/di/serializer_rails_spec.rb @@ -1,103 +1,100 @@ -if defined?(ActiveRecord::Base) - raise 'ok to run tests' - require "datadog/di/spec_helper" - require "datadog/di/serializer" - require_relative 'serializer_helper' - require 'active_record' - require 'sqlite3' - require "datadog/di/contrib/active_record" +require 'datadog/tracing/contrib/rails/rails_helper' +require "datadog/di/spec_helper" +require "datadog/di/serializer" +require_relative 'serializer_helper' +require 'active_record' +require "datadog/di/contrib/active_record" - class SerializerRailsSpecTestEmptyModel < ActiveRecord::Base - end +class SerializerRailsSpecTestEmptyModel < ActiveRecord::Base +end - class SerializerRailsSpecTestBasicModel < ActiveRecord::Base - end +class SerializerRailsSpecTestBasicModel < ActiveRecord::Base +end - RSpec.describe Datadog::DI::Serializer do - di_test +RSpec.describe Datadog::DI::Serializer do + di_test - extend SerializerHelper + extend SerializerHelper - before(:all) do - @original_config = begin - if defined?(::ActiveRecord::Base.connection_db_config) - ::ActiveRecord::Base.connection_db_config - else - ::ActiveRecord::Base.connection_config - end - rescue ActiveRecord::ConnectionNotEstablished + before(:all) do + @original_config = begin + if defined?(::ActiveRecord::Base.connection_db_config) + ::ActiveRecord::Base.connection_db_config + else + ::ActiveRecord::Base.connection_config end + rescue ActiveRecord::ConnectionNotEstablished + end - ActiveRecord::Base.establish_connection('sqlite3::memory:') + ActiveRecord::Base.establish_connection('sqlite3::memory:') - ActiveRecord::Schema.define(version: 20161003090450) do - create_table 'serializer_rails_spec_test_empty_models', force: :cascade do |t| - end + ActiveRecord::Schema.define(version: 20161003090450) do + create_table 'serializer_rails_spec_test_empty_models', force: :cascade do |t| + end - create_table 'serializer_rails_spec_test_basic_models', force: :cascade do |t| - t.string 'title' - t.datetime 'created_at', null: false - t.datetime 'updated_at', null: false - end + create_table 'serializer_rails_spec_test_basic_models', force: :cascade do |t| + t.string 'title' + t.datetime 'created_at', null: false + t.datetime 'updated_at', null: false end end + end - after(:all) do - ::ActiveRecord::Base.establish_connection(@original_config) if @original_config - end + after(:all) do + ::ActiveRecord::Base.establish_connection(@original_config) if @original_config + end - let(:redactor) do - Datadog::DI::Redactor.new(settings) - end + let(:redactor) do + Datadog::DI::Redactor.new(settings) + end - default_settings + default_settings - let(:serializer) do - described_class.new(settings, redactor) - end + let(:serializer) do + described_class.new(settings, redactor) + end - describe "#serialize_value" do - let(:serialized) do - serializer.serialize_value(value, **options) - end + describe "#serialize_value" do + let(:serialized) do + serializer.serialize_value(value, **options) + end - cases = [ - {name: "AR model with no attributes", - input: -> { SerializerRailsSpecTestEmptyModel.new }, - expected: {type: "SerializerRailsSpecTestEmptyModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [[{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}]]}, - ]]}}, - {name: "AR model with empty attributes", - input: -> { SerializerRailsSpecTestBasicModel.new }, - expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [ - [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'title'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'created_at'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'updated_at'}, {type: 'NilClass', isNull: true}], - ]}, - ]]}}, - {name: "AR model with filled out attributes", - input: -> { - SerializerRailsSpecTestBasicModel.new( - title: 'Hello, world!', created_at: Time.utc(2020, 1, 2), updated_at: Time.utc(2020, 1, 3) - ) - }, - expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ - {type: 'Symbol', value: 'attributes'}, - {type: 'Hash', entries: [ - [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], - [{type: 'String', value: 'title'}, {type: 'String', value: 'Hello, world!'}], - # TODO serialize Time, Date, DateTime types - [{type: 'String', value: 'created_at'}, {type: 'Time', value: '2020-01-02T00:00:00Z'}], - [{type: 'String', value: 'updated_at'}, {type: 'Time', value: '2020-01-03T00:00:00Z'}], - ]}, - ]]}}, - ] + cases = [ + {name: "AR model with no attributes", + input: -> { SerializerRailsSpecTestEmptyModel.new }, + expected: {type: "SerializerRailsSpecTestEmptyModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [[{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}]]}, + ]]}}, + {name: "AR model with empty attributes", + input: -> { SerializerRailsSpecTestBasicModel.new }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'created_at'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'updated_at'}, {type: 'NilClass', isNull: true}], + ]}, + ]]}}, + {name: "AR model with filled out attributes", + input: -> { + SerializerRailsSpecTestBasicModel.new( + title: 'Hello, world!', created_at: Time.utc(2020, 1, 2), updated_at: Time.utc(2020, 1, 3) + ) + }, + expected: {type: "SerializerRailsSpecTestBasicModel", entries: [[ + {type: 'Symbol', value: 'attributes'}, + {type: 'Hash', entries: [ + [{type: 'String', value: 'id'}, {type: 'NilClass', isNull: true}], + [{type: 'String', value: 'title'}, {type: 'String', value: 'Hello, world!'}], + # TODO serialize Time, Date, DateTime types + [{type: 'String', value: 'created_at'}, {type: 'Time', value: '2020-01-02T00:00:00Z'}], + [{type: 'String', value: 'updated_at'}, {type: 'Time', value: '2020-01-03T00:00:00Z'}], + ]}, + ]]}}, + ] - define_serialize_value_cases(cases) - end + define_serialize_value_cases(cases) end end From 80de6e56c11e5dcd19ad2ac3f36b1fe85ea15177 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:36:06 -0500 Subject: [PATCH 08/18] rakefile fix --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 5633e23f918..dc6dfdcf07b 100644 --- a/Rakefile +++ b/Rakefile @@ -94,7 +94,7 @@ namespace :spec do RSpec::Core::RakeTask.new(:main) do |t, args| t.pattern = 'spec/**/*_spec.rb' t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking}/**/*_spec.rb,'\ - ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb',\ + ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb,'\ 'spec/datadog/di/*_rails_spec.rb' t.rspec_opts = args.to_a.join(' ') end From 36440f46fae00041854d3fea2d53d4638dfd58c3 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:36:32 -0500 Subject: [PATCH 09/18] rakefile fix --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index dc6dfdcf07b..20c44c87da4 100644 --- a/Rakefile +++ b/Rakefile @@ -133,7 +133,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:rails) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rbx,spec/datadog/di/*_rails_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/*_rails_spec.rb' t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\ 'semantic_logger}*_spec.rb' t.rspec_opts = args.to_a.join(' ') From a4a90c069664de6fd17cf1ad30a54ef5c8a0f55f Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 13:45:02 -0500 Subject: [PATCH 10/18] types --- Steepfile | 1 - lib/datadog/di/contrib/active_record.rb | 3 +++ sig/datadog/di/contrib/active_record.rbs | 0 sig/datadog/di/serializer.rbs | 3 ++- 4 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 sig/datadog/di/contrib/active_record.rbs diff --git a/Steepfile b/Steepfile index c1e8fdc1bc0..c2baeedd576 100644 --- a/Steepfile +++ b/Steepfile @@ -105,7 +105,6 @@ target :datadog do ignore 'lib/datadog/core/workers/queue.rb' ignore 'lib/datadog/core/workers/runtime_metrics.rb' ignore 'lib/datadog/di/configuration/settings.rb' - ignore 'lib/datadog/di/contrib/active_record.rb' ignore 'lib/datadog/kit/appsec/events.rb' # disabled because of https://github.com/soutaro/steep/issues/701 ignore 'lib/datadog/kit/identity.rb' # disabled because of https://github.com/soutaro/steep/issues/701 ignore 'lib/datadog/opentelemetry.rb' diff --git a/lib/datadog/di/contrib/active_record.rb b/lib/datadog/di/contrib/active_record.rb index f13495d8717..d83fbaf0ee1 100644 --- a/lib/datadog/di/contrib/active_record.rb +++ b/lib/datadog/di/contrib/active_record.rb @@ -2,8 +2,11 @@ Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value } # steep:ignore ) do |serializer, value, name:, depth:| + # steep thinks all of the arguments are nil here + # steep:ignore:start value_to_serialize = { attributes: value.attributes, } serializer.serialize_value(value_to_serialize, depth: depth ? depth - 1 : nil, type: value.class) + # steep:ignore:end end diff --git a/sig/datadog/di/contrib/active_record.rbs b/sig/datadog/di/contrib/active_record.rbs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/sig/datadog/di/serializer.rbs b/sig/datadog/di/serializer.rbs index e827ae29a45..acaacece168 100644 --- a/sig/datadog/di/serializer.rbs +++ b/sig/datadog/di/serializer.rbs @@ -19,7 +19,8 @@ module Datadog def serialize_vars: (untyped vars) -> untyped def serialize_value: (untyped value, ?name: String, ?depth: Integer) -> untyped - def self.register: (?condition: Proc) -> void + def self.register: (?condition: Proc) { + (serializer: Serializer, value: untyped, name: Symbol, depth: Integer) -> untyped } -> void private def class_name: (untyped cls) -> untyped From c8b099a615025b859b4962d809ac7f60eac1229e Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 16:52:02 -0500 Subject: [PATCH 11/18] redo test setup --- Rakefile | 5 ++--- .../serializer_active_record_spec.rb} | 0 2 files changed, 2 insertions(+), 3 deletions(-) rename spec/datadog/di/{serializer_rails_spec.rb => contrib/serializer_active_record_spec.rb} (100%) diff --git a/Rakefile b/Rakefile index 20c44c87da4..73dabbf6516 100644 --- a/Rakefile +++ b/Rakefile @@ -94,8 +94,7 @@ namespace :spec do RSpec::Core::RakeTask.new(:main) do |t, args| t.pattern = 'spec/**/*_spec.rb' t.exclude_pattern = 'spec/**/{contrib,benchmark,redis,auto_instrument,opentelemetry,profiling,crashtracking}/**/*_spec.rb,'\ - ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb,'\ - 'spec/datadog/di/*_rails_spec.rb' + ' spec/**/{auto_instrument,opentelemetry}_spec.rb, spec/datadog/gem_packaging_spec.rb' t.rspec_opts = args.to_a.join(' ') end @@ -133,7 +132,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:rails) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/*_rails_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/contrib/*_active_record_spec.rb' t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\ 'semantic_logger}*_spec.rb' t.rspec_opts = args.to_a.join(' ') diff --git a/spec/datadog/di/serializer_rails_spec.rb b/spec/datadog/di/contrib/serializer_active_record_spec.rb similarity index 100% rename from spec/datadog/di/serializer_rails_spec.rb rename to spec/datadog/di/contrib/serializer_active_record_spec.rb From 72c07d3a7c2ce8dc8d88e8f66230ec0da3264359 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 7 Nov 2024 17:12:14 -0500 Subject: [PATCH 12/18] fix helper require --- spec/datadog/di/contrib/serializer_active_record_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/di/contrib/serializer_active_record_spec.rb b/spec/datadog/di/contrib/serializer_active_record_spec.rb index b465e7592ca..baa73a70642 100644 --- a/spec/datadog/di/contrib/serializer_active_record_spec.rb +++ b/spec/datadog/di/contrib/serializer_active_record_spec.rb @@ -1,7 +1,7 @@ require 'datadog/tracing/contrib/rails/rails_helper' require "datadog/di/spec_helper" require "datadog/di/serializer" -require_relative 'serializer_helper' +require_relative '../serializer_helper' require 'active_record' require "datadog/di/contrib/active_record" From 67076306d5fae438637df0dfdecc8d17cf64643a Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 8 Nov 2024 11:20:49 -0500 Subject: [PATCH 13/18] move spec again --- Rakefile | 2 +- .../di/contrib/{ => rails}/serializer_active_record_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename spec/datadog/di/contrib/{ => rails}/serializer_active_record_spec.rb (98%) diff --git a/Rakefile b/Rakefile index 73dabbf6516..8fe0acb1861 100644 --- a/Rakefile +++ b/Rakefile @@ -132,7 +132,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:rails) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/contrib/*_active_record_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/contrib/rails/**/*_spec.rb' t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\ 'semantic_logger}*_spec.rb' t.rspec_opts = args.to_a.join(' ') diff --git a/spec/datadog/di/contrib/serializer_active_record_spec.rb b/spec/datadog/di/contrib/rails/serializer_active_record_spec.rb similarity index 98% rename from spec/datadog/di/contrib/serializer_active_record_spec.rb rename to spec/datadog/di/contrib/rails/serializer_active_record_spec.rb index baa73a70642..f93c80c8a8f 100644 --- a/spec/datadog/di/contrib/serializer_active_record_spec.rb +++ b/spec/datadog/di/contrib/rails/serializer_active_record_spec.rb @@ -1,7 +1,7 @@ require 'datadog/tracing/contrib/rails/rails_helper' require "datadog/di/spec_helper" require "datadog/di/serializer" -require_relative '../serializer_helper' +require_relative '../../serializer_helper' require 'active_record' require "datadog/di/contrib/active_record" From a86a69ee8ee0ba30f4d17d168af53bbfc7f30211 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 8 Nov 2024 11:21:47 -0500 Subject: [PATCH 14/18] standard --- lib/datadog/di/contrib/active_record.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/datadog/di/contrib/active_record.rb b/lib/datadog/di/contrib/active_record.rb index d83fbaf0ee1..d929cde49ed 100644 --- a/lib/datadog/di/contrib/active_record.rb +++ b/lib/datadog/di/contrib/active_record.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true -Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value } # steep:ignore -) do |serializer, value, name:, depth:| +Datadog::DI::Serializer.register(condition: lambda { |value| ActiveRecord::Base === value }) do |serializer, value, name:, depth:| # steep:ignore # steep thinks all of the arguments are nil here # steep:ignore:start value_to_serialize = { From 50f7a6eb3b44728ba61a7af4a93d470fe7178e0e Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 8 Nov 2024 11:59:00 -0500 Subject: [PATCH 15/18] maybe this is the ticket --- Rakefile | 4 ++-- .../{rails => active_record}/serializer_active_record_spec.rb | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename spec/datadog/di/contrib/{rails => active_record}/serializer_active_record_spec.rb (100%) diff --git a/Rakefile b/Rakefile index 8fe0acb1861..46c2416fedc 100644 --- a/Rakefile +++ b/Rakefile @@ -132,7 +132,7 @@ namespace :spec do desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(:rails) do |t, args| - t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb,spec/datadog/di/contrib/rails/**/*_spec.rb' + t.pattern = 'spec/datadog/tracing/contrib/rails/**/*_spec.rb' t.exclude_pattern = 'spec/datadog/tracing/contrib/rails/**/*{active_job,disable_env,redis_cache,auto_instrument,'\ 'semantic_logger}*_spec.rb' t.rspec_opts = args.to_a.join(' ') @@ -279,7 +279,7 @@ namespace :spec do ].each do |contrib| desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(contrib) do |t, args| - t.pattern = "spec/datadog/tracing/contrib/#{contrib}/**/*_spec.rb" + t.pattern = "spec/datadog/tracing/contrib/#{contrib}/**/*_spec.rb,spec/datadog/di/contrib/#{contrib}/**/*_spec.rb" t.rspec_opts = args.to_a.join(' ') end end diff --git a/spec/datadog/di/contrib/rails/serializer_active_record_spec.rb b/spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb similarity index 100% rename from spec/datadog/di/contrib/rails/serializer_active_record_spec.rb rename to spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb From f12ae3ede763e0b54db8abfbc9cd219988317583 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sun, 10 Nov 2024 00:15:08 -0500 Subject: [PATCH 16/18] create a new appraisal configuration --- Matrixfile | 5 ++++- Rakefile | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Matrixfile b/Matrixfile index 56a71886cf7..fe3c0548b57 100644 --- a/Matrixfile +++ b/Matrixfile @@ -285,7 +285,10 @@ 'graphql-2.1' => '❌ 2.5 / ❌ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', 'graphql-2.0' => '✅ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', 'graphql-1.13' => '❌ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ✅ jruby', - } + }, + 'di:active_record' => { + 'rails61-mysql2' => '❌ 2.5 / ✅ 2.6 / ✅ 2.7 / ✅ 3.0 / ✅ 3.1 / ✅ 3.2 / ✅ 3.3 / ✅ 3.4 / ❌ jruby', + }, }.each_with_object({}) do |(tasks, spec_metadata), hash| # Explode arrays of task names into individual tasks # e.g. ['rails', 'railsdisableenv'] => {'...'} becomes 'rails7' => {'...'}, 'railsdisableenv7' => {'...'} diff --git a/Rakefile b/Rakefile index 46c2416fedc..003f12f4f09 100644 --- a/Rakefile +++ b/Rakefile @@ -314,6 +314,14 @@ namespace :spec do task appsec: [:'appsec:all'] + namespace :di do + desc '' # "Explicitly hiding from `rake -T`" + RSpec::Core::RakeTask.new(:active_record) do |t, args| + t.pattern = 'spec/datadog/di/contrib/active_record/**/*_spec.rb' + t.rspec_opts = args.to_a.join(' ') + end + end + namespace :profiling do task all: [:main, :ractors] From e78df735d301fcc764583826207d47f0dcd8d401 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sun, 10 Nov 2024 00:21:13 -0500 Subject: [PATCH 17/18] use mysql since this is what other tests use --- .../serializer_active_record_spec.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb b/spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb index f93c80c8a8f..df6cf17f317 100644 --- a/spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb +++ b/spec/datadog/di/contrib/active_record/serializer_active_record_spec.rb @@ -26,7 +26,7 @@ class SerializerRailsSpecTestBasicModel < ActiveRecord::Base rescue ActiveRecord::ConnectionNotEstablished end - ActiveRecord::Base.establish_connection('sqlite3::memory:') + ActiveRecord::Base.establish_connection(mysql_connection_string) ActiveRecord::Schema.define(version: 20161003090450) do create_table 'serializer_rails_spec_test_empty_models', force: :cascade do |t| @@ -40,6 +40,19 @@ class SerializerRailsSpecTestBasicModel < ActiveRecord::Base end end + def mysql + { + database: ENV.fetch('TEST_MYSQL_DB', 'mysql'), + host: ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1'), + password: ENV.fetch('TEST_MYSQL_ROOT_PASSWORD', 'root'), + port: ENV.fetch('TEST_MYSQL_PORT', '3306') + } + end + + def mysql_connection_string + "mysql2://root:#{mysql[:password]}@#{mysql[:host]}:#{mysql[:port]}/#{mysql[:database]}" + end + after(:all) do ::ActiveRecord::Base.establish_connection(@original_config) if @original_config end From e5ae3e6c29f7d3e93b38b09e61a5cfe57dd21787 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sun, 10 Nov 2024 00:21:38 -0500 Subject: [PATCH 18/18] remove from tracing tests --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 003f12f4f09..08d21a2cfdc 100644 --- a/Rakefile +++ b/Rakefile @@ -279,7 +279,7 @@ namespace :spec do ].each do |contrib| desc '' # "Explicitly hiding from `rake -T`" RSpec::Core::RakeTask.new(contrib) do |t, args| - t.pattern = "spec/datadog/tracing/contrib/#{contrib}/**/*_spec.rb,spec/datadog/di/contrib/#{contrib}/**/*_spec.rb" + t.pattern = "spec/datadog/tracing/contrib/#{contrib}/**/*_spec.rb" t.rspec_opts = args.to_a.join(' ') end end