From d70eedbe3144b230f7b5d13f4b98fd00cfe1a6d3 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Thu, 3 Aug 2023 14:44:52 +0200 Subject: [PATCH 1/2] Add `ddsource` to #to_log_format --- .../contrib/lograge/instrumentation.rb | 18 +---- .../semantic_logger/instrumentation.rb | 23 +----- lib/datadog/tracing/correlation.rb | 20 +++++ spec/datadog/tracing/correlation_spec.rb | 73 +++++++++++++++++++ 4 files changed, 97 insertions(+), 37 deletions(-) diff --git a/lib/datadog/tracing/contrib/lograge/instrumentation.rb b/lib/datadog/tracing/contrib/lograge/instrumentation.rb index d0d8e60a864..65a8b1915b7 100644 --- a/lib/datadog/tracing/contrib/lograge/instrumentation.rb +++ b/lib/datadog/tracing/contrib/lograge/instrumentation.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative '../../../core/logging/ext' - module Datadog module Tracing module Contrib @@ -23,21 +21,7 @@ def custom_options(event) # Retrieves trace information for current thread correlation = Tracing.correlation # merge original lambda with datadog context - - datadog_trace_log_hash = { - # Adds IDs as tags to log output - dd: { - # To preserve precision during JSON serialization, use strings for large numbers - trace_id: correlation.trace_id.to_s, - span_id: correlation.span_id.to_s, - env: correlation.env.to_s, - service: correlation.service.to_s, - version: correlation.version.to_s - }, - ddsource: Core::Logging::Ext::DD_SOURCE - } - - datadog_trace_log_hash.merge(original_custom_options) + correlation.to_h.merge(original_custom_options) end end end diff --git a/lib/datadog/tracing/contrib/semantic_logger/instrumentation.rb b/lib/datadog/tracing/contrib/semantic_logger/instrumentation.rb index 705005e00e0..63d97b69f18 100644 --- a/lib/datadog/tracing/contrib/semantic_logger/instrumentation.rb +++ b/lib/datadog/tracing/contrib/semantic_logger/instrumentation.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative '../../../core/logging/ext' - module Datadog module Tracing module Contrib @@ -23,25 +21,10 @@ def log(log, message = nil, progname = nil, &block) # Retrieves trace information for current thread correlation = Tracing.correlation - # merge original lambda with datadog context - - datadog_trace_log_hash = { - # Adds IDs as tags to log output - dd: { - # To preserve precision during JSON serialization, use strings for large numbers - trace_id: correlation.trace_id.to_s, - span_id: correlation.span_id.to_s, - env: correlation.env.to_s, - service: correlation.service.to_s, - version: correlation.version.to_s - }, - ddsource: Core::Logging::Ext::DD_SOURCE - } - # # if the user already has conflicting log_tags - # # we want them to clobber ours, because we should allow them to override - # # if needed. - log.named_tags = datadog_trace_log_hash.merge(original_named_tags) + # if the user already has conflicting log_tags + # we want them to clobber ours, because we should allow them to override if needed. + log.named_tags = correlation.to_h.merge(original_named_tags) super(log, message, progname, &block) end end diff --git a/lib/datadog/tracing/correlation.rb b/lib/datadog/tracing/correlation.rb index 7fd8edd520b..3e210697d54 100644 --- a/lib/datadog/tracing/correlation.rb +++ b/lib/datadog/tracing/correlation.rb @@ -1,5 +1,6 @@ require_relative 'utils' require_relative 'metadata/ext' +require_relative '../core/logging/ext' module Datadog module Tracing @@ -14,6 +15,7 @@ class Identifier LOG_ATTR_SPAN_ID = 'dd.span_id'.freeze LOG_ATTR_TRACE_ID = 'dd.trace_id'.freeze LOG_ATTR_VERSION = 'dd.version'.freeze + LOG_ATTR_SOURCE = 'ddsource'.freeze attr_reader \ :env, @@ -58,6 +60,23 @@ def initialize( @version = Core::Utils::SafeDup.frozen_dup(version || Datadog.configuration.version) end + def to_h + @to_h ||= { + # Adds IDs as tags to log output + dd: { + # To preserve precision during JSON serialization, use strings for large numbers + env: env.to_s, + service: service.to_s, + version: version.to_s, + trace_id: trace_id.to_s, + span_id: span_id.to_s + }, + ddsource: Core::Logging::Ext::DD_SOURCE + } + end + + # This method (#to_log_format) implements an algorithm by prefixing keys for nested values + # but the algorithm makes the constants implicit. Hence, we use it for validation during test. def to_log_format @log_format ||= begin attributes = [] @@ -66,6 +85,7 @@ def to_log_format attributes << "#{LOG_ATTR_VERSION}=#{version}" unless version.nil? attributes << "#{LOG_ATTR_TRACE_ID}=#{trace_id}" attributes << "#{LOG_ATTR_SPAN_ID}=#{span_id}" + attributes << "#{LOG_ATTR_SOURCE}=#{Core::Logging::Ext::DD_SOURCE}" attributes.join(' ') end end diff --git a/spec/datadog/tracing/correlation_spec.rb b/spec/datadog/tracing/correlation_spec.rb index 6145d1f37d5..78884aa92c8 100644 --- a/spec/datadog/tracing/correlation_spec.rb +++ b/spec/datadog/tracing/correlation_spec.rb @@ -196,6 +196,61 @@ end end + describe '#to_h' do + context 'when given values' do + let(:trace_id) { Datadog::Tracing::Utils.next_id } + let(:span_id) { Datadog::Tracing::Utils.next_id } + + it 'returns a formatted hash' do + identifier = described_class.new( + env: 'dev', + service: 'acme-api', + version: '1.0', + span_id: span_id, + trace_id: trace_id, + ) + + expect(identifier.to_h).to eq( + { + dd: { + env: 'dev', + service: 'acme-api', + version: '1.0', + trace_id: trace_id.to_s, + span_id: span_id.to_s + }, + ddsource: 'ruby' + } + ) + end + end + + context 'when given `nil`' do + it 'returns a formatted hash with default values' do + identifier = described_class.new( + env: nil, + service: nil, + version: nil, + span_id: nil, + trace_id: nil, + ) + + expect(identifier.to_h).to eq( + { + dd: { + env: 'default_env', + service: 'default-service', + version: 'default-version', + trace_id: '0', + span_id: '0', + }, + ddsource: 'ruby' + } + ) + end + end + end + describe '#to_log_format' do shared_examples_for 'a log format string' do subject(:string) { identifier.to_log_format } @@ -219,6 +274,24 @@ it 'doesn\'t have attributes without values' do is_expected.to_not match(/.*=(?=\z|\s)/) end + + RSpec::Matchers.define :be_serialized_nested_hash do |expected| + match do |actual| + result = expected.each_with_object(String.new) do |(key, value), string| + if value.is_a? Hash + value.each_pair { |k, v| string << "#{key}.#{k}=#{v} " unless v.empty? } + else + string << "#{key}=#{value} " + end + end.strip! + + actual == result + end + end + + it 'serializes a nested hash' do + is_expected.to be_serialized_nested_hash(identifier.to_h) + end end # Expect string to contain the attribute, at the beginning/end of the string, From aaea9754283f85567b4e41bafe03df5ab7360ecb Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Wed, 16 Aug 2023 10:31:21 +0200 Subject: [PATCH 2/2] Fix assertion value for env --- spec/datadog/tracing/correlation_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/tracing/correlation_spec.rb b/spec/datadog/tracing/correlation_spec.rb index 78884aa92c8..3067572d30c 100644 --- a/spec/datadog/tracing/correlation_spec.rb +++ b/spec/datadog/tracing/correlation_spec.rb @@ -238,7 +238,7 @@ expect(identifier.to_h).to eq( { dd: { - env: 'default_env', + env: 'default-env', service: 'default-service', version: 'default-version', trace_id: '0',