Centralized way to configure custom instrumentation#1960
Centralized way to configure custom instrumentation#1960antonpirker merged 15 commits intomasterfrom
Conversation
| # ex: "mymodule.submodule.MyClassName.member_function" | ||
|
|
||
| module_name, class_name = module_name.rsplit(".", 1) | ||
| module_obj = import_module(module_name) |
There was a problem hiding this comment.
this import_module is a lot of magic, because
- you might accidentally run user code that was not supposed to run just because sentry needs to know what's in that namespace
- is also a potential security issue / injection vector since you're letting users specify arbitrary modules and then actually performing IO on them. This is basically an
evalcall and we should be very careful of introducing such things in our codebase.
There was a problem hiding this comment.
Yes, true. If there is code executed on module load it could be have unintentional behavior.
But this optional and is only active when functions_to_trace is set.
What if we point out this concerns in the documentation, so people know about them when activating this feature?
There was a problem hiding this comment.
@mitsuhiko can I get your opinion on this? Should we avoid putting import_module in the SDK (although it needs to be enabled by default)?
There was a problem hiding this comment.
I’m okay with importing from strings. But I can’t say about the feature as such.
There was a problem hiding this comment.
Are you saying you can not say anything about the feature, or are you saying that you are not okay with the feature @mitsuhiko ?
There was a problem hiding this comment.
(On another note, @mdtro (Matthew T) said in slack: A note in the documentation is sufficient, imo. The list should be static or loaded from a secure source (no user or untrusted input).)
|
Documentation for this feature is here: getsentry/sentry-docs#6520 |
|
@antonpirker FYI the docs in getsentry/sentry-docs#6520 do not match your implementation in this PR |
|
Hey @davidgrohmann, thanks for pointing that out. I'll fix the docs. |
Have a list of functions that can be passed to "sentry_sdk.init()". When the SDK starts it goes through the list and instruments all the functions in the list.
Fixes #1963
Docs: getsentry/sentry-docs#6520
Fixes getsentry/team-webplatform-meta#19