Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport "Reduce startup logs verbosity" to master #3468

Merged
merged 3 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ target :ddtrace do
ignore 'lib/datadog/core/configuration/option_definition_set.rb'
ignore 'lib/datadog/core/configuration/options.rb'
ignore 'lib/datadog/core/configuration/settings.rb'
ignore 'lib/datadog/core/diagnostics/environment_logger.rb'
ignore 'lib/datadog/core/diagnostics/health.rb'
ignore 'lib/datadog/core/encoding.rb'
ignore 'lib/datadog/core/environment/container.rb'
Expand Down
7 changes: 5 additions & 2 deletions lib/datadog/core/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def build_telemetry(settings, agent_settings, logger)

def initialize(settings)
@logger = self.class.build_logger(settings)
@environment_logger_extra = {}

# This agent_settings is intended for use within Core. If you require
# agent_settings within a product outside of core you should extend
Expand All @@ -90,11 +91,13 @@ def initialize(settings)
@remote = Remote::Component.build(settings, agent_settings)
@tracer = self.class.build_tracer(settings, logger: @logger)

@profiler = Datadog::Profiling::Component.build_profiler_component(
@profiler, profiler_logger_extra = Datadog::Profiling::Component.build_profiler_component(
settings: settings,
agent_settings: agent_settings,
optional_tracer: @tracer,
)
@environment_logger_extra.merge!(profiler_logger_extra) if profiler_logger_extra

@runtime_metrics = self.class.build_runtime_metrics_worker(settings)
@health_metrics = self.class.build_health_metrics(settings)
@telemetry = self.class.build_telemetry(settings, agent_settings, logger)
Expand All @@ -116,7 +119,7 @@ def startup!(settings)
@logger.debug('Profiling is disabled')
end

Core::Diagnostics::EnvironmentLogger.collect_and_log!
Core::Diagnostics::EnvironmentLogger.collect_and_log!(@environment_logger_extra)
end

# Shuts down all the components in use.
Expand Down
7 changes: 4 additions & 3 deletions lib/datadog/core/diagnostics/environment_logger.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# frozen_string_literal: true

require 'date'
require 'json'
require 'rbconfig'
require 'time'

module Datadog
module Core
Expand Down Expand Up @@ -58,9 +58,10 @@ def rspec?
module EnvironmentLogger
extend EnvironmentLogging

def self.collect_and_log!
def self.collect_and_log!(extra_fields = nil)
log_once! do
data = EnvironmentCollector.collect_config!
data = data.merge(extra_fields) if extra_fields
log_configuration!('CORE', data.to_json)
end
rescue => e
Expand Down Expand Up @@ -91,7 +92,7 @@ def collect_config!

# @return [String] current time in ISO8601 format
def date
DateTime.now.iso8601
Time.now.utc.iso8601
end

# Best portable guess of OS information.
Expand Down
1 change: 0 additions & 1 deletion lib/datadog/profiling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ def self.allocation_count # rubocop:disable Lint/NestedMethodDefinition (On purp
require_relative 'profiling/collectors/idle_sampling_helper'
require_relative 'profiling/collectors/stack'
require_relative 'profiling/collectors/thread_context'
require_relative 'profiling/diagnostics/environment_logger'
require_relative 'profiling/stack_recorder'
require_relative 'profiling/exporter'
require_relative 'profiling/flush'
Expand Down
11 changes: 4 additions & 7 deletions lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ module Component
# * Code Hotspots panel in the trace viewer, as well as scoping a profile down to a span
# * Endpoint aggregation in the profiler UX, including normalization (resource per endpoint call)
def self.build_profiler_component(settings:, agent_settings:, optional_tracer:) # rubocop:disable Metrics/MethodLength
require_relative '../profiling/diagnostics/environment_logger'

Profiling::Diagnostics::EnvironmentLogger.collect_and_log!

return unless settings.profiling.enabled
return [nil, { profiling_enabled: false }] unless settings.profiling.enabled

# Workaround for weird dependency direction: the Core::Configuration::Components class currently has a
# dependency on individual products, in this case the Profiler.
Expand All @@ -32,7 +28,8 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
# done, then profiling may not be loaded, and thus to avoid this issue we do a require here (which is a
# no-op if profiling is already loaded).
require_relative '../profiling'
return unless Profiling.supported?

return [nil, { profiling_enabled: false }] unless Profiling.supported?

# Activate forking extensions
Profiling::Tasks::Setup.new.run
Expand Down Expand Up @@ -76,7 +73,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
transport = build_profiler_transport(settings, agent_settings)
scheduler = Profiling::Scheduler.new(exporter: exporter, transport: transport, interval: upload_period_seconds)

Profiling::Profiler.new(worker: worker, scheduler: scheduler)
[Profiling::Profiler.new(worker: worker, scheduler: scheduler), { profiling_enabled: true }]
end

private_class_method def self.build_thread_context_collector(settings, recorder, optional_tracer, timeline_enabled)
Expand Down
39 changes: 0 additions & 39 deletions lib/datadog/profiling/diagnostics/environment_logger.rb

This file was deleted.

4 changes: 2 additions & 2 deletions sig/datadog/core/diagnostics/environment_logger.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Datadog
def log_once!: () ?{ () -> untyped } -> (false | untyped)
def log?: () -> untyped

REPL_PROGRAM_NAMES: ::Array["irb" | "pry"]
REPL_PROGRAM_NAMES: ::Array[::String]

def repl?: () -> untyped

Expand All @@ -19,7 +19,7 @@ module Datadog
module EnvironmentLogger
extend EnvironmentLogging

def self.collect_and_log!: () -> untyped
def self.collect_and_log!: (untyped extra_fields) -> untyped
end
module EnvironmentCollector
def self.collect_config!: () -> { date: untyped, os_name: untyped, version: untyped, lang: untyped, lang_version: untyped, env: untyped, service: untyped, dd_version: untyped, debug: untyped, tags: untyped, runtime_metrics_enabled: untyped, vm: untyped, health_metrics_enabled: untyped }
Expand Down
2 changes: 1 addition & 1 deletion sig/datadog/profiling/component.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Datadog
settings: untyped,
agent_settings: Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings,
optional_tracer: Datadog::Tracing::Tracer?,
) -> Datadog::Profiling::Profiler?
) -> [Datadog::Profiling::Profiler?, {profiling_enabled: bool}]

