-
Notifications
You must be signed in to change notification settings - Fork 10
Minor refactoring - libddprof helper #244
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
base: main
Are you sure you want to change the base?
Conversation
Introduce a helper to cleanup the libddprof errors
|
|
||
| inline void log_warn_and_drop_error(std::string_view message, | ||
| ddog_Error *error) { | ||
| defer { ddog_Error_drop(error); }; |
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.
Why use defer here ?
|
|
||
| LG_ERR("[EXPORTER] Failure generate tag (%.*s)", | ||
| (int)push_tag_res.err.message.len, push_tag_res.err.message.ptr); | ||
| log_warn_and_drop_error("[EXPORTER] Failure generate tag", |
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.
Won't the error message be logged twice ?
What is the benefit of using log_warn_and_drop_error ?
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.
So I was not using ddog_Error_message. The aim is to have a function you can always call in case of error and not think too hard about what to do.
Ideally it would handle the error returns with it, though that behavior is different across the files.
| exporter->_exporter = res_exporter.ok; | ||
| } else { | ||
| defer { ddog_Error_drop(&res_exporter.err); }; | ||
| log_warn_and_drop_error("[EXPORTER] Failure form ddog_prof_Exporter_new", |
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.
double log to be fixed
|
Possible improvements:
|
What does this PR do?
Introduce a helper to cleanup the libddprof errors
Motivation
It is easy to make mistakes when taking into account libddprof errors.