From 095831880740b7a96a4ea872b48475201d235493 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Fri, 22 Nov 2024 15:10:09 -0800 Subject: [PATCH 1/2] Telemetry: Add environment variable to disable logs --- lib/datadog/core/configuration/settings.rb | 10 ++++++ lib/datadog/core/telemetry/component.rb | 11 +++++-- lib/datadog/core/telemetry/ext.rb | 1 + sig/datadog/core/telemetry/component.rbs | 3 +- sig/datadog/core/telemetry/ext.rbs | 1 + sig/datadog/core/utils/forking.rbs | 10 +++--- .../core/configuration/components_spec.rb | 17 +++++++--- .../core/configuration/settings_spec.rb | 33 +++++++++++++++++++ spec/datadog/core/telemetry/component_spec.rb | 25 ++++++++------ 9 files changed, 89 insertions(+), 22 deletions(-) diff --git a/lib/datadog/core/configuration/settings.rb b/lib/datadog/core/configuration/settings.rb index eb0386b3004..a2431bb0049 100644 --- a/lib/datadog/core/configuration/settings.rb +++ b/lib/datadog/core/configuration/settings.rb @@ -863,6 +863,16 @@ def initialize(*_) o.type :float o.default 1.0 end + + # Enable log collection for telemetry. Log collection only works when telemetry is enabled and + # logs are enabled. + # @default `DD_TELEMETRY_LOG_COLLECTION_ENABLED` environment variable, otherwise `true`. + # @return [Boolean] + option :log_collection_enabled do |o| + o.type :bool + o.env Core::Telemetry::Ext::ENV_LOG_COLLECTION + o.default true + end end # Remote configuration diff --git a/lib/datadog/core/telemetry/component.rb b/lib/datadog/core/telemetry/component.rb index 6959977a2ee..575021f118e 100644 --- a/lib/datadog/core/telemetry/component.rb +++ b/lib/datadog/core/telemetry/component.rb @@ -52,6 +52,7 @@ def self.build(settings, agent_settings, logger) metrics_aggregation_interval_seconds: settings.telemetry.metrics_aggregation_interval_seconds, dependency_collection: settings.telemetry.dependency_collection, shutdown_timeout_seconds: settings.telemetry.shutdown_timeout_seconds, + log_collection_enabled: settings.telemetry.log_collection_enabled ) end @@ -67,10 +68,11 @@ def initialize( http_transport:, shutdown_timeout_seconds:, enabled: true, - metrics_enabled: true + metrics_enabled: true, + log_collection_enabled: true ) @enabled = enabled - @stopped = false + @log_collection_enabled = log_collection_enabled @metrics_manager = MetricsManager.new( enabled: enabled && metrics_enabled, @@ -86,6 +88,9 @@ def initialize( dependency_collection: dependency_collection, shutdown_timeout: shutdown_timeout_seconds ) + + @stopped = false + @worker.start end @@ -114,7 +119,7 @@ def integrations_change! end def log!(event) - return unless @enabled || forked? + return if !@enabled || forked? || !@log_collection_enabled @worker.enqueue(event) end diff --git a/lib/datadog/core/telemetry/ext.rb b/lib/datadog/core/telemetry/ext.rb index 74aced512a0..87affa0cb28 100644 --- a/lib/datadog/core/telemetry/ext.rb +++ b/lib/datadog/core/telemetry/ext.rb @@ -13,6 +13,7 @@ module Ext ENV_INSTALL_TYPE = 'DD_INSTRUMENTATION_INSTALL_TYPE' ENV_INSTALL_TIME = 'DD_INSTRUMENTATION_INSTALL_TIME' ENV_AGENTLESS_URL_OVERRIDE = 'DD_TELEMETRY_AGENTLESS_URL' + ENV_LOG_COLLECTION = 'DD_TELEMETRY_LOG_COLLECTION_ENABLED' end end end diff --git a/sig/datadog/core/telemetry/component.rbs b/sig/datadog/core/telemetry/component.rbs index 2d14a670d17..eaf4fb315b0 100644 --- a/sig/datadog/core/telemetry/component.rbs +++ b/sig/datadog/core/telemetry/component.rbs @@ -3,6 +3,7 @@ module Datadog module Telemetry class Component @enabled: bool + @log_collection_enabled: bool @stopped: bool @metrics_manager: Datadog::Core::Telemetry::MetricsManager @worker: Datadog::Core::Telemetry::Worker @@ -15,7 +16,7 @@ module Datadog def self.build: (untyped settings, Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings agent_settings, Datadog::Core::Logger logger) -> Component - def initialize: (http_transport: Datadog::Core::Telemetry::Http::Transport, heartbeat_interval_seconds: Float, metrics_aggregation_interval_seconds: Float, dependency_collection: bool, ?enabled: bool, ?metrics_enabled: bool, shutdown_timeout_seconds: Float | Integer) -> void + def initialize: (http_transport: Datadog::Core::Telemetry::Http::Transport, heartbeat_interval_seconds: Float, metrics_aggregation_interval_seconds: Float, dependency_collection: bool, ?enabled: bool, ?metrics_enabled: bool, shutdown_timeout_seconds: Float | Integer, ?log_collection_enabled: bool) -> void def disable!: () -> void diff --git a/sig/datadog/core/telemetry/ext.rbs b/sig/datadog/core/telemetry/ext.rbs index 05f860a21a3..55df81aad79 100644 --- a/sig/datadog/core/telemetry/ext.rbs +++ b/sig/datadog/core/telemetry/ext.rbs @@ -4,6 +4,7 @@ module Datadog module Ext ENV_DEPENDENCY_COLLECTION: ::String ENV_ENABLED: ::String + ENV_LOG_COLLECTION: ::String ENV_METRICS_ENABLED: ::String ENV_HEARTBEAT_INTERVAL: ::String ENV_METRICS_AGGREGATION_INTERVAL: ::String diff --git a/sig/datadog/core/utils/forking.rbs b/sig/datadog/core/utils/forking.rbs index 0e2cf0d8524..9cb8b00c744 100644 --- a/sig/datadog/core/utils/forking.rbs +++ b/sig/datadog/core/utils/forking.rbs @@ -2,17 +2,19 @@ module Datadog module Core module Utils module Forking + @fork_pid: Integer + def self.included: (untyped base) -> untyped def self.extended: (untyped base) -> untyped - def after_fork!: () { () -> untyped } -> untyped + def after_fork!: () { () -> untyped } -> bool - def forked?: () -> untyped + def forked?: () -> bool - def update_fork_pid!: () -> untyped + def update_fork_pid!: () -> void - def fork_pid: () -> untyped + def fork_pid: () -> Integer module ClassExtensions def initialize: (*untyped args) { () -> untyped } -> untyped diff --git a/spec/datadog/core/configuration/components_spec.rb b/spec/datadog/core/configuration/components_spec.rb index 7d7efe78180..ae44c42f12b 100644 --- a/spec/datadog/core/configuration/components_spec.rb +++ b/spec/datadog/core/configuration/components_spec.rb @@ -232,11 +232,14 @@ { enabled: enabled, http_transport: an_instance_of(Datadog::Core::Telemetry::Http::Transport), metrics_enabled: metrics_enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, - dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds } + dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, + log_collection_enabled: log_collection_enabled, + } end let(:enabled) { true } let(:agentless_enabled) { false } let(:metrics_enabled) { true } + let(:log_collection_enabled) { true } let(:heartbeat_interval_seconds) { 60 } let(:metrics_aggregation_interval_seconds) { 10 } let(:shutdown_timeout_seconds) { 1.0 } @@ -262,7 +265,9 @@ { enabled: false, http_transport: an_instance_of(Datadog::Core::Telemetry::Http::Transport), metrics_enabled: false, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, - dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds } + dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, + log_collection_enabled: true, + } end let(:agent_settings) do instance_double( @@ -287,7 +292,9 @@ { enabled: enabled, http_transport: transport, metrics_enabled: metrics_enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, - dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds } + dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, + log_collection_enabled: log_collection_enabled, + } end before do @@ -306,7 +313,9 @@ { enabled: false, http_transport: transport, metrics_enabled: false, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, - dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds } + dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, + log_collection_enabled: true, + } end it 'does not enable telemetry when agentless mode requested but api key is not present' do diff --git a/spec/datadog/core/configuration/settings_spec.rb b/spec/datadog/core/configuration/settings_spec.rb index 8bc17004801..1d7bb6b730b 100644 --- a/spec/datadog/core/configuration/settings_spec.rb +++ b/spec/datadog/core/configuration/settings_spec.rb @@ -1766,6 +1766,39 @@ end end + describe '#log_collection_enabled' do + subject(:log_collection_enabled) { settings.telemetry.log_collection_enabled } + let(:env_var_name) { 'DD_TELEMETRY_LOG_COLLECTION_ENABLED' } + + context 'when DD_TELEMETRY_LOG_COLLECTION_ENABLED' do + context 'is not defined' do + let(:env_var_value) { nil } + + it { is_expected.to be true } + end + + [true, false].each do |value| + context "is defined as #{value}" do + let(:env_var_value) { value.to_s } + + it { is_expected.to be value } + end + end + end + end + + describe '#log_collection_enabled=' do + let(:env_var_name) { 'DD_TELEMETRY_LOG_COLLECTION_ENABLED' } + let(:env_var_value) { 'true' } + + it 'updates the #log_collection_enabled setting' do + expect { settings.telemetry.log_collection_enabled = false } + .to change { settings.telemetry.log_collection_enabled } + .from(true) + .to(false) + end + end + describe '#heartbeat_interval' do subject(:heartbeat_interval_seconds) { settings.telemetry.heartbeat_interval_seconds } let(:env_var_name) { 'DD_TELEMETRY_HEARTBEAT_INTERVAL' } diff --git a/spec/datadog/core/telemetry/component_spec.rb b/spec/datadog/core/telemetry/component_spec.rb index f5c2852a268..02b02851de6 100644 --- a/spec/datadog/core/telemetry/component_spec.rb +++ b/spec/datadog/core/telemetry/component_spec.rb @@ -8,6 +8,7 @@ enabled: enabled, http_transport: http_transport, metrics_enabled: metrics_enabled, + log_collection_enabled: log_collection_enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, dependency_collection: dependency_collection, @@ -17,6 +18,7 @@ let(:enabled) { true } let(:metrics_enabled) { true } + let(:log_collection_enabled) { true } let(:heartbeat_interval_seconds) { 0 } let(:metrics_aggregation_interval_seconds) { 1 } let(:shutdown_timeout_seconds) { 1 } @@ -232,8 +234,10 @@ telemetry.stop! end - describe 'when enabled' do + describe 'when enabled and log_collection_enabled is enabled' do let(:enabled) { true } + let(:log_collection_enabled) { true } + it do event = instance_double(Datadog::Core::Telemetry::Event::Log) telemetry.log!(event) @@ -243,11 +247,12 @@ context 'when in fork', skip: !Process.respond_to?(:fork) do it do + telemetry expect_in_fork do event = instance_double(Datadog::Core::Telemetry::Event::Log) telemetry.log!(event) - expect(worker).to have_received(:enqueue).with(event) + expect(worker).not_to have_received(:enqueue) end end end @@ -262,16 +267,16 @@ expect(worker).not_to have_received(:enqueue) end + end - context 'when in fork', skip: !Process.respond_to?(:fork) do - it do - expect_in_fork do - event = instance_double(Datadog::Core::Telemetry::Event::Log) - telemetry.log!(event) + describe 'when log_collection_enabled is disabled' do + let(:log_collection_enabled) { false } - expect(worker).not_to have_received(:enqueue) - end - end + it do + event = instance_double(Datadog::Core::Telemetry::Event::Log) + telemetry.log!(event) + + expect(worker).not_to have_received(:enqueue) end end end From cd6eff5ffd60c0b472d12b22c5906755fda8a0b8 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Mon, 25 Nov 2024 15:16:06 -0800 Subject: [PATCH 2/2] Add comment on forked process --- lib/datadog/core/telemetry/component.rb | 1 + spec/datadog/core/configuration/components_spec.rb | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/datadog/core/telemetry/component.rb b/lib/datadog/core/telemetry/component.rb index 575021f118e..27058df2f6f 100644 --- a/lib/datadog/core/telemetry/component.rb +++ b/lib/datadog/core/telemetry/component.rb @@ -14,6 +14,7 @@ module Datadog module Core module Telemetry # Telemetry entrypoint, coordinates sending telemetry events at various points in app lifecycle. + # Note: Telemetry does not spawn its worker thread in fork processes, thus no telemetry is sent in forked processes. class Component attr_reader :enabled diff --git a/spec/datadog/core/configuration/components_spec.rb b/spec/datadog/core/configuration/components_spec.rb index ae44c42f12b..ba1df917aac 100644 --- a/spec/datadog/core/configuration/components_spec.rb +++ b/spec/datadog/core/configuration/components_spec.rb @@ -233,8 +233,7 @@ metrics_enabled: metrics_enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, - log_collection_enabled: log_collection_enabled, - } + log_collection_enabled: log_collection_enabled, } end let(:enabled) { true } let(:agentless_enabled) { false } @@ -266,8 +265,7 @@ metrics_enabled: false, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, - log_collection_enabled: true, - } + log_collection_enabled: true, } end let(:agent_settings) do instance_double( @@ -293,8 +291,7 @@ metrics_enabled: metrics_enabled, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, - log_collection_enabled: log_collection_enabled, - } + log_collection_enabled: log_collection_enabled, } end before do @@ -314,8 +311,7 @@ metrics_enabled: false, heartbeat_interval_seconds: heartbeat_interval_seconds, metrics_aggregation_interval_seconds: metrics_aggregation_interval_seconds, dependency_collection: dependency_collection, shutdown_timeout_seconds: shutdown_timeout_seconds, - log_collection_enabled: true, - } + log_collection_enabled: true, } end it 'does not enable telemetry when agentless mode requested but api key is not present' do