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.

Sign up for GitHub

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

Jump to bottom

USDT Python API and example #624

Merged
merged 4 commits into from
Jul 23, 2016
Merged

USDT Python API and example #624

merged 4 commits into from
Jul 23, 2016

Conversation

brendangregg
Copy link
Member

Code from @vmg

brendangregg referenced this pull request Jul 22, 2016
@vmg
[WIP] Port the Python examples to the new USDT API
7cec2b8
@4ast
Copy link
Member

4ast commented Jul 22, 2016

lgtm, but I'm not a python guy. @markdrayton could you please review?

markdrayton
markdrayton reviewed Jul 22, 2016
View reviewed changes
src/python/bcc/usdt.py
return normalized
def enable_probe(self, probe, fn_name):
if lib.bcc_usdt_enable_probe(self.context, probe, fn_name) != 0:
raise Exception("failed to enable probe '%s'", probe)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be % instead of , otherwise we throw a tuple:

$ sudo python ../examples/tracing/nodejs_http_server.py 25549
..
Exception: ("failed to enable probe '%s'", 'http__server__request')

@markdrayton
Copy link
Contributor

Looks good but I am still trying to test it locally. It looks like tplist.py and trace.py need updating to not call USDTReader now it was replaced with USDT:

$ grep -r USDTReader
tools/trace.py:                reader = USDTReader(bin_path=self.library)
tools/tplist.py:from bcc import USDTReader
tools/tplist.py:        reader = USDTReader(bin_path=lib, pid=pid)

Trivially replacing USDTReader with USDT shows at least one more snag:

$ /usr/share/bcc/tools/tplist -p 25549
'USDT' object has no attribute 'probes'

@brendangregg
Copy link
Member Author

Thanks; @goldshtn has committed to fix his tools after this is merged #621 :)

@markdrayton
Copy link
Contributor

Alrighty then. LGTM!

@4ast 4ast merged commit 4f88a94 into iovisor:master Jul 23, 2016
@brendangregg brendangregg mentioned this pull request Jul 23, 2016
Closed
@4ast
Copy link
Member

4ast commented Sep 8, 2016

@goldshtn tplist and trace are still broken...

@goldshtn
Copy link
Collaborator

goldshtn commented Sep 8, 2016

Sorry, it's been a crazy summer. Still on my list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brendangregg @4ast @markdrayton @goldshtn