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

Add development environment detection #3036

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Add development environment detection #3036

merged 5 commits into from
Aug 10, 2023

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Aug 8, 2023

This PR adds a utility to detect if ddtrace is running in a development environment (an environment that is unlikely to need to send traces, profiles, perform appsec checking).

This will enable us to selectively disable heavy features (background workers, network requests) performed by ddtrace when the environment does not need it. This is especially helpful when running tests with ddtrace installed.

@marcotc marcotc self-assigned this Aug 8, 2023
@marcotc marcotc requested a review from a team August 8, 2023 22:44
@github-actions github-actions bot added the core Involves Datadog core libraries label Aug 8, 2023
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

I suspect you're already aware (because you reused some code from it?) but just-in-case I'll point out that the EnvironmentLogger does something similar so it would be a good target to be updated to use the new helper:

# Are we logging the environment data?
def log?
startup_logs_enabled = Datadog.configuration.diagnostics.startup_logs.enabled
if startup_logs_enabled.nil?
!repl? # Suppress logs if we running in a REPL
else
startup_logs_enabled
end
end
REPL_PROGRAM_NAMES = %w[irb pry].freeze
def repl?
REPL_PROGRAM_NAMES.include?($PROGRAM_NAME)
end

Comment on lines +16 to +30
# RSpec is detected through the $PROGRAM_NAME.
# Changing it will make RSpec detection to return false.
#
# We change the $PROGRAM_NAME instead of stubbing
# `Datadog::Core::Environment::Execution.rspec?` because
# otherwise we'll have no real test for non-RSpec cases.
around do |example|
begin
original = $PROGRAM_NAME
$PROGRAM_NAME = 'not-rspec'
example.run
ensure
$PROGRAM_NAME = original
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This made me think that one possible alternative implementation strategy is to maybe have a proc-per-thing we want to detect, e.g.

RSPEC_PROBE = proc { $PROGRAM_NAME.end_with?(RSPEC_PROGRAM_NAME) }
MINITEST_PROBE = proc { ... }

def test?(test_probes: [RSPEC_PROBE, MINITEST_PROBE])
  test_probes.any?(&:call)
end

then we could override individual probes (or even dependency inject them).

This to say -- it came to mind; I think the current version is great as well so as usual with my random suggestions, feel free to ignore :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this suggested approach looks good!

I think the file is small now to keep it as it, but it's definitely a great alternative.

@marcotc
Copy link
Member Author

marcotc commented Aug 9, 2023

The EnvironmentLogger is undergoing a big refactoring by @sarahchen6 as we speak, I didn't want to refactor it as to avoid conflicts, but I agree that they will have to converge in the future.

@codecov-commenter
Copy link

Codecov Report

Merging #3036 (cd17e14) into master (6031043) will increase coverage by 0.03%.
Report is 20 commits behind head on master.
The diff coverage is 96.05%.

@@            Coverage Diff             @@
##           master    #3036      +/-   ##
==========================================
+ Coverage   98.08%   98.12%   +0.03%     
==========================================
  Files        1317     1323       +6     
  Lines       74362    74649     +287     
  Branches     3402     3403       +1     
==========================================
+ Hits        72941    73248     +307     
+ Misses       1421     1401      -20     
Files Changed Coverage Δ
spec/datadog/profiling/validate_benchmarks_spec.rb 100.00% <ø> (ø)
spec/datadog/core/environment/execution_spec.rb 91.89% <91.89%> (ø)
lib/datadog/core/environment/execution.rb 100.00% <100.00%> (ø)
lib/datadog/tracing/contrib.rb 100.00% <100.00%> (ø)
.../datadog/tracing/contrib/suite/integration_spec.rb 100.00% <100.00%> (ø)
spec/datadog/tracing/contrib_spec.rb 100.00% <100.00%> (ø)

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@TonyCTHsu TonyCTHsu added this to the 1.13.1 milestone Aug 10, 2023
@marcotc marcotc merged commit 5c390e1 into master Aug 10, 2023
@marcotc marcotc deleted the non-monitor-detection branch August 10, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants