NH-76676: using noop for all possible error from our agent #123
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
One more batch of comments, getting close thanks @xuan-cao-swi !
lib/solarwinds_apm/noop/api.rb
Outdated
|
|
||
| # OpenTelemetry | ||
| module OpenTelemetry | ||
| def in_span(*); end |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Description
Test (if applicable)