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

WebMock::NetConnectNotAllowedError when using dd-trace-rb 1.13.1 with test suite #3050

Closed
ivoanjo opened this issue Aug 16, 2023 · 19 comments
Closed
Assignees
Labels
bug Involves a bug community Was opened by a community member
Milestone

Comments

@ivoanjo
Copy link
Member

ivoanjo commented Aug 16, 2023

Current behaviour

Reported by @tim-wovn in #2823 (comment):

@ivoanjo Thanks for your work on this! Unfortunately, after upgrading to 1.13.1, if I try to remove my ddtrace config for tests, tests fail:

Real HTTP connections are disabled. Unregistered request: GET http://127.0.0.1:8126/info with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Datadog-Client-Computed-Top-Level'=>'1', 'Datadog-Container-Id'=>'f410e64833816ae1fad333abe62854c2bd764f9890d1fa26e0bf086da9fb924d', 'Datadog-Meta-Lang'=>'ruby', 'Datadog-Meta-Lang-Interpreter'=>'ruby-x86_64-linux', 'Datadog-Meta-Lang-Version'=>'3.2.2', 'Datadog-Meta-Tracer-Version'=>'1.13.1', 'User-Agent'=>'Ruby'} (WebMock::NetConnectNotAllowedError)

Expected behaviour

In #3039 we added support for auto-detecting test environments (and not trying to perform telemetry/remote configuration-related requests), but we may have missed something.

Steps to reproduce

TBA

Environment

  • ddtrace version: 1.13.1
  • Configuration block (Datadog.configure ...): ?
  • Ruby version: ?
  • Operating system: ?
  • Relevant library versions: ?
@ivoanjo
Copy link
Member Author

ivoanjo commented Aug 16, 2023

@tim-wovn you mentioned that you had an existing configuration for using dd-trace-rb that worked with your tests.

Can you share it here? I'd like to compare it with what the auto-detection is doing, see what we can do to support your setup/or what we missed.

@tim-wovn
Copy link

tim-wovn commented Aug 17, 2023

@ivoanjo Sorry for the hassle.

Here is some information on my environment:

  • ddtrace version: 1.13.1
  • Configuration block:

in config/environments/test.rb

Datadog.configure do |c|
   c.telemetry.enabled = false
   c.remote.enabled = false
end

in config/initializers/datadog.rb

Datadog.configure do |c|
  c.tracing.enabled = Rails.env.production? || Rails.env.staging?
  c.tracing.log_injection = false
  c.tracing.instrument(:rails, service_name: "maestro-#{Rails.env}")
  c.tracing.instrument(:redis, analytics_enabled: true, service_name: 'redis')
  c.tracing.instrument(:http, analytics_enabled: true, service_name: 'NetHttp')
end
  • Ruby version: 3.2.2
  • Operating system: Mac OS X 13.4.1 (c); Ubuntu 22.04 (via cimg/ruby:3.2.2 on circleci)
  • Relevant libraries:
    • from our gemfile: gem 'ddtrace', require: 'ddtrace/auto_instrument'

Regarding the configuration block, I logged some stats before and after each--namely Datadog.configuration.tracing.enabled, Datadog.configuration.telemetry.enabled, and Datadog.configuration.remote.enabled.

Before hitting any configuration block, all three values were true. After the first configuration block, tracing was still true. After the second configuration block, all three values are false. Just to be sure, I checked the Rails environment and it is set to test.

We added the line c.telemetry.enabled = false when we upgraded from ddtrace 1.10.1 to 1.11.1. And we added the line c.remote.enabled = false when we upgraded from ddtrace 1.12.1 to 1.13.0. Both still seem to be required with 1.13.1

If tracing, telemetry, or remote are true, we get an error when running tests, with output like:

Real HTTP connections are disabled. Unregistered request: POST http://127.0.0.1:8126/telemetry/proxy/api/v2/apmtelemetry

If there is any other information that could be helpful, please let me know and I can try to provide it! Thank you!

@marcotc
Copy link
Member

marcotc commented Aug 17, 2023

👋, @tim-wovn I think that's all the information we need, thank you for the detailed report!

@marcotc
Copy link
Member

marcotc commented Aug 17, 2023

Actually, one question: what test runner do you use, @tim-wovn?

@marcotc
Copy link
Member

marcotc commented Aug 17, 2023

😅 a few more:

  1. When you are running tests, are you doing it with Spring enabled?
  2. If you run your Rails console locally, but with the Datadog agent not available, can you report if you get errors like Unable to send telemetry event to agent: Failed to open TCP connection to 127.0.0.1:8126?

@tim-wovn
Copy link

Thanks for taking the time to look at this!

Actually, one question: what test runner do you use, @tim-wovn?

Mini-test.

When you are running tests, are you doing it with Spring enabled?

Spring is in our gemfile but we're not using it for tests. Just to be sure, I tried the branch you opened #3058 and get the same result I reported for tracing, telemetry, and remote.

If you run your Rails console locally, but with the Datadog agent not available, can you report if you get errors like

I don't have the datadog agent installed locally and it isn't installed in CI either. We only have it installed in our staging and production environments. Locally I'm not seeing errors like you mentioned from the rails console.


From our end, we have a workaround (setting those 3 variables to false in our config). It's possible (likely?) that we have some custom config that is confusing ddtrace, although I've been poking through our configuration files and I didn't see any likely candidates.

@tim-wovn
Copy link

As a follow-up. I tried to generate a new rails project using the same datadog configuration as in the project I've described.

I generated the project using rails new blog only modifying the project by adding ddtrace to my gemfile and adding the file config/initializers/datadog.rb with the same configuration.

