Skip to content

RSDK-5986: Resource-level logging for the C++ SDK #383

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

Open
wants to merge 97 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 66 commits
Commits
Show all changes
97 commits
Select commit Hold shift + click to select a range
31437c3
initial commit of instance object
lia-viam Feb 18, 2025
932ebaa
wip: initial commit to get sdk building with instance
lia-viam Feb 18, 2025
04d0158
add missing implementation
lia-viam Feb 18, 2025
a105cef
update examples to use instance
lia-viam Feb 18, 2025
403f316
only instance can construct registry
lia-viam Feb 18, 2025
27eeb13
make tests work with registry member approach
lia-viam Feb 21, 2025
ce2aa8c
pimpl registry and update examples
lia-viam Feb 24, 2025
e5cf6d0
add instance management infra
lia-viam Feb 25, 2025
85194da
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into regi…
lia-viam Feb 25, 2025
5024d3a
update tflite module
lia-viam Feb 25, 2025
ef8ec8e
silence spurious const warning
lia-viam Feb 25, 2025
16113fd
make method static
lia-viam Feb 25, 2025
216e5e2
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into regi…
lia-viam Feb 26, 2025
dbf1647
update cml boost links
lia-viam Feb 26, 2025
d1dffb8
add logger to resource
lia-viam Feb 27, 2025
4771390
use static current to populate registry member
lia-viam Feb 27, 2025
18d26c9
revert and adapt examples
lia-viam Feb 27, 2025
6018323
make instance more of a black box
lia-viam Feb 27, 2025
d4aeb86
Merge branch 'registry-member' into logger
lia-viam Feb 27, 2025
5535616
add impl instance to git
lia-viam Feb 27, 2025
4d2582c
Merge branch 'registry-member' into logger
lia-viam Feb 27, 2025
eeac195
Add instance comment boilerplate
lia-viam Mar 3, 2025
7560f6d
Add instance comment boilerplate
lia-viam Mar 3, 2025
af08403
connect robot client logging when parent of module service
lia-viam Mar 3, 2025
4e25d35
remove old ctor arg
lia-viam Mar 3, 2025
55baa6c
prohibit multi instance
lia-viam Mar 3, 2025
10cd711
remove initialized flag
lia-viam Mar 3, 2025
c745422
meyers singleton registry
lia-viam Mar 3, 2025
75ab450
remove registry member and fix member spacing/ordering
lia-viam Mar 3, 2025
fc21c35
remove unused include
lia-viam Mar 3, 2025
1758ee9
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into regi…
lia-viam Mar 3, 2025
cd1d8c5
Merge branch 'registry-member' of github.com:lia-viam/viam-cpp-sdk in…
lia-viam Mar 3, 2025
679f1b6
conditionally connect robot logging to backend
lia-viam Mar 3, 2025
795578c
first attempt full logger machinery
lia-viam Mar 4, 2025
1b88d1c
update log to have keywords in header
lia-viam Mar 7, 2025
5eb243b
init grpc logging in module service impl ready, and disable console l…
lia-viam Mar 7, 2025
6e311bc
remove set severity from args
lia-viam Mar 7, 2025
f84fe6e
update boost_log_trivial invocations
lia-viam Mar 7, 2025
9311990
remove now-unused boost includes and uses of boost log trivial
lia-viam Mar 7, 2025
a09901f
remove unused log trivial include
lia-viam Mar 7, 2025
f9b1cb3
readability member arrangement/spacing
lia-viam Mar 7, 2025
68add9d
readability spacing
lia-viam Mar 7, 2025
05eddd0
add from string
lia-viam Mar 7, 2025
5debae1
add log level
lia-viam Mar 7, 2025
6866567
add basic logging tests
lia-viam Mar 10, 2025
6e154ce
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into regi…
lia-viam Mar 12, 2025
f02ce0f
reorder includes
lia-viam Mar 12, 2025
e80bf80
conditionally find unit test framework and link it to tests
lia-viam Mar 12, 2025
185121a
registry get cannot create instance by default
lia-viam Mar 12, 2025
7a747ec
instance models fixture
lia-viam Mar 12, 2025
9c4dc97
create if needed instance in fixture
lia-viam Mar 12, 2025
30fe47e
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into regi…
lia-viam Mar 12, 2025
7409dc7
revert namespace ws
lia-viam Mar 12, 2025
7fafee8
Merge branch 'registry-member' into logger
lia-viam Mar 13, 2025
0dbd491
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into logger
lia-viam Mar 13, 2025
c67166b
move cout redirect to test_utils
lia-viam Mar 13, 2025
0745e16
implement set resource log level
lia-viam Mar 13, 2025
04edd0d
implement set resource log level for resource itself
lia-viam Mar 13, 2025
537ea67
test resource level logging in unit tests
lia-viam Mar 13, 2025
56e706c
set resource log level from config in start/reconfigure
lia-viam Mar 13, 2025
818825a
conditionally find program options as well
lia-viam Mar 14, 2025
6d43bbd
remove trailing semicolon
lia-viam Mar 14, 2025
de5e33a
private/friend decls
lia-viam Mar 14, 2025
a286508
docs
lia-viam Mar 14, 2025
587d1f6
Merge branch 'main' into logger
lia-viam Mar 14, 2025
e45bafc
update method name to avoid conflict
lia-viam Mar 17, 2025
14a6049
const qualify method and member ptr
lia-viam Mar 17, 2025
216df1d
silence tidy error
lia-viam Mar 17, 2025
f4b0f11
fix file logging
lia-viam Mar 17, 2025
e3e2697
handle empty string and clarify level formatting
lia-viam Mar 17, 2025
bdebbab
log the logging
lia-viam Mar 17, 2025
6755cd6
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into logger
lia-viam Mar 17, 2025
8c372cc
linter warnings...........
lia-viam Mar 17, 2025
1744939
linter.....
lia-viam Mar 17, 2025
d7842c1
add log_setup
lia-viam Mar 17, 2025
9829e44
remove sdk::
lia-viam Mar 17, 2025
e10df01
global resource is const char*
lia-viam Mar 17, 2025
192b29f
log log as debug
lia-viam Mar 17, 2025
2de9f40
fix out of date comment
lia-viam Mar 17, 2025
d681f6f
rename to log manager and global logger
lia-viam Mar 17, 2025
3b8d7dc
rename files to logging
lia-viam Mar 17, 2025
82eb403
use has_log_configuration
lia-viam Mar 18, 2025
ad079c8
unit test trim filename
lia-viam Mar 18, 2025
6b83aed
handle fallback console logging for grpc log failures
lia-viam Mar 18, 2025
f9f39a1
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into logger
lia-viam Mar 18, 2025
4eb8643
add logging to simple module example
lia-viam Mar 19, 2025
418f882
add logging to complex module example
lia-viam Mar 19, 2025
ee85b28
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into logger
lia-viam Mar 19, 2025
40b5400
use VIAM_SDK macro prefix
lia-viam Mar 20, 2025
22d29e3
disable fail-fast
lia-viam Mar 20, 2025
ba154d1
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into logger
lia-viam Apr 3, 2025
2e9d015
link boost log and log_setup in public libs
lia-viam Apr 3, 2025
b8dcf7c
set boost log dyn link in test script
lia-viam Apr 3, 2025
f6c21ff
clean up example camera and use logging instead of cout
lia-viam Apr 4, 2025
ac25b4e
update example code to use logging
lia-viam Apr 4, 2025
04c92ce
add test and use set_global_resource_name
lia-viam Apr 4, 2025
9297062
silence const warnings
lia-viam Apr 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,18 @@ if (VIAMCPPSDK_BUILD_TESTS)
set(VIAMCPPSDK_BOOST_TEST "unit_test_framework")
endif()

find_package(Boost ${VIAMCPPSDK_BOOST_VERSION_MINIMUM} REQUIRED COMPONENTS headers log program_options ${VIAMCPPSDK_BOOST_TEST})
if (VIAMCPPSDK_BUILD_EXAMPLES)
set(VIAMCPPSDK_BOOST_PROGRAM_OPTIONS "program_options")
endif()

find_package(Boost ${VIAMCPPSDK_BOOST_VERSION_MINIMUM} REQUIRED
COMPONENTS
headers
log
log_setup
${VIAMCPPSDK_BOOST_PROGRAM_OPTIONS}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was a drive-by

${VIAMCPPSDK_BOOST_TEST}
)

# Time to find `protobuf` and `gRPC[++]`. Normally this would just be
# something like `find_package(gRPC <version> CONFIG REQUIRED)`, and
Expand Down
1 change: 0 additions & 1 deletion src/viam/examples/modules/complex/main.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <memory>
#include <signal.h>

#include <boost/log/trivial.hpp>
#include <grpcpp/grpcpp.h>
#include <grpcpp/server_context.h>

Expand Down
4 changes: 4 additions & 0 deletions src/viam/sdk/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ target_sources(viamsdk
components/sensor.cpp
components/servo.cpp
config/resource.cpp
log/logger.cpp
log/private/log_backend.cpp
module/handler_map.cpp
module/module.cpp
module/service.cpp
Expand Down Expand Up @@ -170,6 +172,7 @@ target_sources(viamsdk
../../viam/sdk/components/sensor.hpp
../../viam/sdk/components/servo.hpp
../../viam/sdk/config/resource.hpp
../../viam/sdk/log/logger.hpp
../../viam/sdk/module/handler_map.hpp
../../viam/sdk/module/module.hpp
../../viam/sdk/module/service.hpp
Expand Down Expand Up @@ -244,6 +247,7 @@ viamcppsdk_link_viam_api(viamsdk PRIVATE)
target_link_libraries(viamsdk
PUBLIC Boost::headers
PUBLIC Boost::log
PUBLIC Boost::log_setup
PUBLIC xtensor
PRIVATE ${VIAMCPPSDK_GRPCXX_REFLECTION_LIBRARIES}
PRIVATE ${VIAMCPPSDK_GRPCXX_LIBRARIES}
Expand Down
7 changes: 3 additions & 4 deletions src/viam/sdk/common/client_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,17 @@
#include <grpcpp/client_context.h>
#include <grpcpp/support/status.h>

#include <boost/log/trivial.hpp>

#include <viam/sdk/common/private/version_metadata.hpp>
#include <viam/sdk/log/logger.hpp>

namespace viam {
namespace sdk {

namespace client_helper_details {

[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status* status) noexcept {
BOOST_LOG_TRIVIAL(fatal) << "ClientHelper error handler callback returned instead of throwing: "
<< status->error_message() << '(' << status->error_details() << ')';
VIAM_LOG(fatal) << "ClientHelper error handler callback returned instead of throwing: "
<< status->error_message() << '(' << status->error_details() << ')';
std::abort();
}

Expand Down
1 change: 1 addition & 0 deletions src/viam/sdk/common/instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Instance::Instance() {

impl_ = std::make_unique<Instance::Impl>();
impl_->registry.initialize();
impl_->logger.init_logging();
}

Instance::~Instance() {
Expand Down
1 change: 1 addition & 0 deletions src/viam/sdk/common/instance.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Instance {

private:
friend class Registry;
friend class Logger;

struct Impl;
std::unique_ptr<Impl> impl_;
Expand Down
2 changes: 2 additions & 0 deletions src/viam/sdk/common/private/instance.hpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#pragma once

#include <viam/sdk/common/instance.hpp>
#include <viam/sdk/log/logger.hpp>
#include <viam/sdk/registry/registry.hpp>

namespace viam {
namespace sdk {

struct Instance::Impl {
Registry registry;
Logger logger;
};

} // namespace sdk
Expand Down
12 changes: 0 additions & 12 deletions src/viam/sdk/common/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

#include <boost/algorithm/string.hpp>
#include <boost/blank.hpp>
#include <boost/log/core.hpp>
#include <boost/log/expressions.hpp>
#include <boost/log/trivial.hpp>
#include <boost/optional/optional.hpp>

#include <viam/api/common/v1/common.pb.h>
Expand Down Expand Up @@ -96,15 +93,6 @@ std::string bytes_to_string(const std::vector<unsigned char>& b) {
return img_string;
}

void set_logger_severity_from_args(int argc, char** argv) {
if (argc >= 3 && strcmp(argv[2], "--log-level=debug") == 0) {
boost::log::core::get()->set_filter(boost::log::trivial::severity >=
boost::log::trivial::debug);
return;
}
boost::log::core::get()->set_filter(boost::log::trivial::severity >= boost::log::trivial::info);
}

std::string random_debug_key() {
static const char alphanum[] = "abcdefghijklmnopqrstuvwxyz";
static std::default_random_engine generator(
Expand Down
10 changes: 0 additions & 10 deletions src/viam/sdk/common/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,6 @@ ProtoStruct with_debug_entry(ProtoStruct&& map, std::string debug_key);
/// @returns the new ProtoStruct
ProtoStruct with_debug_entry(ProtoStruct&& map);

/// @brief Set the boost trivial logger's severity depending on args.
/// @param argc The number of args.
/// @param argv The commandline arguments to parse.
///
/// Sets the boost trivial logger's severity to debug if "--log-level=debug" is the
/// the third argument. Sets severity to info otherwise. Useful for module
/// implementations. See LogLevel documentation in the RDK for more information on
/// how to start modules with a "log-level" commandline argument.
void set_logger_severity_from_args(int argc, char** argv);

/// @brief Used in modular filter components to get the 'fromDataManagement' value from an extra
/// ProtoStruct.
/// @param extra The extra ProtoStruct.
Expand Down
14 changes: 11 additions & 3 deletions src/viam/sdk/config/resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@ ResourceConfig::ResourceConfig(std::string type,
ProtoStruct attributes,
std::string api,
Model model,
LinkConfig frame)
LinkConfig frame,
sdk::log_level lvl)
Copy link
Member

Choose a reason for hiding this comment

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

(q) do we need the sdk:: prefix here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think not anymore, initially the getter was also called log_level so it was needed to distinguish them

: api_({kRDK, type, ""}),
frame_(std::move(frame)),
model_(std::move(model)),
name_(std::move(name)),
namespace__(std::move(namespace_)),
type_(std::move(type)),
attributes_(std::move(attributes)) {
attributes_(std::move(attributes)),
log_level_(lvl) {
if (api.find(':') != std::string::npos) {
api_ = API::from_string(std::move(api));
}
Expand Down Expand Up @@ -81,6 +83,10 @@ const std::string& ResourceConfig::type() const {
return type_;
}

log_level ResourceConfig::get_log_level() const {
return log_level_;
}

const std::vector<std::string>& viam::sdk::ResourceConfig::depends_on() const {
return depends_on_;
}
Expand Down Expand Up @@ -137,6 +143,7 @@ void to_proto_impl<ResourceConfig>::operator()(const ResourceConfig& self,
*proto->mutable_namespace_() = self.namespace_();
*proto->mutable_type() = self.type();
*proto->mutable_api() = self.api().to_string();
*proto->mutable_log_configuration()->mutable_level() = to_string(self.get_log_level());
*proto->mutable_model() = self.model().to_string();
*proto->mutable_attributes() = to_proto(self.attributes());

Expand All @@ -154,7 +161,8 @@ ResourceConfig from_proto_impl<app::v1::ComponentConfig>::operator()(
from_proto(proto->attributes()),
proto->api(),
Model::from_str(proto->model()),
proto->has_frame() ? from_proto(proto->frame()) : LinkConfig{});
proto->has_frame() ? from_proto(proto->frame()) : LinkConfig{},
level_from_string(proto->log_configuration().level()));
Copy link
Member

Choose a reason for hiding this comment

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

(minor) maybe a little paranoid, but proto exposes a has_log_configuration() method which makes me worry that there might be cases where this isn't set, or where that could become the case going forward. Might be better to use the ternary like in the frame setting, and default to info

edit: I see you updated here already, disregard!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually ended up fixing this due to your suggestion to paste some sample logs because I saw an error that kept firing! But it's helpful to know about has_log_configuration, I will use that

}

std::vector<ResourceConfig>
Expand Down
25 changes: 23 additions & 2 deletions src/viam/sdk/config/resource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <viam/sdk/common/proto_convert.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/log/logger.hpp>
#include <viam/sdk/referenceframe/frame.hpp>
#include <viam/sdk/resource/resource_api.hpp>

Expand Down Expand Up @@ -45,34 +46,54 @@ class ResourceConfig {
ProtoStruct attributes,
std::string api,
Model model,
LinkConfig frame);
LinkConfig frame,
sdk::log_level lvl = sdk::log_level::info);

ResourceConfig(std::string type);

Name resource_name();

const API& api() const;

const LinkConfig& frame() const;

const Model& model() const;

const std::string& name() const;
const std::string& namespace_() const;
const std::string& type() const;

log_level get_log_level() const;

const std::vector<std::string>& depends_on() const;

const std::vector<ResourceLevelServiceConfig>& service_config() const;

const ProtoStruct& attributes() const;

private:
void fix_api();

API api_;

LinkConfig frame_;

Model model_;

std::string name_;
std::string namespace__;
std::string type_;

std::vector<std::string> depends_on_;

std::vector<ResourceLevelServiceConfig> service_config_;

ProtoStruct attributes_;
ProtoValue converted_attributes_;

std::vector<std::string> implicit_depends_on_;
void fix_api();

sdk::log_level log_level_;
};

namespace proto_convert_details {
Expand Down
Loading
Loading