Feature/ticket194 #201
Feature/ticket194 #201
Conversation
…on code to an application using autoinstrumentation.
… are only initialized once.
|
|
||
| modules_array = MODULES.split(',') | ||
|
|
||
| if len(modules_array) == 1 and modules_array[0] == '': |
There was a problem hiding this comment.
Is modules_array[0] == '' needed? Not sure how split works in python.
There was a problem hiding this comment.
If an empty string is given, the result is an array with one element that is a blank string. To make it more intuitive, I changed the logic to check for a blank string before calling split.
$ cat test.py
str1 = "This is a test"
str_array = str1.split(' ')
print(str(str_array))
str2 = "Thisisatest"
str_array2 = str2.split(' ')
print(str(str_array2))
str3 = ""
str_array3 = str3.split(' ')
print(str(str_array3))
$ python test.py
['This', 'is', 'a', 'test']
['Thisisatest']
['']
|
|
||
| def is_registered(self, module: str) -> bool: | ||
| '''Is an instrumentation module already registered?''' | ||
| try: |
There was a problem hiding this comment.
I think it is pretty straighforward to check if the key exists and then return the value instead of throwing an exception. In general I would only expect an exception thrown for things I can't control.
There was a problem hiding this comment.
It may be straightforward, but python throws a KeyError exception if an invalid key is provided and the keys are based on external input. You can see this behavior below.
So, to avoid this disrupting the flow of logic, a try-except clause that returns False on any exception is used.
$ cat test.py
tester = {
'test1':'test1',
'test2':'test2',
'test3':'test3'
}
print(str(tester['test1']))
print(str(tester['test4']))
$ python test.py
test1
Traceback (most recent call last):
File "test.py", line 8, in
print(str(tester['test4']))
KeyError: 'test4'
…THONPATH update logic into a separate function.
…) vs instrument(). Added detailed explanation of how it is supposed to owrik in comments.
|
All issues have been addressed or explained. |
| - Use the autoinstrumentation CLI | ||
| ``` | ||
| HT_INSTRUMENTED_MODULES=flask,mysql | ||
| hypertrace-instrument python app.py |
There was a problem hiding this comment.
If instrumenting a flask app, would hypertrace-instrument flask run be the more common use case?
There was a problem hiding this comment.
double tapping this comment. We should provide examples on how to launch with and without the autoinstrumentation (hypertrace-instrument)
There was a problem hiding this comment.
we should document starting up with hypertrace-instrument flask run to avoid confusing people accustomed to launching flask apps using the flask cli
| agent.registerFlaskApp(app) # instrument a flask application | ||
| agent.registerMySQL() # instrument the MySQL client | ||
| agent.register_flask_app(app) # instrument a flask application | ||
| agent.register_mysql() # instrument the MySQL client |
Description
Testing
Documentation