sentry-python icon indicating copy to clipboard operation
sentry-python copied to clipboard

KeyError: 'warnings' in catch_warnings

Open leonard-henriquez opened this issue 1 year ago • 2 comments

How do you use Sentry?

Sentry Saas (sentry.io)

Version

2.11.0

Steps to Reproduce

Steps:

  • Use sentry
  • In a temporal workflow
  • And issue a warning

Problem Description: The issue arises because the Sentry SDK assumes that the warnings module is always present. However, when running in a safe environment like Temporal (and other similar environments), this module may be restricted for security reasons, as these environments limit access to certain global state modules (like warnings).

Proposed Solution: Sentry should not assume the presence of the warnings module. The catch_warnings function should first check if the warnings module exists, and handle its absence gracefully. This adjustment should ensure better compatibility with secure environments like Temporal.

Expected Result

> poetry run python run_workflow.py "generate_follow_ups"

CLI 1.1.0 (Server 1.25.0, UI 2.30.3)

Server:  localhost:7233
Metrics: http://localhost:61488/metrics
Temporal environment and worker started. Press Ctrl+C to stop.
Workflow started with ID: c5835ec9-d4c3-4aba-919d-09e488ee2a78
time=2024-09-29T01:06:09.558 level=WARN msg="error in prometheus reporter" error="a previously registered descriptor with the same fully-qualified name as Desc{fqName: \"task_schedule_to_start_latency\", help: \"task_schedule_to_start_latency histogram\", constLabels: {}, variableLabels: {service_name,operation,namespace,taskqueue}} has different label names or a different help string"
2024-09-29T08:06:12.624266+00:00 [info     ] [GenerateFollowUps] Getting followups for the campaign 0191bd3b-e688-7350-802a-cbda765557ff. [main]
2024-09-29T08:06:13.603476+00:00 [info     ] [GetFollowUpsToGenerate] Found 100 follow ups to generate. [main] span={'trace_id': 46501998728614327432160006987081809693, 'span_id': 4107830682838171212, 'parent_span_id': 3188224582942380771}

Actual Result

> poetry run python run_workflow.py "generate_follow_ups"

CLI 1.1.0 (Server 1.25.0, UI 2.30.3)

Server:  localhost:7233
UI:      http://localhost:8233
Metrics: http://localhost:53003/metrics
Temporal environment and worker started. Press Ctrl+C to stop.
Workflow started with ID: 96826e23-7073-4134-9c9f-b62456e51d40
Failed activation on workflow GenerateFollowUps with ID 96826e23-7073-4134-9c9f-b62456e51d40 and run ID d0700809-fdb5-477b-9ace-0895e4715f40
Traceback (most recent call last):
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_workflow_instance.py", line 369, in activate
    self._run_once(check_conditions=index == 1 or index == 2)
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_workflow_instance.py", line 1767, in _run_once
    raise self._current_activation_error
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_workflow_instance.py", line 1785, in _run_top_level_workflow_function
    await coro
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_workflow_instance.py", line 849, in run_workflow
    result = await self._inbound.execute_workflow(input)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/contrib/opentelemetry.py", line 356, in execute_workflow
    return await super().execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_interceptor.py", line 328, in execute_workflow
    return await self.next.execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/testing/_workflow.py", line 493, in execute_workflow
    return await super().execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_interceptor.py", line 328, in execute_workflow
    return await self.next.execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/contrib/opentelemetry.py", line 356, in execute_workflow
    return await super().execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/_interceptor.py", line 328, in execute_workflow
    return await self.next.execute_workflow(input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/app/utils/worker/error_tracking.py", line 80, in execute_workflow
    with Hub(Hub.current):
             ^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/sentry_sdk/hub.py", line 103, in current
    with _suppress_hub_deprecation_warning():
  File "/Users/leonard/.asdf/installs/python/3.12.2/lib/python3.12/contextlib.py", line 137, in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/sentry_sdk/hub.py", line 87, in _suppress_hub_deprecation_warning
    with warnings.catch_warnings():
         ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/leonard/.asdf/installs/python/3.12.2/lib/python3.12/warnings.py", line 466, in __init__
    self._module = sys.modules['warnings'] if module is None else module
                   ~~~~~~~~~~~^^^^^^^^^^^^
  File "/Users/leonard/code/agent/apps/backend/.venv/lib/python3.12/site-packages/temporalio/worker/workflow_sandbox/_importer.py", line 393, in __getitem__
    return self.current[key]
           ~~~~~~~~~~~~^^^^^
KeyError: 'warnings'

leonard-henriquez avatar Sep 29 '24 08:09 leonard-henriquez

Actually I'm sorry I just notice that the error is coming from the Python standard library, not Sentry's SDK. But, is there anything you could do about it ?

leonard-henriquez avatar Sep 29 '24 08:09 leonard-henriquez

@leonard-henriquez would you be able to provide a minimal reproduction?

szokeasaurusrex avatar Sep 30 '24 07:09 szokeasaurusrex

I'm having the same issue, also trying to use sentry inside Temporal. @leonard-henriquez did you find any solution to this?

fisadev avatar Oct 24 '24 17:10 fisadev

One ugly solution I found, in case anyone reaches this issue: you can add import warnings to the execute_workflow method of your custom workflow interceptor, right before the code that tries to use Sentry. This is needed because deep down Sentry assumes warnings is already imported, but Temporal breaks that assumption .

fisadev avatar Oct 24 '24 19:10 fisadev

@leonard-henriquez @fisadev, can either of you provide a minimal reproduction of this issue? That would make it much easier for us to help out here

szokeasaurusrex avatar Oct 28 '24 10:10 szokeasaurusrex

@szokeasaurusrex I know and I'm sorry, I'm the kind of person who usually does that and provides way more info on issues. But in this particular case, I'm still learning Temporal (at my new job), which is quite a beast and somewhat still obscure for me, so I'm not sure how to setup a small example that's easy to run. I'll still try and post one if I'm successful.

fisadev avatar Oct 28 '24 18:10 fisadev

@fisadev No problem, no need to apologize.

Would it be helpful if I also try to look into whether I can reproduce this in the meantime? I've never used Temporal before, so I cannot really promise that I will find anything, but I can try if you would like

szokeasaurusrex avatar Oct 29 '24 10:10 szokeasaurusrex

@szokeasaurusrex I was able to get a minimal example to fail, using the Sentry interceptor in the official Temporal demos. I left everything in this repo, with steps to run it and solutions I found in the readme: https://github.com/fisadev/sentry_temporal_issue

The workflow and worker themselves are super simple. The interceptor is a bit more complex, but it's copied without changes from the official Temporal demo on how to integrate Sentry.

Let me know if I can help in any other way! :)

fisadev avatar Oct 31 '24 21:10 fisadev

Hey @fisadev, thank you for preparing the reproduction, and thank you for the detailed reproduction steps!

I was able to use your code to reproduce both of the errors that you described in the reproduction's README. However, after looking into both of them, I think these are probably bugs in Temporal or side effects of how Temporal's sandboxing works.

The first problem

The first error related to the warnings module seems to be occurring because Temporal somehow breaks the warnings.catch_warnings function, which we call in the Sentry SDK. You can see that this is not an issue with the Sentry SDK by replacing the SentryInterceptor with this FakeWarningsInterceptor I wrote:

import warnings
from typing import Any, Optional, Type

from temporalio.worker import (
    ActivityInboundInterceptor,
    ExecuteActivityInput,
    ExecuteWorkflowInput,
    Interceptor,
    WorkflowInboundInterceptor,
    WorkflowInterceptorClassInput,
)


class _FakeWarningsInboundInterceptor(ActivityInboundInterceptor):
    async def execute_activity(self, input: ExecuteActivityInput) -> Any:
        with warnings.catch_warnings():
            return await super().execute_activity(input)


class _FakeWarningsInboundInterceptor(WorkflowInboundInterceptor):
    async def execute_workflow(self, input: ExecuteWorkflowInput) -> Any:
        with warnings.catch_warnings():
            return await super().execute_workflow(input)


class FakeWarningsInterceptor(Interceptor):
    """Temporal Interceptor class which will report workflow & activity exceptions to Sentry"""

    def intercept_activity(
        self, next: ActivityInboundInterceptor
    ) -> ActivityInboundInterceptor:
        """Implementation of
        :py:meth:`temporalio.worker.Interceptor.intercept_activity`.
        """
        return _FakeWarningsInboundInterceptor(super().intercept_activity(next))

    def workflow_interceptor_class(
        self, input: WorkflowInterceptorClassInput
    ) -> Optional[Type[WorkflowInboundInterceptor]]:
        return _FakeWarningsInboundInterceptor

Basically, all the interceptor does is call warnings.catch_warnings. You will observe the same error as you did with the Sentry SDK.

The solution here is to avoid hitting the warnings.catch_warnings call in the Sentry SDK. You can do this by replacing the with Hub(Hub.current) lines with with isolation_scope(), which you can import with from sentry_sdk import isolation_scope. The Hub API is deprecated, so we would recommend avoiding using it, even if you did not have this error.

The second problem

I am a bit less sure why this problem is occurring. But, since you are able to work around the problem with Temporal's with workflow.unsafe.imports_passed_through(), my guess is that the problem occurs because the Sentry SDK is trying to do something which Temporal's sandboxing functionality blocks.

Since I am not familiar with Temporal, I am not sure what the best way is for you to work around this problem. But most likely, if you want to use Sentry with Temporal, you will need to disable any sandboxing features that prevent the Sentry SDK from doing what it needs to do. There is not much that we can do from our side. If you are stuck or think there is a bug here, I would advise you to raise an issue with Temporal.


Since this issue is not really actionable from our side, I am going to go ahead and close this issue. I would suggest raising these issues with Temporal, instead. But, if there is any other way we could help you out, please let me know!

szokeasaurusrex avatar Nov 04 '24 14:11 szokeasaurusrex

Thanks for taking a look at it and for the explanations! It makes sense to close the issue then, and I'll try to migrate to the isolation_scope context manager. If everything works, I'll also send a PR to the Temporal examples repo, so this doesn't bite more people.

fisadev avatar Nov 04 '24 19:11 fisadev