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

(1) Add new scopes related API (but not yet calling it) #2609

Closed
wants to merge 97 commits into from

Conversation

antonpirker
Copy link
Member

@antonpirker antonpirker commented Dec 19, 2023

This adds the API for the new Scopes to the code base but does not call/use it.

The ideas behind the new Scopes implementation:

  • The current and the isolation scope are saved as ContextVars
  • The global scope is a global variable
  • In the new context managers with new_scope() and with isolated_scope() the current and/or isolation scope are forked (meaning copied) and given to the code block wrapped with the context manager. On exit of the code block the current and/or isolation scope are reset to the old value that they had before the with block. (This is basically the same functionality as the push and pop scope of the stack in the current implementation)
  • A forked scope holds a reference to the scope it was forked from (the original_scope). We need this to implement copy-on-write functionality in forked scopes. As long as you do not change the data in the forked scope, all the data in the forked scope is still the same references as in the original scope. If you manipulate the data, a copy of the data is made and this copy is then changed. (implemented in the _copy_on_write decorator, that will be applied to the properties of the Scope when the API is actually implemented. (this PR))
  • There will always be a client in this implementation. The standard client will be referenced in the global scope. If the SDK is not yet initialized there will also be a client, but it will be a NoopClient meaning it can be used as a normal client, but it will not send any data to Sentry.
  • There is also the possibility to set a custom client on the Scope (possible on current, isolation and global scope), so one could use multiple clients to send data to different targets

There is a follow up PR where the API added in this PR is actually implemented: #2610

Moved some functionality from Hub to Scope or Client:
- moved `add_breadcrumb` from Hub to Scope
- moved session functions from Hub to Scope
- moved `get_integration1` from Hub to Client.

This is preparation work for refactoring how we deal with Hubs and Scopes in the future.
Each commit is moving one function, so it should be easy to review commit by commit.
…)` (#2570)

Improves the capabilities of our threadlocal context variables, we use when no "real" context variables are available (for example in old Python versions)

- Adds a no-op [copy_context](https://docs.python.org/3/library/contextvars.html#contextvars.copy_context) function to use in environments without context vars
- Adds [reset](https://docs.python.org/3/library/contextvars.html#contextvars.ContextVar.reset) functionality to our threadlocal based context vars.

This is preparation work for refactoring how we deal with Hubs and Scopes in the future.
Moved some functionality from Hub to Client:

- Capture Event:
  - moved `capture_event` from Hub to Client
  - created new `capture_event` in Scope that calls `capture_event` in Client
  - made `capture_event` in Hub call the new `capture_event` in Scope

- Capture Exception:
  - created new `capture_exception` in Scope
  - made `capture_exception` in Hub call the new one in Scope

- Capture Message:
  - created new `capture_message` in Scope
  - made `capture_message` in Hub call the new one in Scope

- renamed `**scope_args` to `**scope_kwargs` because it contains keyword arguments.
- moved `_update_scope` from Hub to Scope and renamed it to `_merge_scopes` 

This is preparation work for refactoring how we deal with Hubs and Scopes in the future.
Its properly easier to reason about this change when checking out the branch than looking at the diff.
Moved some functionality from Hub to Client:

- sorted some typing imports
- moved `get_traceparent` from Hub to Scope
- moved `get_baggage` from Hub to Scope
- moved `iter_trace_propagation_headers` from Hub to Scope
- moved `trace_propagation_meta` from Hub to Scope

This is preparation work for refactoring how we deal with Hubs and Scopes in the future.
@antonpirker antonpirker changed the title Add new scopes related API (1) Add new scopes related API Dec 19, 2023
@antonpirker antonpirker marked this pull request as ready for review December 20, 2023 09:37
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

i need to think some of the scope stuff through later but I already left some comments about the client. We need to clean this up before I'm happy with it.

sentry_sdk/api.py Outdated Show resolved Hide resolved
sentry_sdk/api.py Outdated Show resolved Hide resolved
sentry_sdk/api.py Outdated Show resolved Hide resolved
sentry_sdk/client.py Outdated Show resolved Hide resolved
sentry_sdk/client.py Outdated Show resolved Hide resolved
sentry_sdk/scope.py Outdated Show resolved Hide resolved
sentry_sdk/scope.py Show resolved Hide resolved
sentry_sdk/scope.py Show resolved Hide resolved
sentry_sdk/scope.py Outdated Show resolved Hide resolved
sentry_sdk/__init__.py Show resolved Hide resolved
sentry_sdk/api.py Outdated Show resolved Hide resolved
sentry_sdk/api.py Outdated Show resolved Hide resolved
sentry_sdk/client.py Outdated Show resolved Hide resolved
sentry_sdk/client.py Show resolved Hide resolved
sentry_sdk/scope.py Outdated Show resolved Hide resolved
sentry_sdk/scope.py Show resolved Hide resolved
sentry_sdk/scope.py Outdated Show resolved Hide resolved
sentry_sdk/scope.py Outdated Show resolved Hide resolved
sentry_sdk/__init__.py Show resolved Hide resolved
antonpirker and others added 4 commits February 12, 2024 15:17
Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
Co-authored-by: Daniel Szoke <szokeasaurusrex@users.noreply.github.com>
@antonpirker
Copy link
Member Author

Closing this PR in favor of #2610

All the changes from this PR are in the linked PR now.

antonpirker added a commit that referenced this pull request Feb 26, 2024
This refactors the SDK to move away from the Hub and have all the functionality in the Scope. Introducing different types of scopes. This aligns the SDK with how Opentelementry (OTel) handles data bringing us closer to be 100% OTel compatible.

This change was discussed in this RFC: 
getsentry/rfcs#122

There is also a small FAQ: 
https://gist.github.com/mitsuhiko/1bc78d04ea7d08e5b50d27e42676db80

And a Miro board showing how the new scopes manage data: 
https://miro.com/app/board/uXjVNtPiOfI=/?share_link_id=216270218892

### This RP contains
- Introduction of global, isolation, and current scope
- Deprecation of the Hub
- All existing Hub based API still works and is still used by most of our integrations. Under the hood the new Scopes are used.
- (this PR now includes all the changes made in the [first PR](#2609) introducing the new API)

### Breaking changes
- The Pyramid integration will not capture errors that might happen in `authenticated_userid()` in a custom `AuthenticationPolicy` class.
- The parameter `propagate_hub` in `ThreadingIntegration()` was deprecated and renamed to `propagate_scope`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants