From 304891821b5d78406f20f72c04c7577cc07f4773 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 4 Jul 2024 11:16:08 +0200 Subject: [PATCH 1/4] ignore percent-w rule --- static-analysis.datadog.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/static-analysis.datadog.yml b/static-analysis.datadog.yml index 79a2fc4df7b..299ccbb4444 100644 --- a/static-analysis.datadog.yml +++ b/static-analysis.datadog.yml @@ -2,4 +2,17 @@ schema-version: v1 rulesets: - ruby-code-style - ruby-security - - ruby-best-practices + - ruby-best-practices: + rules: + symbols-as-keys: + ignore: + - "**" + hash-fetch: + ignore: + - "**" + percent-w: + ignore: + - spec/**/* + no-optional-hash-params: + ignore: + - "**" From 3a9e378bd03932e1f6df88679a4af99a527faf81 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 4 Jul 2024 11:16:30 +0200 Subject: [PATCH 2/4] do not wrap whole method body with begin ... end --- .../core/telemetry/http/adapters/net.rb | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/datadog/core/telemetry/http/adapters/net.rb b/lib/datadog/core/telemetry/http/adapters/net.rb index 3aa65e6d49d..98b45bf1cbc 100644 --- a/lib/datadog/core/telemetry/http/adapters/net.rb +++ b/lib/datadog/core/telemetry/http/adapters/net.rb @@ -34,19 +34,17 @@ def open(&block) end def post(env) - begin - post = ::Net::HTTP::Post.new(env.path, env.headers) - post.body = env.body - - http_response = open do |http| - http.request(post) - end - - Response.new(http_response) - rescue StandardError => e - Datadog.logger.debug("Unable to send telemetry event to agent: #{e}") - Telemetry::Http::InternalErrorResponse.new(e) + post = ::Net::HTTP::Post.new(env.path, env.headers) + post.body = env.body + + http_response = open do |http| + http.request(post) end + + Response.new(http_response) + rescue StandardError => e + Datadog.logger.debug("Unable to send telemetry event to agent: #{e}") + Telemetry::Http::InternalErrorResponse.new(e) end # Data structure for an HTTP Response From e6add8cadbc3ff0615591037b2f33f9ba5c55353 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 4 Jul 2024 16:01:12 +0200 Subject: [PATCH 3/4] automatically avoid WebMock stubs when sending telemetry events --- .../core/telemetry/http/adapters/net.rb | 10 +++- .../core/telemetry/http/adapters/net.rbs | 4 ++ .../core/telemetry/http/adapters/net_spec.rb | 58 +++++++++++++++---- vendor/rbs/webmock/0/webmock.rbs | 2 + 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/lib/datadog/core/telemetry/http/adapters/net.rb b/lib/datadog/core/telemetry/http/adapters/net.rb index 98b45bf1cbc..383cf1953d1 100644 --- a/lib/datadog/core/telemetry/http/adapters/net.rb +++ b/lib/datadog/core/telemetry/http/adapters/net.rb @@ -25,7 +25,7 @@ def initialize(hostname:, port: nil, timeout: DEFAULT_TIMEOUT, ssl: true) end def open(&block) - req = ::Net::HTTP.new(@hostname, @port) + req = net_http_client.new(@hostname, @port) req.use_ssl = @ssl req.open_timeout = req.read_timeout = @timeout @@ -103,6 +103,14 @@ def inspect "#{super}, http_response:#{http_response}" end end + + private + + def net_http_client + return ::Net::HTTP unless defined?(WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetHTTP) + + WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetHTTP + end end end end diff --git a/sig/datadog/core/telemetry/http/adapters/net.rbs b/sig/datadog/core/telemetry/http/adapters/net.rbs index 311c5989f95..f748f46cf39 100644 --- a/sig/datadog/core/telemetry/http/adapters/net.rbs +++ b/sig/datadog/core/telemetry/http/adapters/net.rbs @@ -45,6 +45,10 @@ module Datadog def inspect: () -> ::String end + + private + + def net_http_client: () -> singleton(::Net::HTTP) end end end diff --git a/spec/datadog/core/telemetry/http/adapters/net_spec.rb b/spec/datadog/core/telemetry/http/adapters/net_spec.rb index e4513c0ae87..358ec60203c 100644 --- a/spec/datadog/core/telemetry/http/adapters/net_spec.rb +++ b/spec/datadog/core/telemetry/http/adapters/net_spec.rb @@ -14,7 +14,8 @@ let(:http_connection) { instance_double(::Net::HTTP) } before do - allow(::Net::HTTP).to receive(:new) + # webmock stores real ::Net::HTTP in this constant + allow(::WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetHTTP).to receive(:new) .with( adapter.hostname, adapter.port, @@ -90,25 +91,58 @@ end describe '#post' do - include_context 'HTTP connection stub' include_context 'HTTP Env' - subject(:post) { adapter.post(env) } + context 'with stubbed Net::HTTP' do + include_context 'HTTP connection stub' + + subject(:post) { adapter.post(env) } + + let(:http_response) { double('http_response') } - let(:http_response) { double('http_response') } + context 'when request goes through' do + before { expect(http_connection).to receive(:request).and_return(http_response) } - context 'when request goes through' do - before { expect(http_connection).to receive(:request).and_return(http_response) } + it 'produces a response' do + is_expected.to be_a_kind_of(described_class::Response) + expect(post.http_response).to be(http_response) + end + end - it 'produces a response' do - is_expected.to be_a_kind_of(described_class::Response) - expect(post.http_response).to be(http_response) + context 'when error in connecting to agent' do + before { expect(http_connection).to receive(:request).and_raise(StandardError) } + it { expect(post).to be_a_kind_of(Datadog::Core::Telemetry::Http::InternalErrorResponse) } end end - context 'when error in connecting to agent' do - before { expect(http_connection).to receive(:request).and_raise(StandardError) } - it { expect(post).to be_a_kind_of(Datadog::Core::Telemetry::Http::InternalErrorResponse) } + context 'with real Net::HTTP' do + subject(:post) { adapter.post(env) } + + let(:hostname) { 'hostname_that_doesnt_exist' } + let(:port) { 4444 } + let(:timeout) { 1 } + let(:expected_error) do + # Socket::ResolutionError is for Ruby 3.3+ + if defined?(Socket::ResolutionError) + Socket::ResolutionError + else + SocketError + end + end + + before do + WebMock.disable_net_connect! + end + + after do + WebMock.disable! + end + + it 'makes real HTTP request and fails' do + response = post + expect(response).to be_a_kind_of(Datadog::Core::Telemetry::Http::InternalErrorResponse) + expect(response.error).to be_a_kind_of(expected_error) + end end end end diff --git a/vendor/rbs/webmock/0/webmock.rbs b/vendor/rbs/webmock/0/webmock.rbs index 6db1144f896..777f174c2fb 100644 --- a/vendor/rbs/webmock/0/webmock.rbs +++ b/vendor/rbs/webmock/0/webmock.rbs @@ -4,6 +4,8 @@ module WebMock def self.enable!: -> void def self.disable!: -> void + + OriginalNetHTTP: singleton(::Net::HTTP) end end end From 6650e3f39751cf90e2a5413cf1532782e651503f Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 4 Jul 2024 16:30:36 +0200 Subject: [PATCH 4/4] use WebMock.allow_net_connect! after test --- spec/datadog/core/telemetry/http/adapters/net_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/datadog/core/telemetry/http/adapters/net_spec.rb b/spec/datadog/core/telemetry/http/adapters/net_spec.rb index 358ec60203c..1bf34cb36b4 100644 --- a/spec/datadog/core/telemetry/http/adapters/net_spec.rb +++ b/spec/datadog/core/telemetry/http/adapters/net_spec.rb @@ -135,7 +135,7 @@ end after do - WebMock.disable! + WebMock.allow_net_connect! end it 'makes real HTTP request and fails' do