Running tests, tracing, telemetry, and remote are all set to true by default. My (naive) expectation is that those would be false because my rails environment is test. Is a different outcome expected?

@marcotc
Copy link
Member

marcotc commented Aug 18, 2023

@tim-wovn, could you try the changes in this PR? #3062

I'm confident of the changes, just having a hard time getting portable tests for the relevant Rails combinations at this moment.

@tim-wovn
Copy link

could you try the changes in this PR? #3062

@marcotc The PR does resolve the issue in both my work project and the rails new blog. Thank you!

Under my work project, I tested opening the rails console under different environments (e.g. RAILS_ENV=staging bundle exec rails console).

For test and development, tracing, telemetry, and remote were all false while for production they were all true. For staging on the other hand, tracing was true but the other two were false.

Testing with ddtrace 1.13.1, all three values are true for both production and staging. I'm not saying this is a problem for my work's usecase, but I thought I would report this difference.

Thank you again for your work on this!

@marcotc
Copy link
Member

marcotc commented Aug 28, 2023

@tim-wovn, I'm still on it 😅.

How would this version work for you? #3062

It changes detection so that staging is not considered development? anymore.

@tim-wovn
Copy link

@marcotc I tried out your latest changes, testing with the same setup as before for both my work project and the blog.

I'm happy to confirm that everything seems good now! 🎉

For test and development, all three variables were false while for production and staging all three were true.

If there is any other info I can provide, please let me know. Thanks again for your work on this! 🙇

@marcotc marcotc added this to the 1.15.0 milestone Sep 19, 2023
@marcotc
Copy link
Member

marcotc commented Oct 10, 2023

Thank you for your patience, the fix in #3062 has been released in v1.15.0.

Please let us know if the issue persists.

@marcotc marcotc closed this as completed Oct 10, 2023
@tim-wovn
Copy link

I pulled updated to 1.15 and all seems well! Thanks again for your work on this!

@mcg
Copy link

mcg commented Oct 11, 2023

Sadly we continue to have this issue on 1.15. I've tried other suggestions to disable during CI and nothing works except removing require: "ddtrace/auto_instrument" from the Gemfile.

@tim-wovn
Copy link

@mcg What does your config/initializers/datadog.rb file look like?

Mine is like this:

Datadog.configure do |c|
  c.tracing.enabled = Rails.env.production? || Rails.env.staging?
  c.tracing.log_injection = false

  # rubocop:disable Rails/Output
  puts '================='
  puts "Tracing: #{c.tracing.enabled}"
  puts "Telemetry: #{c.telemetry.enabled}"
  puts "Remote: #{c.remote.enabled}"
  puts "Rails env: #{Rails.env}"
  # rubocop:enable Rails/Output
end

In my gemfile I have:
gem 'ddtrace', require: 'ddtrace/auto_instrument'

For me, with ddtrace 1.14.0 on circleci I get:

Tracing: false
Telemetry: true
Remote: true
Rails env: test

With ddtrace 1.15.0 on circleci I get:

Tracing: false
Telemetry: false
Remote: false
Rails env: test

With 1.14.0 I was able to avoid the issue by manually setting telemetry and remove to false in
config/environments/test.rb

Rails.application.configure do
  ...
  # Datadog's telemetry causes an error when running tests
  # https://github.com/DataDog/dd-trace-rb/issues/2823
  # https://github.com/WOVNio/translation_platform/pull/1695
  Datadog.configure do |c|
    c.telemetry.enabled = false
    c.remote.enabled = false
  end
  ...
end

@mcg
Copy link

mcg commented Oct 12, 2023

@tim-wovn We only enable DD in the initializer for RAILS_ENV=production, but commenting that out and only using your snippet we get:

=================
Tracing: true
Telemetry: false
Remote: false
Rails env: test

Tracing is still true. Do we still need to explicitly disable that for testing?

@tim-wovn
Copy link

tim-wovn commented Oct 12, 2023

Tracing is still true. Do we still need to explicitly disable that for testing?

@mcg I'm not sure to be honest--although it seems so. In my config/initializers/datadog.rb file of my work project I have:
c.tracing.enabled = Rails.env.production? || Rails.env.staging?

Checking the history, we added that around the time we upgraded from ddtrace 0.54 to 1.8 I'm guessing that we experienced errors in running tests with tracing enabled by default and we disabled it.

To upgrade from ddtrace 1.10 to 1.11 I had to add c.telemetry.enabled = false to config/environments/test.rb and when upgrading ddtrace from 1.12 to 1.13 I had to add c.remote.enabled = false.

With 1.15 I've been able to remove those lines at least from test.rb. But we still manually disable tracing for tests.

@mcg
Copy link

mcg commented Oct 12, 2023

I tried this In our DD initializer:

Datadog.configure do |c|
    c.telemetry.enabled = false
    c.remote.enabled = false
    c.tracing.enabled - false
  end

and we are still failing with WebMock. The only way I can get this to work is removing require: "ddtrace/auto_instrument" from the Gemfile and then in the initializer:

if Rails.env.production?
  require 'ddtrace'
  require 'ddtrace/auto_instrument'
  Datadog.configure do |c|
    c.remote.enabled = true
    .....

@ivoanjo
Copy link
Member Author

ivoanjo commented Oct 23, 2023

@mcg sorry that you're still getting bitten by this!

Can I ask you to open a new issue with it, so we can use it to investigate what we missed for your setup? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug community Was opened by a community member
Projects
None yet
Development

No branches or pull requests

4 participants