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.

Sign up for GitHub

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

NH-76676: using noop for all possible error from our agent #123

Merged
merged 10 commits into from
Apr 8, 2024
Merged

NH-76676: using noop for all possible error from our agent #123

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4506aac
NH-76676: using noop for all possible error from our agent
xuan-cao-swi Apr 1, 2024
e071ccf
NH-76676: lint
xuan-cao-swi Apr 1, 2024
bd6f759
NH-76676: linting
xuan-cao-swi Apr 1, 2024
3d1bcae
lint
xuan-cao-swi Apr 1, 2024
f9caf11
lint
xuan-cao-swi Apr 1, 2024
9817e1a
NH-76676: update noop location
xuan-cao-swi Apr 2, 2024
a456188
NH-76676: lint
xuan-cao-swi Apr 2, 2024
e3801f0
NH-76676: revision
xuan-cao-swi Apr 5, 2024
d4b01a0
NH-76676: lint
xuan-cao-swi Apr 5, 2024
4995718
Update opentelemetry.rb
cheempz Apr 5, 2024
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
2 changes: 1 addition & 1 deletion .rubocop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ AllCops:
# or gems.rb file, RuboCop reads the final value from the lock file.) If the
# Ruby version is still unresolved, RuboCop will use the oldest officially
# supported Ruby version (currently Ruby 2.6).
TargetRubyVersion: ~
TargetRubyVersion: 3.0
# Determines if a notification for extension libraries should be shown when
# rubocop is run. Keys are the name of the extension, and values are an array
# of gems in the Gemfile that the extension is suggested for, if not already
Expand Down
8 changes: 7 additions & 1 deletion lib/solarwinds_apm.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
begin
require 'solarwinds_apm/logger'
require 'solarwinds_apm/version'
require 'opentelemetry-api'
if ENV.fetch('SW_APM_ENABLED', 'true') == 'false'
SolarWindsAPM.logger.info '==================================================================='
SolarWindsAPM.logger.info 'SW_APM_ENABLED environment variable detected and was set to false. SolarWindsAPM disabled'
SolarWindsAPM.logger.info '==================================================================='
require 'solarwinds_apm/noop'
cheempz marked this conversation as resolved.
Show resolved Hide resolved
return
end

Expand All @@ -25,6 +27,7 @@
SolarWindsAPM.logger.warn 'SW_APM_SERVICE_KEY Error. SolarWinds APM disabled'
SolarWindsAPM.logger.warn 'Please check previous log messages for more details.'
SolarWindsAPM.logger.warn '=============================================================='
require 'solarwinds_apm/noop'
return
end

Expand Down Expand Up @@ -65,11 +68,11 @@
SolarWindsAPM.logger.warn '=============================================================='
end
else
require 'solarwinds_apm/noop'
SolarWindsAPM.logger.warn '=============================================================='
SolarWindsAPM.logger.warn 'SolarWindsAPM not loaded. SolarWinds APM disabled'
SolarWindsAPM.logger.warn 'Please check previous log messages.'
SolarWindsAPM.logger.warn '=============================================================='
require 'solarwinds_apm/noop'
end
else
SolarWindsAPM.logger.warn '==================================================================='
Expand All @@ -78,16 +81,19 @@
SolarWindsAPM.logger.warn 'SolarWinds APM disabled.'
SolarWindsAPM.logger.warn 'Contact technicalsupport@solarwinds.com if this is unexpected.'
SolarWindsAPM.logger.warn '==================================================================='
require 'solarwinds_apm/noop'
end
rescue LoadError => e
SolarWindsAPM.logger.error '=============================================================='
SolarWindsAPM.logger.error 'Error occurs while loading solarwinds_apm. SolarWinds APM disabled.'
SolarWindsAPM.logger.error "Error: #{e.message}"
SolarWindsAPM.logger.error 'See: https://documentation.solarwinds.com/en/success_center/observability/default.htm#cshid=config-ruby-agent'
SolarWindsAPM.logger.error '=============================================================='
require 'solarwinds_apm/noop'
end

rescue StandardError => e
$stderr.puts "[solarwinds_apm/error] Problem loading: #{e.inspect}"
$stderr.puts e.backtrace
require 'solarwinds_apm/noop'
end
1 change: 1 addition & 0 deletions lib/solarwinds_apm/noop.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
require_relative './noop/metadata'
require_relative './noop/reporter'
require_relative './noop/span'
require_relative './noop/api'
101 changes: 101 additions & 0 deletions lib/solarwinds_apm/noop/api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# © 2023 SolarWinds Worldwide, LLC. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.

####
# noop version of SolarWindsAPM::API
#
module SolarWindsAPM
# API
module API
end

# Tracing
module Tracing
# (wait_milliseconds=3000, integer_response: false)
def solarwinds_ready?(*_args, **options)
options && options[:integer_response] ? 0 : false
end
end

# CurrentTraceInfo
module CurrentTraceInfo
def current_trace_info
TraceInfo.new
end

class TraceInfo
attr_reader :tracestring, :trace_id, :span_id, :trace_flags, :do_log

REGEXP = /^(?<tracestring>(?<version>[a-f0-9]{2})-(?<trace_id>[a-f0-9]{32})-(?<span_id>[a-f0-9]{16})-(?<flags>[a-f0-9]{2}))$/.freeze # rubocop:disable Style/RedundantFreeze
Fixed Show fixed Hide fixed
private_constant :REGEXP
cheempz marked this conversation as resolved.
Show resolved Hide resolved

def initialize
@trace_id, @span_id, @trace_flags, @tracestring = current_span
@service_name = ENV['OTEL_SERVICE_NAME']
@do_log = log?
end

def for_log
''
end

def hash_for_log
{}
end

private

def current_span
%w[00000000000000000000000000000000 0000000000000000 00 00-00000000000000000000000000000000-0000000000000000-00]
end

def log?
:never
end

def valid?(*)
false
end

def sampled?(*)
false
end

def split(*)
REGEXP.match('00-00000000000000000000000000000000-0000000000000000-00')
end
end
end

# CustomMetrics
module CustomMetrics
def increment_metric(*)
false
end

def summary_metric(*)
false
end
end

# OpenTelemetry
module OpenTelemetry
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
def in_span(*); end
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this actually yield the given block, i.e. application code still gets executied? e.g.

SolarWindsAPM::API.in_span('custom_span') do |span|
   url = URI.parse("http://www.google.ca/")
   req = Net::HTTP::Get.new(url.to_s)
   res = Net::HTTP.start(url.host, url.port) {|http| http.request(req)}
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Also just noticed the API docstring for in_span return value is probably a typo, please update to clarify what the expected value is.

apm-ruby/lib/solarwinds_apm/api/opentelemetry.rb

Lines 30 to 31 in 5e236b7

# === Returns:
# * Objective

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this actually yield the given block, i.e. application code still gets executied? e.g.

No, I thought for noop the return value will be nil. Just checked the noop (api) in_span in otel-ruby, it seems like it will execute the block not just return nil. Will update.

please update to clarify what the expected value is.

in_span (include otel original in_span method) will return the last value of given block. In this case it return Net::HTTPOK, but if I have in_span('custom_span') do |span|; 'abc' end, it will return string. That's why I put objective here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I'd expect a proper no-op to still yield the block, in order to not impact the application code. I updated the docstring in 4995718 because "Objective" isn't a commonly understood terminology in this context IMHO.

end

# TransactionName
module TransactionName
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
def set_transaction_name(*)
false
cheempz marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

SolarWindsAPM::API.extend(SolarWindsAPM::Tracing)
SolarWindsAPM::API.extend(SolarWindsAPM::CurrentTraceInfo)
SolarWindsAPM::API.extend(SolarWindsAPM::CustomMetrics)
SolarWindsAPM::API.extend(SolarWindsAPM::OpenTelemetry)
SolarWindsAPM::API.extend(SolarWindsAPM::TransactionName)
5 changes: 4 additions & 1 deletion lib/solarwinds_apm/noop/metadata.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
####
# noop version of SolarWindsAPM::Metadata
#
#
module SolarWindsAPM
# Metadata
class Metadata
Expand All @@ -20,6 +19,10 @@ def self.makeRandom
Metadata.new
end

def self.fromString
''
end

def isValid
false
end
Expand Down
24 changes: 24 additions & 0 deletions test/initest_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,27 @@ def $stdout.write(string)
Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new

$LOAD_PATH.unshift("#{Dir.pwd}/lib/")

def noop_shared_test
_(SolarWindsAPM::Reporter.respond_to?(:start)).must_equal true
_(SolarWindsAPM::Reporter.respond_to?(:send_status)).must_equal true
_(SolarWindsAPM::Reporter.respond_to?(:send_report)).must_equal true
_(SolarWindsAPM::Metadata.respond_to?(:makeRandom)).must_equal true
_(SolarWindsAPM::Span.respond_to?(:createHttpSpan)).must_equal true
_(SolarWindsAPM::Span.respond_to?(:createSpan)).must_equal true
_(SolarWindsAPM::Context.toString).must_equal '99-00000000000000000000000000000000-0000000000000000-00'

_(defined?(SolarWindsAPM::API)).must_equal 'constant'
_(SolarWindsAPM::API.solarwinds_ready?(300)).must_equal false
_(SolarWindsAPM::API.increment_metric).must_equal false
_(SolarWindsAPM::API.summary_metric).must_equal false
_(SolarWindsAPM::API.in_span).must_equal nil
cheempz marked this conversation as resolved.
Show resolved Hide resolved
_(SolarWindsAPM::API.set_transaction_name).must_equal false
_(SolarWindsAPM::API.current_trace_info.hash_for_log.to_s).must_equal '{}'
_(SolarWindsAPM::API.current_trace_info.for_log).must_equal ''
_(SolarWindsAPM::API.current_trace_info.tracestring).must_equal '00-00000000000000000000000000000000-0000000000000000-00'
_(SolarWindsAPM::API.current_trace_info.trace_flags).must_equal '00'
_(SolarWindsAPM::API.current_trace_info.span_id).must_equal '0000000000000000'
_(SolarWindsAPM::API.current_trace_info.trace_id).must_equal '00000000000000000000000000000000'
_(SolarWindsAPM::API.current_trace_info.do_log).must_equal :never
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_1_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@
ENV['SW_APM_ENABLED'] = 'false'
require './lib/solarwinds_apm'
assert_includes log_output.string, 'SW_APM_ENABLED environment variable detected and was set to false. SolarWindsAPM disabled'

noop_shared_test
end
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_2_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@

require './lib/solarwinds_apm'
assert_includes log_output.string, 'SW_APM_SERVICE_KEY problem. API Token in wrong format'

noop_shared_test
end
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_3_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@

require './lib/solarwinds_apm'
assert_includes log_output.string, 'SW_APM_SERVICE_KEY format problem. Service Name is missing.'

noop_shared_test
end
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_4_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@

require './lib/solarwinds_apm'
assert_includes log_output.string, 'SolarWindsAPM warning: Platform macos not yet supported on current solarwinds_apm'

noop_shared_test
end
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_5_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@

require './lib/solarwinds_apm'
assert_includes log_output.string, 'SW_APM_SERVICE_KEY format problem. Service Name is missing.'

noop_shared_test
end
end
2 changes: 2 additions & 0 deletions test/solarwinds_apm/init_test/init_6_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@

require './lib/solarwinds_apm'
assert_includes log_output.string, 'SW_APM_SERVICE_KEY not configured.'

noop_shared_test
end
end
6 changes: 1 addition & 5 deletions test/solarwinds_apm/init_test/init_7_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@

assert_nil(defined?(SolarWindsAPM.loaded))

# we don't load noop in case that the oboe c lib is missing
assert_nil(defined?(SolarWindsAPM::Reporter))
assert_nil(defined?(SolarWindsAPM::Metadata))
assert_nil(defined?(SolarWindsAPM::Span))
assert_nil(defined?(SolarWindsAPM::Context))
noop_shared_test
end
end
10 changes: 2 additions & 8 deletions test/solarwinds_apm/init_test/init_8_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@

_(SolarWindsAPM.loaded).must_equal false

_(SolarWindsAPM::Reporter.respond_to?(:start)).must_equal true
_(SolarWindsAPM::Reporter.respond_to?(:send_status)).must_equal true
_(SolarWindsAPM::Reporter.respond_to?(:send_report)).must_equal true
_(SolarWindsAPM::Metadata.respond_to?(:makeRandom)).must_equal true
_(SolarWindsAPM::Span.respond_to?(:createHttpSpan)).must_equal true
_(SolarWindsAPM::Span.respond_to?(:createSpan)).must_equal true
_(SolarWindsAPM::Context.toString).must_equal '99-00000000000000000000000000000000-0000000000000000-00'

FileUtils.rm("#{Dir.pwd}/lib/libsolarwinds_apm.so")

noop_shared_test
end
end