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.

Sign up for GitHub

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

🐛 FIX #52676 update sentry scope configuration to remove deprecation wa… #53398

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

🐛 FIX #52676 update sentry scope configuration to remove deprecation wa… #53398

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a161a86
FIX #52676 update sentry scope configuration to remove deprecation wa…
saimadib Feb 8, 2025
7611201
Merge branch 'master' into saimadib/fix-sentry-deprecation-warning
saimadib Feb 8, 2025
a48cc7f
FIX #52676 add docstrings to Sentry context decorators for improved r…
saimadib Feb 9, 2025
ac9749b
Merge branch 'saimadib/fix-sentry-deprecation-warning' of https://git…
saimadib Feb 9, 2025
fe98425
Merge branch 'master' into saimadib/fix-sentry-deprecation-warning
saimadib Feb 9, 2025
920e429
Merge branch 'master' into saimadib/fix-sentry-deprecation-warning
saimadib Feb 11, 2025
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
74 changes: 37 additions & 37 deletions airbyte-ci/connectors/pipelines/pipelines/helpers/sentry_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,55 +38,55 @@ def before_send(event: Dict[str, Any], hint: Dict[str, Any]) -> Optional[Dict[st

def with_step_context(func: Callable) -> Callable:
def wrapper(self: Step, *args: Any, **kwargs: Any) -> Step:
with sentry_sdk.configure_scope() as scope:
step_name = self.__class__.__name__
scope.set_tag("pipeline_step", step_name)
scope = sentry_sdk.get_current_scope()
step_name = self.__class__.__name__
scope.set_tag("pipeline_step", step_name)
scope.set_context(
"Pipeline Step",
{
"name": step_name,
"step_title": self.title,
"max_retries": self.max_retries,
"max_duration": self.max_duration,
"retry_count": self.retry_count,
},
)

if hasattr(self.context, "connector"):
connector: Connector = self.context.connector
scope.set_tag("connector", connector.technical_name)
scope.set_context(
"Pipeline Step",
"Connector",
{
"name": step_name,
"step_title": self.title,
"max_retries": self.max_retries,
"max_duration": self.max_duration,
"retry_count": self.retry_count,
"name": connector.name,
"technical_name": connector.technical_name,
"language": connector.language,
"version": connector.version,
"support_level": connector.support_level,
},
)

if hasattr(self.context, "connector"):
connector: Connector = self.context.connector
scope.set_tag("connector", connector.technical_name)
scope.set_context(
"Connector",
{
"name": connector.name,
"technical_name": connector.technical_name,
"language": connector.language,
"version": connector.version,
"support_level": connector.support_level,
},
)

return func(self, *args, **kwargs)
return func(self, *args, **kwargs)

return wrapper


def with_command_context(func: Callable) -> Callable:
def wrapper(self: Command, ctx: Context, *args: Any, **kwargs: Any) -> Command:
with sentry_sdk.configure_scope() as scope:
scope.set_tag("pipeline_command", self.name)
scope.set_context(
"Pipeline Command",
{
"name": self.name,
"params": self.params,
},
)
scope = sentry_sdk.get_current_scope()
Copy link
Contributor

Choose a reason for hiding this comment

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

Dropping with here might mean you're not discarding scope once you're done, but honestly, I don't see lots of harm in this.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for bringing this up! I can clarify the implementation choice with reference to Sentry's official migration guide.
The change we made is exactly as recommended in Sentry's migration documentation. According to the guide, we should replace the deprecated scope configuration:

with sentry_sdk.configure_scope() as scope:
    # do something with scope

with the new recommended approach:

scope = sentry_sdk.get_current_scope()
# do something with scope

While the with statement might seem to provide better scope cleanup, the new get_current_scope() method is specifically designed to work with Sentry's scope management system in version 2.x.

scope.set_tag("pipeline_command", self.name)
scope.set_context(
"Pipeline Command",
{
"name": self.name,
"params": self.params,
},
)

scope.set_context("Click Context", ctx.obj)
scope.set_tag("git_branch", ctx.obj.get("git_branch", "unknown"))
scope.set_tag("git_revision", ctx.obj.get("git_revision", "unknown"))
scope.set_context("Click Context", ctx.obj)
scope.set_tag("git_branch", ctx.obj.get("git_branch", "unknown"))
scope.set_tag("git_revision", ctx.obj.get("git_revision", "unknown"))

return func(self, ctx, *args, **kwargs)
return func(self, ctx, *args, **kwargs)

return wrapper
Loading