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

Tests named test_handles_backoff_v2_api fail when backoff is <2.0 #3087

Closed
musicinmybrain opened this issue Dec 11, 2022 · 1 comment · Fixed by #3106
Closed

Tests named test_handles_backoff_v2_api fail when backoff is <2.0 #3087

musicinmybrain opened this issue Dec 11, 2022 · 1 comment · Fixed by #3106
Labels
bug Something isn't working exporters help wanted

Comments

@musicinmybrain
Copy link
Contributor

Describe your environment

I’m working on updating the python-opentelemetry package in Fedora Linux (Rawhide, the development version). We still have python-backoff 1.10.0, not 2.x. Python is at 3.11.1.

Steps to reproduce

I wish I could provide more useful instructions to actually reproduce this, but I think it should happen whenever the installed backoff is pre-2.0.

In our environment, the tests test_handles_backoff_v2_api, namely

  • TestOTLPSpanExporter.test_handles_backoff_v2_api
  • TestOTLPHTTPLogExporter.test_handles_backoff_v2_api
  • TestOTLPMetricExporter.test_handles_backoff_v2_api

in exporter/opentelemetry-exporter-otlp-proto-grpc and exporter/opentelemetry-exporter-otlp-proto-http fail with output like:

_______________ TestOTLPSpanExporter.test_handles_backoff_v2_api _______________

self = <tests.test_otlp_trace_exporter.TestOTLPSpanExporter testMethod=test_handles_backoff_v2_api>
mock_sleep = <MagicMock name='sleep' id='139741991915664'>
mock_backoff = <MagicMock name='backoff' id='139741930872336'>

    @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.backoff")
    @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.sleep")
    def test_handles_backoff_v2_api(self, mock_sleep, mock_backoff):
        # In backoff ~= 2.0.0 the first value yielded from expo is None.
        def generate_delays(*args, **kwargs):
            yield None
            yield 1

        mock_backoff.expo.configure_mock(**{"side_effect": generate_delays})

        add_TraceServiceServicer_to_server(
            TraceServiceServicerUNAVAILABLE(), self.server
        )
        self.exporter.export([self.span])
>       mock_sleep.assert_called_once_with(1)

exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py:453:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <MagicMock name='sleep' id='139741991915664'>, args = (1,), kwargs = {}
msg = "Expected 'sleep' to be called once. Called 2 times.\nCalls: [call(None), call(1)]."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'sleep' to be called once. Called 2 times.
E           Calls: [call(None), call(1)].

/usr/lib64/python3.11/unittest/mock.py:944: AssertionError

What is the expected behavior?

It seems like this test doesn’t quite work when backoff is not 2.x, and it probably should.

What is the actual behavior?

The test fails in our environment with backoff < 2.0.

Additional context

I’m not sure what fix to propose here. It seems like the test is intended to emulate backoff v2 regardless of the installed backoff version, so perhaps skipping the test or modifying generate_delays()

# In backoff ~= 2.0.0 the first value yielded from expo is None.
def generate_delays(*args, **kwargs):
yield None
yield 1

would be missing the point. On the other hand, it’s not immediately obvious what a better fix should be.

For now, in the Fedora package, I’m going to skip these tests because it seems that this is a testing problem rather than a library problem. In the medium term, we should get our python-backoff package updated.

I’m happy to contribute a PR if you can suggest a sensible approach. Otherwise I will just keep skipping these tests downstream until we get python-backoff updated.

@musicinmybrain musicinmybrain added the bug Something isn't working label Dec 11, 2022
@srikanthccv
Copy link
Member

The generate_delays should be updated to emulate the backoff 1.x and 2.x behaviour conditionally based on the installed version.

musicinmybrain added a commit to musicinmybrain/opentelemetry-python that referenced this issue Jan 1, 2023
musicinmybrain added a commit to musicinmybrain/opentelemetry-python that referenced this issue Jan 1, 2023
musicinmybrain added a commit to musicinmybrain/opentelemetry-python that referenced this issue Jan 1, 2023
gen-xu pushed a commit to gen-xu/opentelemetry-python that referenced this issue Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exporters help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants