-
Notifications
You must be signed in to change notification settings - Fork 555
feat(tracing): Improve @trace
decorator.
#4648
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
Open
antonpirker
wants to merge
53
commits into
master
Choose a base branch
from
antonpirker/manual-instrumentation
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+118
−56
Open
Changes from all commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
1162254
First version of helpers for better manual instrumentation
antonpirker 4346db1
minor improvements
antonpirker bfc6626
A nicer implementation
antonpirker f193ecb
style
antonpirker 8488a53
fixing setting of name
antonpirker 5621f15
Some linting
antonpirker 43ff0c6
Replaced original trace decorator with new_decorator
antonpirker 6a5c88e
Cleanup
antonpirker 74941d4
fix start child
antonpirker 6d36b28
If no span, do nothing
antonpirker 09d5646
change less code
antonpirker 3020f92
comment
antonpirker acd5401
its description internally
antonpirker 695dd06
move _set_span_attribuets into span
antonpirker 5068c5b
typing
antonpirker 98c8d42
.
antonpirker 35e7809
Add enum for template
antonpirker a538d2a
Make some consts public
antonpirker 241d5cf
Also make op part of public api
antonpirker 33f79b8
sort
antonpirker dbbf3b4
Allow setting a key to None
antonpirker cfec8c0
Merge branch 'master' into antonpirker/manual-instrumentation
antonpirker c234dd4
typing
antonpirker cf5382f
Guess some attributes from input/response
antonpirker fdcdfdf
fix tool description
antonpirker 9917808
Added more token names
antonpirker 7c1076b
better span naming
antonpirker a6eed59
Tool input and output
antonpirker 2011ab4
Check PII settings before adding tool input/output
antonpirker 668a421
Better usage extraction
antonpirker d8b5838
cleanup
antonpirker 94796c6
make attribute a string
antonpirker ee845b1
cleanup
antonpirker c2d47bc
renaming
antonpirker 5e7d44b
Merge branch 'master' into antonpirker/manual-instrumentation
antonpirker 4843674
update
antonpirker f0d3ab6
cleanup
antonpirker a75954e
Merge branch 'master' into antonpirker/manual-instrumentation
antonpirker 58a1646
apidocs
antonpirker 4002e86
Merge branch 'master' into antonpirker/manual-instrumentation
antonpirker 989e0be
apidoc
antonpirker aa82162
apidocs
antonpirker 0c3da75
typo
antonpirker 55a1035
default value
antonpirker 0b84d48
remove ai stuff
antonpirker 35d8f12
remove template stuff
antonpirker 10e82cd
remove
antonpirker c2d4e48
removed too much
antonpirker 9a63d8c
cleanup
antonpirker c20429b
Apply suggestions from code review
antonpirker 2d93a65
typing
antonpirker b0cc71a
typing
antonpirker 42afdfa
moved up
antonpirker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
import contextlib | ||
import functools | ||
import inspect | ||
import os | ||
import re | ||
import sys | ||
from collections.abc import Mapping | ||
from datetime import timedelta | ||
from decimal import ROUND_DOWN, Decimal, DefaultContext, localcontext | ||
from functools import wraps | ||
from random import Random | ||
from urllib.parse import quote, unquote | ||
import uuid | ||
|
@@ -770,70 +770,86 @@ def normalize_incoming_data(incoming_data): | |
return data | ||
|
||
|
||
def start_child_span_decorator(func): | ||
# type: (Any) -> Any | ||
def create_span_decorator(op=None, name=None, attributes=None): | ||
antonpirker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# type: (Optional[str], Optional[str], Optional[dict[str, Any]]) -> Any | ||
""" | ||
Decorator to add child spans for functions. | ||
Create a span decorator that can wrap both sync and async functions. | ||
|
||
See also ``sentry_sdk.tracing.trace()``. | ||
:param op: The operation type for the span. | ||
:param name: The name of the span. | ||
:param attributes: Additional attributes to set on the span. | ||
Comment on lines
+778
to
+780
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we intend to add more parameters in the future? How were these ones selected? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there will be a |
||
""" | ||
# Asynchronous case | ||
if inspect.iscoroutinefunction(func): | ||
|
||
@wraps(func) | ||
async def func_with_tracing(*args, **kwargs): | ||
# type: (*Any, **Any) -> Any | ||
def span_decorator(f): | ||
# type: (Any) -> Any | ||
""" | ||
Decorator to create a span for the given function. | ||
""" | ||
|
||
span = get_current_span() | ||
@functools.wraps(f) | ||
async def async_wrapper(*args, **kwargs): | ||
# type: (*Any, **Any) -> Any | ||
current_span = get_current_span() | ||
|
||
if span is None: | ||
if current_span is None: | ||
logger.debug( | ||
"Cannot create a child span for %s. " | ||
"Please start a Sentry transaction before calling this function.", | ||
qualname_from_function(func), | ||
qualname_from_function(f), | ||
) | ||
return await func(*args, **kwargs) | ||
return await f(*args, **kwargs) | ||
|
||
span_op = op or OP.FUNCTION | ||
span_name = name or qualname_from_function(f) or "" | ||
|
||
with span.start_child( | ||
op=OP.FUNCTION, | ||
name=qualname_from_function(func), | ||
): | ||
return await func(*args, **kwargs) | ||
with current_span.start_child( | ||
op=span_op, | ||
name=span_name, | ||
) as span: | ||
span.update_data(attributes or {}) | ||
result = await f(*args, **kwargs) | ||
return result | ||
|
||
try: | ||
func_with_tracing.__signature__ = inspect.signature(func) # type: ignore[attr-defined] | ||
async_wrapper.__signature__ = inspect.signature(f) # type: ignore[attr-defined] | ||
except Exception: | ||
pass | ||
|
||
# Synchronous case | ||
else: | ||
|
||
@wraps(func) | ||
def func_with_tracing(*args, **kwargs): | ||
@functools.wraps(f) | ||
def sync_wrapper(*args, **kwargs): | ||
# type: (*Any, **Any) -> Any | ||
current_span = get_current_span() | ||
|
||
span = get_current_span() | ||
|
||
if span is None: | ||
if current_span is None: | ||
logger.debug( | ||
"Cannot create a child span for %s. " | ||
"Please start a Sentry transaction before calling this function.", | ||
qualname_from_function(func), | ||
qualname_from_function(f), | ||
) | ||
return func(*args, **kwargs) | ||
return f(*args, **kwargs) | ||
|
||
span_op = op or OP.FUNCTION | ||
span_name = name or qualname_from_function(f) or "" | ||
|
||
with span.start_child( | ||
op=OP.FUNCTION, | ||
name=qualname_from_function(func), | ||
): | ||
return func(*args, **kwargs) | ||
with current_span.start_child( | ||
op=span_op, | ||
name=span_name, | ||
) as span: | ||
span.update_data(attributes or {}) | ||
result = f(*args, **kwargs) | ||
return result | ||
|
||
try: | ||
func_with_tracing.__signature__ = inspect.signature(func) # type: ignore[attr-defined] | ||
sync_wrapper.__signature__ = inspect.signature(f) # type: ignore[attr-defined] | ||
except Exception: | ||
pass | ||
|
||
return func_with_tracing | ||
if inspect.iscoroutinefunction(f): | ||
return async_wrapper | ||
else: | ||
return sync_wrapper | ||
|
||
return span_decorator | ||
antonpirker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def get_current_span(scope=None): | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Circular Import Issue in Tracing Module
The new top-level import of
create_span_decorator
insentry_sdk/tracing.py
creates a circular import dependency withsentry_sdk/tracing_utils.py
.tracing_utils.py
imports fromtracing.py
at its bottom, whiletracing.py
now imports fromtracing_utils.py
at its top. This breaks the existing pattern for handling circular imports (which previously used a local import within thetrace()
function) and can cause module loading failures or import order issues.