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

feat: add example using Sentry V2 SDK #140

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.venv
__pycache__
.idea
22 changes: 13 additions & 9 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ dependencies = { pydantic = "^1.10.4" }

[tool.poetry.group.sentry]
optional = true
dependencies = { sentry-sdk = "^1.11.0" }
dependencies = { sentry-sdk = [
{version = "^1.11.0", python = "<3.6" },
{version = "^2.13.0", python = ">=3.6" }
]}

[tool.poe.tasks]
format = [{cmd = "black ."}, {cmd = "isort ."}]
Expand Down
33 changes: 33 additions & 0 deletions sentry_v2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Sentry V2 Sample

This sample shows how to configure [Sentry](https://sentry.io) SDK V2 to intercept and capture errors from the Temporal SDK.

Note: This is a small modification of the original example [sentry (v1)](../sentry) which seems to have
issues in the Workflow interceptor, as there are modules (`warnings` and `threading`) that Sentry uses that aren't passed through
to Temporal's sandbox properly, causing the Workflows to fail to execute.
Copy link
Member

Choose a reason for hiding this comment

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

If Sentry V1 doesn't work anymore, let's replace the sample instead of adding a new one

Copy link
Author

Choose a reason for hiding this comment

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

Hey @cretz - I haven't been able to test this, but I suspect now the example didn't work for me because when I installed sentry-sdk, it will have defaulted to the latest version (V2), but the example only works with V1. I imagine if you pin the version to the one in the example, it might work.

It does look like development on the V1 SDK has stopped now (last feature release was in April with a security patch in July).

So, I will let you decide if we should replace the V1 example. I'll update the PR as necessary.

Copy link
Member

@cretz cretz Sep 11, 2024

Choose a reason for hiding this comment

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

Hrmm, assuming the current sentry works, how about we just move the current one to sentry_v1 and make yours just sentry? So change the dirs and the dependency group name and the top-level README and such. Then we can come back and delete sentry_v1 later when they officially EOL it.

Copy link
Author

Choose a reason for hiding this comment

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

@cretz I renamed sentry -> sentry_v1 and kept sentry_v2 as is, so the commit history doesn't get skewed for the original impl. Let me know if you'd prefer to make sentry_v2 -> sentry.

Copy link
Member

Choose a reason for hiding this comment

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

Does Sentry still support v1? If it is EOL, maybe we should consider it EOL too.


> Sentry V2 only supports Python 3.6 and higher. So if you're on Python 3.5, you may need to refer to the original example
Copy link
Member

Choose a reason for hiding this comment

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

Temporal Python SDK only supports 3.8 or higher, so this disclaimer is not needed

Copy link
Author

Choose a reason for hiding this comment

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

@cretz I've removed and reworded both the readme's in the sample.

Btw, you cannot have two different poetry groups containing the same package with different versions. Previously, I added a constraint on Python, so SDK v1 was installed for Python<3.6 and SDK v2 for Python>=3.6. However, since the samples project uses Python 3.10 (and the Temporal SDK only supports 3.8 and higher), I think this hack would confuse users, as it would always install SDK v2 and the original sample wouldn't work as expected.

I've updated the V1 README to require the user to install sentry-sdk==1.11.0 with pip rather than using poetry.

Copy link
Member

Choose a reason for hiding this comment

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

you cannot have two different poetry groups containing the same package with different versions

Ug, that is unfortunate. I do still think we should have the Poetry groups named after the sample they represent, even if it's a dupe for sentry_v1 and sentry_v2 (or there isn't even a sentry_v1 group at all and pip install is manual). This also may give more credence to the approach of making sentry_v2 just sentry.

Copy link
Author

Choose a reason for hiding this comment

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

I've asked on the Sentry's discussion board what the timeline is for v1 support. Will get back once they provide an answer.

One idea would be to just replace the v1 impl. with the new v2 integration, keep the example called sentry, and then add a link in the README to the original v1 commit for posterity?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, that would work well. Either way I think we want this to be the primary sentry sample since we have to have the dependency group named that.

> (which doesn't work for me) or upgrade your Python version.

### Further details

Sentry's `Hub` object is now deprecated in the V2 SDK in favour of scopes. See [Activating Current Hub Clone](https://docs.sentry.io/platforms/python/migration/1.x-to-2.x#activating-current-hub-clone)
for more details. The changes are simple, just replace `with Hub(Hub.current):` with `with isolation_scope() as scope:`.

## Running the Sample

For this sample, the optional `sentry` dependency group must be included. To include, run:

poetry install --with sentry

To run, first see [README.md](../README.md) for prerequisites. Set `SENTRY_DSN` environment variable to the Sentry DSN.
Then, run the following from this directory to start the worker:

poetry run python worker.py

This will start the worker. Then, in another terminal, run the following to execute the workflow:

poetry run python starter.py

The workflow should complete with the hello result. If you alter the workflow or the activity to raise an
`ApplicationError` instead, it should appear in Sentry.
Empty file added sentry_v2/__init__.py
Empty file.
85 changes: 85 additions & 0 deletions sentry_v2/interceptor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
from dataclasses import asdict, is_dataclass
from typing import Any, Optional, Type, Union

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

with workflow.unsafe.imports_passed_through():
from sentry_sdk import isolation_scope, Scope


def _set_common_workflow_tags(scope: Scope, info: Union[workflow.Info, activity.Info]):
scope.set_tag("temporal.workflow.type", info.workflow_type)
scope.set_tag("temporal.workflow.id", info.workflow_id)


class _SentryActivityInboundInterceptor(ActivityInboundInterceptor):
async def execute_activity(self, input: ExecuteActivityInput) -> Any:
# https://docs.sentry.io/platforms/python/troubleshooting/#addressing-concurrency-issues
with isolation_scope() as scope:
scope.set_tag("temporal.execution_type", "activity")
scope.set_tag("module", input.fn.__module__ + "." + input.fn.__qualname__)

activity_info = activity.info()
_set_common_workflow_tags(scope, activity_info)
scope.set_tag("temporal.activity.id", activity_info.activity_id)
scope.set_tag("temporal.activity.type", activity_info.activity_type)
scope.set_tag("temporal.activity.task_queue", activity_info.task_queue)
scope.set_tag("temporal.workflow.namespace", activity_info.workflow_namespace)
scope.set_tag("temporal.workflow.run_id", activity_info.workflow_run_id)
try:
return await super().execute_activity(input)
except Exception as e:
if len(input.args) == 1 and is_dataclass(input.args[0]):
scope.set_context("temporal.activity.input", asdict(input.args[0]))
scope.set_context("temporal.activity.info", activity.info().__dict__)
scope.capture_exception()
raise e


class _SentryWorkflowInterceptor(WorkflowInboundInterceptor):
async def execute_workflow(self, input: ExecuteWorkflowInput) -> Any:
# https://docs.sentry.io/platforms/python/troubleshooting/#addressing-concurrency-issues
with isolation_scope() as scope:
scope.set_tag("temporal.execution_type", "workflow")
scope.set_tag("module", input.run_fn.__module__ + "." + input.run_fn.__qualname__)
workflow_info = workflow.info()
_set_common_workflow_tags(scope, workflow_info)
scope.set_tag("temporal.workflow.task_queue", workflow_info.task_queue)
scope.set_tag("temporal.workflow.namespace", workflow_info.namespace)
scope.set_tag("temporal.workflow.run_id", workflow_info.run_id)
try:
return await super().execute_workflow(input)
except Exception as e:
if len(input.args) == 1 and is_dataclass(input.args[0]):
scope.set_context("temporal.workflow.input", asdict(input.args[0]))
scope.set_context("temporal.workflow.info", workflow.info().__dict__)

if not workflow.unsafe.is_replaying():
with workflow.unsafe.sandbox_unrestricted():
scope.capture_exception()
raise e


class SentryInterceptor(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 _SentryActivityInboundInterceptor(super().intercept_activity(next))

def workflow_interceptor_class(
self, input: WorkflowInterceptorClassInput
) -> Optional[Type[WorkflowInboundInterceptor]]:
return _SentryWorkflowInterceptor
24 changes: 24 additions & 0 deletions sentry_v2/starter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import asyncio
import os

from temporalio.client import Client

from sentry.worker import GreetingWorkflow


async def main():
# Connect client
client = await Client.connect("localhost:7233")

# Run workflow
result = await client.execute_workflow(
GreetingWorkflow.run,
"World",
id="sentry-workflow-id",
task_queue="sentry-task-queue",
)
print(f"Workflow result: {result}")


if __name__ == "__main__":
asyncio.run(main())
64 changes: 64 additions & 0 deletions sentry_v2/worker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import asyncio
import logging
import os
from dataclasses import dataclass
from datetime import timedelta

import sentry_sdk
from temporalio import activity, workflow
from temporalio.client import Client
from temporalio.worker import Worker

from sentry.interceptor import SentryInterceptor


@dataclass
class ComposeGreetingInput:
greeting: str
name: str


@activity.defn
async def compose_greeting(input: ComposeGreetingInput) -> str:
activity.logger.info("Running activity with parameter %s" % input)
return f"{input.greeting}, {input.name}!"


@workflow.defn
class GreetingWorkflow:
@workflow.run
async def run(self, name: str) -> str:
workflow.logger.info("Running workflow with parameter %s" % name)
return await workflow.execute_activity(
compose_greeting,
ComposeGreetingInput("Hello", name),
start_to_close_timeout=timedelta(seconds=10),
)


async def main():
# Uncomment the line below to see logging
# logging.basicConfig(level=logging.INFO)

# Initialize the Sentry SDK
sentry_sdk.init(
dsn=os.environ.get("SENTRY_DSN"),
)

# Start client
client = await Client.connect("localhost:7233")

# Run a worker for the workflow
worker = Worker(
client,
task_queue="sentry-task-queue",
workflows=[GreetingWorkflow],
activities=[compose_greeting],
interceptors=[SentryInterceptor()], # Use SentryInterceptor for error reporting
)

await worker.run()


if __name__ == "__main__":
asyncio.run(main())