honeycombio / libhoney-rb

Ruby library for sending data to Honeycomb
Apache License 2.0
11 stars 30 forks source link

Spans Do Not Get Sent to Honeycomb when running rspec with Webmock #85

Closed Conrimaceogain closed 2 years ago

Conrimaceogain commented 3 years ago

(Copying this from my report in the Pollinators slack)

When running libhoney (via the Beeline) as a part of an rspec suite with webmock enabled, webmock automatically enables the Excon mocking middleware. This causes any spans that would normally be sent to Honeycomb to fail (with exceptions getting silently swallowed) as the 'api.honeycomb.io' endpoint was not stubbed. Adding WebMock.disable_net_connect!(allow: 'api.honeycomb.io') to the spec_helper mitigated the issue somewhat, but there was some interesting behavior in webmock that appeared to reset that occasionally (potentially an issue with our test suite). Adding WebMock::HttpLibAdapters::ExconAdapter.disable! completely resolve the problem, allowing all spans to be sent to Honeycomb.

In my examination of the issue, it looked like the exceptions were being swallowed here: https://github.com/honeycombio/libhoney-rb/blob/5a10ab5e206b4d40ad9c9cafb25986edf369f6e5/lib/libhoney/transmission.rb#L103

ajvondrak commented 3 years ago

Link to the Pollinators discussion: https://honeycombpollinators.slack.com/archives/CJR134U2F/p1618435375045500

Summarizing for posterity:

I looked into the possibility of re-raising the WebMock::NetConnectNotAllowedError instead of swallowing it in the rescue Exception clause. This itself highlighted a spot in the test suite where requests weren't being stubbed! https://github.com/honeycombio/libhoney-rb/blob/5a10ab5e206b4d40ad9c9cafb25986edf369f6e5/test/libhoney_test.rb#L523-L534

However, re-raising the webmock error only halts the Libhoney::TransmissionClient#send_loop background thread, which won't necessarily bubble up to the level of the rspec suite. Since the tests & send loop are running in different threads, the most we might be able to do is log something from the background thread - say, to stderr. That functionality technically already exists: https://github.com/honeycombio/libhoney-rb/blob/5a10ab5e206b4d40ad9c9cafb25986edf369f6e5/lib/libhoney/transmission.rb#L104-L109

The UX for this is pretty bad, though (hidden away behind the LOG_LEVEL environment variable and/or manually monitoring the response queue). We're currently having internal discussions about how best to address the more general problem of communicating when/why libhoney is failing. This use case is very valuable for our discussion, and will definitely shape our future updates. We'll let you know when we have better docs/code around these problems!

pkanal commented 2 years ago

Hello, We are trimming our OSS backlog. We will be closing this issue as it is a low priority for us to fix. It is unlikely that we'll ever get to it, and so we'd like to set expectations accordingly.

If this issue is important to you, please feel free to ping here and we can discuss/re-open.