-
Notifications
You must be signed in to change notification settings - Fork 440
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
Add plugin support #36
Conversation
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.
Nice work!
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.
Nice.
examples/plugin/plugin/tracer.h
Outdated
opentelemetry::nostd::string_view name, | ||
const opentelemetry::trace::StartSpanOptions &options) noexcept override; | ||
|
||
private: |
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.
Drop?
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.
drop what?
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 empty private:
section.
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.
Yeah, that makes sense. Removed.
examples/plugin/load/main.cc
Outdated
{ | ||
if (argc != 3) | ||
{ | ||
std::cerr << "Usage: load_plugin <plugin> <confg>\n"; |
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.
config
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.
Fixed. Thanks.
ci/do_ci.sh
Outdated
cmake -DCMAKE_BUILD_TYPE=Debug \ | ||
-DCMAKE_CXX_FLAGS="-Werror" \ | ||
"${SRC_DIR}" | ||
make | ||
make test | ||
exit 0 | ||
elif [[ "$1" == "cmake.build_example_plugin" ]]; then |
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.
Please update ci/README.md
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.
Done
{ | ||
namespace detail | ||
{ | ||
inline void CopyErrorMessage(const char *source, std::string &destination) noexcept |
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.
Since this isn't a virtual, can it just use std::string
?
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.
It's using a std::string for the destination.
It takes a const char*
instead of string_view or string for the source to handle the case when it might be a nullptr.
Adds an interface for loading tracer as plugins.
The plugin-tracer interface only use types that have well defined ABIs. Provided the standard-c++ library is suitably linked, plugins can be portable and not tied to a particular standard-c++ library implementation.
Because we're not far enough along with a tracer implementation to test against, this also adds a small dynamic load example for testing. But it can be replaced when there's an SDK implementation.