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

fix: remove only protected c files and their directories #1441

Conversation

tamirdavid1
Copy link
Collaborator

During an issue with eBPF instrumentation breaking upon odiglet restarts, I implemented a "smart" recreation of instrumentation files. The goal was to retain compiled C files when odiglet restarted. However, this approach led to other files not being properly recreated in certain cases. For example, if the instrumentation version was upgraded, the host filesystem could end up with both versions, which caused problems, especially in Python.

This PR addresses the issue by ensuring that everything is deleted and recreated except for the C files and their directories. For instance, in /var/odigos/python-ebpf/, everything will be recreated except for pythonUSDT.abi3.so.

shouldRecreateCFiles := ShouldRecreateAllCFiles()
log.Logger.V(0).Info(fmt.Sprintf("Removing files in directory: %s, shouldRecreateCFiles: %s", hostDir, fmt.Sprintf("%t", shouldRecreateCFiles)))
log.Logger.V(0).Info(fmt.Sprintf("Removing files in directory: %s, shouldRecreateCFiles: %t", hostDir, shouldRecreateCFiles))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: probably better to use structured logging (arguments as key-value pairs instead of one formatted string)

@tamirdavid1 tamirdavid1 force-pushed the not-removing-only-c-files-and-their-dirs branch from 6af7dc3 to 19b2883 Compare August 14, 2024 05:59
@tamirdavid1 tamirdavid1 merged commit 655977d into odigos-io:main Aug 14, 2024
17 checks passed
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.

2 participants