Skip to content

Commit dda2762

Browse files
p-ddsignclaude
andcommitted
Address review findings on component.rb
Six small changes flagged in review on the Component class: - build: `unless environment_supported?(symdb_logger) ... return nil end` collapsed to guard-clause `return nil unless environment_supported?(...)`, with a note that the helper logs the specific reason internally. - build: compound `unless settings.remote&.enabled || ... force_upload` flipped to `if !A && !B` (DeMorgan) — easier to read. - shutdown!: removed standalone `# @api private` marker before `private` keyword (redundant; the keyword already enforces visibility). - scheduler_loop: added a comment explaining what `should_fire = true` means (debounce deadline elapsed; extract_and_upload runs once after the mutex is released). - scheduler_loop: removed the steep:ignore:start/:end block around the mutex section. NOTE: removal is unverified locally because the libdatadog version mismatch blocks Steep from running here; CI will surface a regression if removal isn't safe. - extract_and_upload: moved `extraction_duration` and `targetable_count` calculations inside the `@logger.debug` block so the work is skipped when debug logging is off (the latter iterates the scope tree). Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 4473fe6 commit dda2762

1 file changed

Lines changed: 10 additions & 10 deletions

File tree

lib/datadog/symbol_database/component.rb

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,11 @@ def self.build(settings, agent_settings, logger)
9191
# Symbol database requires MRI Ruby 2.6+.
9292
# Configuration accessors (settings.symbol_database.*) remain available on all
9393
# platforms — only the component (upload) is disabled on unsupported engines/versions.
94-
unless environment_supported?(symdb_logger)
95-
return nil
96-
end
94+
# environment_supported? logs the specific reason (engine or version) internally.
95+
return nil unless environment_supported?(symdb_logger)
9796

9897
# Requires remote config (unless force mode)
99-
unless settings.remote&.enabled || settings.symbol_database.internal.force_upload
98+
if !settings.remote&.enabled && !settings.symbol_database.internal.force_upload
10099
symdb_logger.debug("symdb: remote config not available and force_upload not set, skipping")
101100
return nil
102101
end
@@ -255,7 +254,6 @@ def shutdown!
255254
@scope_batcher.shutdown
256255
end
257256

258-
# @api private
259257
private
260258

261259
# Check whether the runtime environment supports symbol database upload.
@@ -291,9 +289,10 @@ def ensure_scheduler_thread
291289
# @return [void]
292290
def scheduler_loop
293291
loop do
292+
# should_fire = true means the debounce deadline elapsed without further
293+
# signals; extract_and_upload runs once after the mutex is released.
294294
should_fire = false
295295

296-
# steep:ignore:start
297296
@scheduler_mutex.synchronize do
298297
return if @shutdown
299298
return if Component.uploaded_this_process?
@@ -317,7 +316,6 @@ def scheduler_loop
317316
end
318317
end
319318
end
320-
# steep:ignore:end
321319

322320
# `next` inside `synchronize` only exits the synchronize block — not the
323321
# surrounding loop. Use an explicit flag so the loop only fires
@@ -357,9 +355,11 @@ def extract_and_upload
357355
log_scope_tree(scope, 0)
358356
end
359357

360-
extraction_duration = Datadog::Core::Utils::Time.get_time - start_time
361-
targetable_count = count_targetable_methods(file_scopes)
362-
@logger.debug { "symdb: extracted #{extracted_count} scopes (#{targetable_count} methods with targetable lines) in #{'%.2f' % extraction_duration}s" }
358+
@logger.debug do
359+
extraction_duration = Datadog::Core::Utils::Time.get_time - start_time
360+
targetable_count = count_targetable_methods(file_scopes)
361+
"symdb: extracted #{extracted_count} scopes (#{targetable_count} methods with targetable lines) in #{'%.2f' % extraction_duration}s"
362+
end
363363

364364
# Flush any remaining scopes (triggers upload)
365365
@scope_batcher.flush

0 commit comments

Comments
 (0)