def self.build_thread_context_collector: (
untyped settings,
Expand Down
16 changes: 0 additions & 16 deletions sig/datadog/profiling/diagnostics/environment_logger.rbs

This file was deleted.

15 changes: 13 additions & 2 deletions spec/datadog/core/configuration/components_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
let(:remote) { instance_double(Datadog::Core::Remote::Component, start: nil, shutdown!: nil) }
let(:telemetry) { instance_double(Datadog::Core::Telemetry::Client) }

let(:environment_logger_extra) { { hello: 123, world: '456' } }

include_context 'non-development execution environment'

before do
Expand Down Expand Up @@ -70,7 +72,7 @@
settings: settings,
agent_settings: agent_settings,
optional_tracer: tracer,
).and_return(profiler)
).and_return([profiler, environment_logger_extra])

expect(described_class).to receive(:build_runtime_metrics_worker)
.with(settings)
Expand Down Expand Up @@ -1082,7 +1084,7 @@
settings: settings,
agent_settings: agent_settings,
optional_tracer: anything,
).and_return(profiler)
).and_return([profiler, environment_logger_extra])
end

it do
Expand Down Expand Up @@ -1131,6 +1133,15 @@
end
end
end

it 'calls the EnvironmentLogger' do
expect(Datadog::Profiling::Component).to receive(:build_profiler_component)
.and_return([nil, environment_logger_extra])

expect(Datadog::Core::Diagnostics::EnvironmentLogger).to receive(:collect_and_log!).with(environment_logger_extra)

startup!
end
end

describe '#shutdown!' do
Expand Down
62 changes: 45 additions & 17 deletions spec/datadog/core/diagnostics/environment_logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
subject(:env_logger) { described_class }

before do
allow(DateTime).to receive(:now).and_return(DateTime.new(2020))
allow(Time).to receive(:now).and_return(Time.new(2020))

# Resets "only-once" execution pattern of `collect_and_log!`
env_logger.instance_variable_set(:@executed, nil)
Expand All @@ -21,6 +21,23 @@
subject(:collect_and_log!) { env_logger.collect_and_log! }

let(:logger) { instance_double(Datadog::Core::Logger) }
let(:expected_logger_result) do
{
'date' => '2020-01-01T00:00:00Z',
'os_name' => (include('x86_64').or include('i686').or include('aarch64').or include('arm')),
'version' => DDTrace::VERSION::STRING,
'lang' => 'ruby',
'lang_version' => match(/[23]\./),
'env' => nil,
'service' => be_a(String),
'dd_version' => nil,
'debug' => false,
'tags' => nil,
'runtime_metrics_enabled' => false,
'vm' => be_a(String),
'health_metrics_enabled' => false,
}
end

before do
allow(env_logger).to receive(:rspec?).and_return(false) # Allow rspec to log for testing purposes
Expand All @@ -35,21 +52,7 @@
collect_and_log!
expect(logger).to have_received(:info).with start_with('DATADOG CONFIGURATION - CORE') do |msg|
json = JSON.parse(msg.partition('- CORE -')[2].strip)
expect(json).to match(
'date' => '2020-01-01T00:00:00+00:00',
'os_name' => (include('x86_64').or include('i686').or include('aarch64').or include('arm')),
'version' => DDTrace::VERSION::STRING,
'lang' => 'ruby',
'lang_version' => match(/[23]\./),
'env' => nil,
'service' => be_a(String),
'dd_version' => nil,
'debug' => false,
'tags' => nil,
'runtime_metrics_enabled' => false,
'vm' => be_a(String),
'health_metrics_enabled' => false,
)
expect(json).to match(expected_logger_result)
end
end

Expand Down Expand Up @@ -104,6 +107,31 @@
expect(logger).to have_received(:warn).with start_with('Failed to collect core environment information')
end
end

context 'when extra fields are provided' do
let(:extra_fields) { { hello: 123, world: '456' } }

subject(:collect_and_log!) { env_logger.collect_and_log!(extra_fields) }

it 'includes the base fields' do
collect_and_log!
expect(logger).to have_received(:info).with start_with('DATADOG CONFIGURATION - CORE') do |msg|
json = JSON.parse(msg.partition('- CORE -')[2].strip)
expect(json).to include(expected_logger_result)
end
end

it 'includes the extra fields' do
collect_and_log!
expect(logger).to have_received(:info).with start_with('DATADOG CONFIGURATION - CORE') do |msg|
json = JSON.parse(msg.partition('- CORE -')[2].strip)
expect(json).to include(
'hello' => 123,
'world' => '456',
)
end
end
end
end

describe Datadog::Core::Diagnostics::EnvironmentCollector do
Expand All @@ -114,7 +142,7 @@

it 'with a default core' do
is_expected.to match(
date: '2020-01-01T00:00:00+00:00',
date: '2020-01-01T00:00:00Z',
os_name: (include('x86_64').or include('i686').or include('aarch64').or include('arm')),
version: DDTrace::VERSION::STRING,
lang: 'ruby',
Expand Down
10 changes: 7 additions & 3 deletions spec/datadog/profiling/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
context 'when profiling is not supported' do
before { allow(Datadog::Profiling).to receive(:supported?).and_return(false) }

it { is_expected.to be nil }
it { is_expected.to eq [nil, { profiling_enabled: false }] }
end

context 'by default' do
it 'does not build a profiler' do
is_expected.to be nil
is_expected.to eq [nil, { profiling_enabled: false }]
end
end

Expand All @@ -38,7 +38,7 @@
end

it 'does not build a profiler' do
is_expected.to be nil
is_expected.to eq [nil, { profiling_enabled: false }]
end
end

Expand All @@ -50,6 +50,10 @@
allow(profiler_setup_task).to receive(:run)
end

it 'builds a profiler instance' do
expect(build_profiler_component).to match([instance_of(Datadog::Profiling::Profiler), { profiling_enabled: true }])
end

context 'when using the new CPU Profiling 2.0 profiler' do
it 'initializes a ThreadContext collector' do
allow(Datadog::Profiling::Collectors::CpuAndWallTimeWorker).to receive(:new)
Expand Down
Loading
Loading