-
-
Notifications
You must be signed in to change notification settings - Fork 84
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: log runtime output to env var "BPFTIME_LOG_OUTPUT" #301
Conversation
af3495e
to
4fc55b3
Compare
Also:
|
1488e7b
to
918ceff
Compare
53b112e
to
5892756
Compare
@@ -4,6 +4,11 @@ | |||
#include <cstdlib> | |||
#include <string> | |||
|
|||
#ifndef DEFAULT_LOGGER_OUTPUT_PATH | |||
#define DEFAULT_LOGGER_OUTPUT_PATH "~/.bpftime/runtime.log" |
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.
Avoid using #define to define constants in C++
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.
Ok, I can fix that in next pr
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.
The problem is that it might be input by the cmake... is it better to
#ifndef DEFAULT_LOGGER_OUTPUT_PATH
const char* default_log_path = "~/.bpftime/runtime.log"
#else
const char* default_log_path = DEFAULT_LOGGER_OUTPUT_PATH
#endif
I think the old one is a straightforward and clean way to ensure that a default value is set if none is provided.
if (input_path[0] == '~') { | ||
const char *homeDir = getenv("HOME"); | ||
if (!homeDir) { | ||
return "console"; |
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.
Will string "console" be parsed as a path?
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.
Yes, to stderr. The console log output will also be used in CI
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.
Yes, to stderr. The console log output will also be used in CI
I mean, if user want to print things to a file named console
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.
That might still be print to the console...Any suggestion for handling this case?
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.
For example, use std::optional as the return type of this function. If $HOME is not set, returning empty optional
Description
A draft version of logging. It implements:
Used file but not named pipe. bpftime server should be similar to docker daemon, managing "services" of injected ebpf programs. The log should be persistent, reusable.
Now usage:
I may add these feature in this PR, hoping suggestions or confirmation from you:
resolves # 279
Type of change
How Has This Been Tested?
Test Configuration:
Checklist