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

set_tags missing from python SDK #1344

Closed
bityob opened this issue Feb 16, 2022 · 5 comments · Fixed by #2978
Closed

set_tags missing from python SDK #1344

bityob opened this issue Feb 16, 2022 · 5 comments · Fixed by #2978
Assignees
Labels
Enhancement New feature or request Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! Triaged Has been looked at recently during old issue triage

Comments

@bityob
Copy link

bityob commented Feb 16, 2022

According to unified-api page, each SDK should have a set_tags method.

https://develop.sentry.dev/sdk/unified-api/

image

I found it was dicscussed and implemented 2 years ago in this PR #530 but the PR was closed.

Can this feature be added?

Thanks

@sl0thentr0py
Copy link
Member

thx for the issue @bityob, we know we have to generally clean up the tags/context/user API a bit. We'll try to fit this at some point. See also #933.

@VojtechBartos
Copy link

VojtechBartos commented Apr 8, 2022

@sl0thentr0py is there any update? :)

We are currently thinking how to extend our contextual logging (works in similar fashion as scope stacking in Sentry), but we can't use push_scope cause it's not automatically reporting errors.

So we ended up with using configure_scopeand following

  • capturing scope tags on __enter__
  • setting up specific tags overrides for actual functionality in the context
  • if anything raises exception in the context manager it will have the new tags
  • re-hydrating the captured scope tags on __exit__ to get it to previous state

We are having a pretty much nested context managers and works pretty well, but there is a downside of using private scope attribute _tags given there is not getter.

Would it be okay for you guys to add set_tags and along with that get_tags to Scope? If yes, i can make that change pretty fast. :) ⏩ :shipit:

Thanks

@bityob
Copy link
Author

bityob commented Aug 16, 2022

@sl0thentr0py is there any update? :)

We are currently thinking how to extend our contextual logging (works in similar fashion as scope stacking in Sentry), but we can't use push_scope cause it's not automatically reporting errors.

So we ended up with using configure_scopeand following

  • capturing scope tags on __enter__
  • setting up specific tags overrides for actual functionality in the context
  • if anything raises exception in the context manager it will have the new tags
  • re-hydrating the captured scope tags on __exit__ to get it to previous state

We are having a pretty much nested context managers and works pretty well, but there is a downside of using private scope attribute _tags given there is not getter.

Would it be okay for you guys to add set_tags and along with that get_tags to Scope? If yes, i can make that change pretty fast. :) fast_forward :shipit:

Thanks

Can you share your context manager?

Will be helpful maybe for us too

@bityob
Copy link
Author

bityob commented Aug 16, 2022

Ok, I came up with this context manager to handle sentry tags -

@contextmanager
def sentry_tags_session(logger=None, **tags):
    with sentry_sdk.configure_scope() as scope:
        try:
            curr_tags = scope._tags.copy()
            for k, v in tags.items():
                scope.set_tag(k, v)
            yield
        except:
            if logger:
                # Log here to have the exception attached with the tags too
                logger.exception("Failed inside sentry_tags_session context")
            raise
        finally:
            # Must clear explicitly the scope, because it's not handled by sentry_sdk,
            # and need also to restore the previous sentry tags because calling `clear()` remove all tags...
            scope.clear()
            for k, v in curr_tags.items():
                scope.set_tag(k, v)

@VojtechBartos
Copy link

@bityob we have something similar :)

@contextmanager
def create_logging_ctx(context: LoggingContext) -> Generator[None, None, None]:
    # NOTE(vojta) unfortunately we can't use Sentry push_scope given the fact it
    # wont automatically capture any excentions happening inside the push_scope context
    # manager and you have to do it manually. For there reason we have ended up with
    # using configure_scope, which actually tweaks the root scope, but with leaving
    # context manager keep the values. To make it work, we are capturing the actual tags
    # on entering and re-hydrating them on exiting the context manager
    with sentry_sdk.configure_scope() as scope:
        # there is no official API how to get all the scope tags se we have to use
        # private attribute and in meantime we have asked about potentially extending
        # the API
        # https://github.com/getsentry/sentry-python/issues/1344#issuecomment-1092700348
        captured_tags = dict(scope._tags)

        _LOGGING_CONTEXTS.append(context)

        # setting up the sentry tags to be able to see the information on the issues and
        # be able to search by them. Context not suitable for this purpose
        # https://docs.sentry.io/platforms/python/enriching-events/tags/
        for tag_name, tag_value in context.tags.items():
            scope.set_tag(tag_name, tag_value)

        # in case that wrapped funcionality inside by the logging context trigger any
        # exception we need to make sure we always do clean up aka remove Sentry scopes
        # and logging context
        try:
            yield
        finally:
            # re-hydrating the captured tags only for the keys which has been set from
            # the logging context
            for tag_name in context.tags.keys():
                value = captured_tags.get(tag_name)
                if value is None:
                    # value set from logging context, but not in the original captured logs
                    # so instead of rewriting, just removing
                    scope.remove_tag(tag_name)
                    continue
                scope.set_tag(tag_name, value)

            _LOGGING_CONTEXTS.pop()

@sentrivana sentrivana added Triaged Has been looked at recently during old issue triage Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! labels Oct 23, 2023
@szokeasaurusrex szokeasaurusrex self-assigned this Apr 15, 2024
szokeasaurusrex added a commit that referenced this issue Apr 15, 2024
`Scope.set_tags` allows multiple tags to be set at the same time by passing the tags to update as a dictionary (or other `Mapping` type).

Closes GH-1344
@szokeasaurusrex szokeasaurusrex linked a pull request Apr 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest! Triaged Has been looked at recently during old issue triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants