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

Use proc_events on Java processes #208

Merged
merged 27 commits into from Nov 26, 2021

Conversation

YishaiZinkin
Copy link
Contributor

@YishaiZinkin YishaiZinkin commented Nov 14, 2021

Description

Use proc_events from granulate-utils to track async-profiled processes and log when they exit with non-zero code.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots

Checklist:

  • I have read the CONTRIBUTING document.
  • I have updated the relevant documentation.
  • I have added tests for new logic.

gprofiler/proc_events.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Jongy Jongy left a comment

Choose a reason for hiding this comment

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

Looks good. I'll do a full CR once we restructure it ,let's make it an object instead of a set of functions, I think that's more managable.

@Jongy
Copy link
Contributor

Jongy commented Nov 19, 2021

@YishaiZinkin - let's move this code to https://github.com/Granulate/utils so it can be used by other components. I'll explain on Sunday.

d3dave
d3dave previously requested changes Nov 22, 2021
gprofiler/proc_events.py Outdated Show resolved Hide resolved
@YishaiZinkin
Copy link
Contributor Author

I suggest we finish the PR here just to concentrate the comments in one place. I'll move the module to utils once we're finished.

gprofiler/proc_events.py Outdated Show resolved Hide resolved
gprofiler/proc_events.py Outdated Show resolved Hide resolved
gprofiler/proc_events.py Outdated Show resolved Hide resolved
gprofiler/proc_events.py Outdated Show resolved Hide resolved
gprofiler/proc_events.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Jongy Jongy left a comment

Choose a reason for hiding this comment

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

Very clean API 👍

Some small comments.

gprofiler/proc_events.py Outdated Show resolved Hide resolved
@Jongy Jongy mentioned this pull request Nov 23, 2021
@YishaiZinkin YishaiZinkin changed the title RFC: Add proc_events Use proc_events Nov 25, 2021
@YishaiZinkin YishaiZinkin changed the title Use proc_events Use proc_events on Java processes Nov 25, 2021
Jongy
Jongy previously approved these changes Nov 25, 2021
Jongy
Jongy previously approved these changes Nov 25, 2021
@Jongy
Copy link
Contributor

Jongy commented Nov 25, 2021

@YishaiZinkin can you merge from master? The other PR got merged

# Conflicts:
#	gprofiler/profilers/java.py
#	requirements.txt
@@ -114,6 +115,23 @@ def get_ap_log(self) -> str:
return self._ap_log


class AsyncProfiledProcessMonitor:
def __init__(self):
self._attached_processes: List[int] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

You're now tracking processes both here and in JavaProfiler._profiled_processes :(

@Jongy
Copy link
Contributor

Jongy commented Nov 26, 2021

Getting this error:

Exception in thread Process Events Listener:
Traceback (most recent call last):
  File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 165, in _proc_events_listener
    self._register_for_connector_events(self._socket)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 110, in _register_for_connector_events
    socket.send(nl_msg)
ConnectionRefusedError: [Errno 111] Connection refused

@Jongy
Copy link
Contributor

Jongy commented Nov 26, 2021

Getting this error:

Exception in thread Process Events Listener:
Traceback (most recent call last):
  File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 165, in _proc_events_listener
    self._register_for_connector_events(self._socket)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 110, in _register_for_connector_events
    socket.send(nl_msg)
ConnectionRefusedError: [Errno 111] Connection refused

We're failing tests upon ERRORs in the log, but since this happens in another thread and it's not logged, it doesn't fail the tests :/

@Jongy
Copy link
Contributor

Jongy commented Nov 26, 2021

I pushed an update that merges this logic with #219 , as suggested in #219 (comment).

@Jongy
Copy link
Contributor

Jongy commented Nov 26, 2021

Getting this error:

Exception in thread Process Events Listener:
Traceback (most recent call last):
  File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 165, in _proc_events_listener
    self._register_for_connector_events(self._socket)
  File "/usr/local/lib/python3.8/dist-packages/granulate_utils/linux/proc_events.py", line 110, in _register_for_connector_events
    socket.send(nl_msg)
ConnectionRefusedError: [Errno 111] Connection refused

It seems like this happens since we're running in a network namespace. forkstat fails on a privileged container but passes on if --net=host.

@Jongy Jongy force-pushed the feature/track-java-processes-termination branch from 6447740 to 99a1b59 Compare November 26, 2021 00:37
@Jongy Jongy merged commit 89fcf8e into master Nov 26, 2021
@Jongy Jongy deleted the feature/track-java-processes-termination branch November 26, 2021 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants