-
Notifications
You must be signed in to change notification settings - Fork 165
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(libsinsp/container_engine): proper containerd support #2195
base: master
Are you sure you want to change the base?
Changes from all commits
fa73862
7c8e8d2
9d0abd8
ee13d83
6ed5dca
455d3dd
0ba5c53
b90847d
431d095
13aead4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,14 +31,16 @@ std::string event_capture::s_engine_path; | |
unsigned long event_capture::s_buffer_dim = DEFAULT_DRIVER_BUFFER_BYTES_DIM * 4; | ||
|
||
event_capture::event_capture(captured_event_callback_t captured_event_callback, | ||
before_capture_t before_open, | ||
before_open_t before_open, | ||
before_capture_t before_capture, | ||
after_capture_t before_close, | ||
event_filter_t filter, | ||
uint32_t max_thread_table_size, | ||
uint64_t thread_timeout_ns, | ||
uint64_t inactive_thread_scan_time_ns) { | ||
m_captured_event_callback = std::move(captured_event_callback); | ||
m_before_capture = std::move(before_open); | ||
m_before_open = std::move(before_open); | ||
m_before_capture = std::move(before_capture); | ||
m_after_capture = std::move(before_close); | ||
m_filter = std::move(filter); | ||
|
||
|
@@ -68,6 +70,7 @@ void event_capture::start(bool dump, libsinsp::events::set<ppm_sc_code>& sc_set) | |
} | ||
} | ||
} | ||
m_before_open(m_inspector.get()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you need to introduce these new callback? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, if it needs to stay, at the very least i'd rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The container engines are initialised before the start of the capture and so we need an hook to set the cri settings before the start of the capture and after the open.
👍 |
||
open_engine(event_capture::get_engine(), sc_set); | ||
|
||
const ::testing::TestInfo* const test_info = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Care to expand on this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until now the
container.id
field was just the truncated version of thecontainer.full_id
which was a 64 char string. This is not always the case (e.g. containerd permits to define an arbitrary container id string) and we should accept container id of arbitrary length. Given that, the container id can now be less than 12 chars.