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

feat: add GreenletProfiler #246

Merged
merged 12 commits into from
Nov 5, 2022
Merged

Conversation

jaychoww
Copy link
Contributor

No description provided.

@jaychoww
Copy link
Contributor Author

jaychoww commented Oct 25, 2022

it is a draft, but it can profiling with threading model and greenlet model automatically now. it needs more things:
test cases

@wu-sheng
Copy link
Member

Verifying through e2e profiling case would be a good place to start.

@wu-sheng
Copy link
Member

Profiling e2e has been included in the main repo, https://github.com/apache/skywalking/blob/317d539371c588c4754029134ad76087ecb6d0cd/.github/workflows/skywalking.yaml#L435-L454 powered by our e2e-infra tool, https://skywalking.apache.org/docs/skywalking-infra-e2e/next/readme/

You could learn how we run tests using the latest agent with real OAP/SkyWalking backend, and verify the result through SkyWalking CLI through this tool.

@jaychoww
Copy link
Contributor Author

thank you, it is the docs what I need

@jaychoww jaychoww changed the title add GreenletProfiler feat: add GreenletProfiler Oct 25, 2022
@jaychoww jaychoww marked this pull request as ready for review October 28, 2022 16:00

if __name__ == '__main__':
# noinspection PyTypeChecker
app.run(host='0.0.0.0', port=9090)
Copy link

Choose a reason for hiding this comment

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

hardcoded_bind_all_interfaces: Possible binding to all interfaces.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -0,0 +1,3 @@
set -ex
Copy link

Choose a reason for hiding this comment

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

SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@jaychoww
Copy link
Contributor Author

jaychoww commented Oct 28, 2022

there is something wrong when profiling start:

skywalking [Thread-127] [DEBUG] profile task [1666974131787_ZTJlLXNlcnZpY2UtcHJvdmlkZXI=.1] for endpoint [/artist] started
Traceback (most recent call last):
  File "src/gevent/_waiter.py", line 122, in gevent._gevent_c_waiter.Waiter.switch
  File "/skywalking-python/venv/lib/python3.6/site-packages/apache_skywalking-0.8.0-py3.6.egg/skywalking/profile/profile_context.py", line 329, in callback
    agent.add_profiling_snapshot(snapshot)
  File "/skywalking-python/venv/lib/python3.6/site-packages/apache_skywalking-0.8.0-py3.6.egg/skywalking/agent/__init__.py", line 288, in add_profiling_snapshot
    __snapshot_queue.put(snapshot)
  File "/usr/local/lib/python3.6/queue.py", line 126, in put
    with self.not_full:
  File "/usr/local/lib/python3.6/threading.py", line 240, in __enter__
    return self._lock.__enter__()
  File "src/gevent/_semaphore.py", line 278, in gevent._gevent_c_semaphore.Semaphore.__enter__
  File "src/gevent/_semaphore.py", line 279, in gevent._gevent_c_semaphore.Semaphore.__enter__
  File "src/gevent/_semaphore.py", line 180, in gevent._gevent_c_semaphore.Semaphore.acquire
  File "/skywalking-python/venv/lib/python3.6/site-packages/gevent/thread.py", line 121, in acquire
    acquired = BoundedSemaphore.acquire(self, blocking, timeout)
  File "src/gevent/_semaphore.py", line 180, in gevent._gevent_c_semaphore.Semaphore.acquire
  File "src/gevent/_semaphore.py", line 249, in gevent._gevent_c_semaphore.Semaphore.acquire
  File "src/gevent/_abstract_linkable.py", line 521, in gevent._gevent_c_abstract_linkable.AbstractLinkable._wait
  File "src/gevent/_abstract_linkable.py", line 487, in gevent._gevent_c_abstract_linkable.AbstractLinkable._wait_core
  File "src/gevent/_abstract_linkable.py", line 490, in gevent._gevent_c_abstract_linkable.AbstractLinkable._wait_core
  File "src/gevent/_abstract_linkable.py", line 442, in gevent._gevent_c_abstract_linkable.AbstractLinkable._AbstractLinkable__wait_to_be_notified
  File "src/gevent/_abstract_linkable.py", line 451, in gevent._gevent_c_abstract_linkable.AbstractLinkable._switch_to_hub
  File "src/gevent/_greenlet_primitives.py", line 61, in gevent._gevent_c_greenlet_primitives.SwitchOutGreenletWithLoop.switch
  File "src/gevent/_greenlet_primitives.py", line 64, in gevent._gevent_c_greenlet_primitives.SwitchOutGreenletWithLoop.switch
  File "src/gevent/_greenlet_primitives.py", line 67, in gevent._gevent_c_greenlet_primitives.SwitchOutGreenletWithLoop.switch_out
  File "src/gevent/_greenlet_primitives.py", line 68, in gevent._gevent_c_greenlet_primitives.SwitchOutGreenletWithLoop.switch_out
gevent.exceptions.BlockingSwitchOutError: Impossible to call blocking function in the event loop callback
2022-10-28T16:22:34Z <built-in method switch of gevent._gevent_cgreenlet.Greenlet object at 0x7ff213196a48> failed with BlockingSwitchOutError

this Exception will raise when gevent upgrade to version>1.4.0, and before the greenlet quit and switch to the hub.
so we can catch it for a signal to quit the profiling

@jaychoww jaychoww marked this pull request as draft October 28, 2022 16:33
@jaychoww jaychoww marked this pull request as ready for review October 29, 2022 10:14
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
@wu-sheng wu-sheng added the feature New feature label Nov 2, 2022
@Superskyyy
Copy link
Member

Generally looks good to me. @kezhenxu94 can you take a look? I'm not very familiar with the profiling feature.

@Superskyyy
Copy link
Member

Superskyyy commented Nov 4, 2022

The sonatype bug can be ignored, since it's testing code.

@Superskyyy Superskyyy self-requested a review November 4, 2022 17:12
Superskyyy
Superskyyy previously approved these changes Nov 4, 2022
@Superskyyy Superskyyy added this to the 1.0.0 milestone Nov 4, 2022
@Superskyyy
Copy link
Member

Superskyyy commented Nov 4, 2022

We will need documentation. The original documentation for threading-profiling was posted to here https://skywalking.apache.org/blog/2021-09-12-skywalking-python-profiling/, now with the addition of Greenlet Profiler, it will be better to move it back to our repository and add the corresponding part for greenlet. @westarest, can you work on the docs in the next PR?

kezhenxu94
kezhenxu94 previously approved these changes Nov 5, 2022
skywalking/profile/profile_context.py Outdated Show resolved Hide resolved
@jaychoww jaychoww dismissed stale reviews from kezhenxu94 and Superskyyy via 58ba602 November 5, 2022 03:22
@jaychoww
Copy link
Contributor Author

jaychoww commented Nov 5, 2022

We will need documentation. The original documentation for threading-profiling was posted to here https://skywalking.apache.org/blog/2021-09-12-skywalking-python-profiling/, now with the addition of Greenlet Profiler, it will be better to move it back to our repository and add the corresponding part for greenlet. @westarest, can you work on the docs in the next PR?

no problem. I can post a document about the difference between the Threading profiler and the Greenlet profiler that I committed.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Cool. @westarest Would you like to write a new blog for this feature, to introduce why it is important, and how you implement this, and use it?

I believe this is a very important and valuable blog.

@kezhenxu94 kezhenxu94 merged commit 11a74be into apache:master Nov 5, 2022
@jaychoww jaychoww deleted the feat_greenlet branch November 9, 2022 06:36
jaychoww added a commit to jaychoww/skywalking-python that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
4 participants