feat(logging): add communication log REST API endpoints#245
Open
feat(logging): add communication log REST API endpoints#245
Conversation
…, ring buffer, config)
- Fix include order in log_manager.hpp: system/external before project headers - Document OR-aggregation semantics in LogProvider::on_log_entry() doc comment - Add thread safety comment to PluginManager::get_log_observers() explaining the pre-existing race condition with disable_plugin() (same as get_introspection_providers) - Document dynamic_cast across dlopen boundary limitation (RTTI visibility hazard) with workaround guidance in load_plugins() - Use named constants (Log::DEBUG/INFO/WARN/ERROR/FATAL) instead of magic numbers in level_to_severity() and severity_to_level() - Use std::move(entry) in on_rosout() and inject_entry_for_testing() push_back calls
// @issue 208 Add LOGS to CapabilityBuilder::Capability enum and capability_to_name() switch. Wire Cap::LOGS into Component and App discovery responses so that entity detail endpoints advertise the /logs capability href.
// @issue 208 - LogHandlers: handle_get_logs, handle_get_logs_configuration, handle_put_logs_configuration. Components use prefix matching (all nodes under namespace); Apps use exact FQN matching. - Wire LogManager into GatewayNode: get_log_manager() accessor, log_mgr_ member initialized after plugin_mgr_ setup so LogProvider plugins are registered before /rosout subscription starts. - Register log_handlers.hpp in umbrella handlers.hpp and rest_server.hpp. - Add test_log_handlers: 9 unit tests verifying 400 on missing matches.
…erver // @issue 208 Register GET /components/{id}/logs, GET /apps/{id}/logs, GET /{entity}/logs/configuration, and PUT /{entity}/logs/configuration routes in RESTServer::setup_routes(). Instantiate LogHandlers alongside other handler objects in RESTServer constructor.
// @issue 208 Parallel to get_raw(): sends PUT with JSON body and returns the raw Response object without JSON parsing. Needed by the logging endpoint integration tests to inspect responses from PUT /logs/configuration.
… endpoints // @issue 208 Feature test covering: - GET /apps/{id}/logs: returns 200 with items array, polls for log entries from startup, validates entry schema (id, timestamp, severity, message, context) - GET /apps/{id}/logs?severity=error: severity filter excludes lower levels - GET/PUT /apps/{id}/logs/configuration: config round-trip, invalid severity returns 400, max_entries=0 returns 400 - GET /components/{id}/logs and /logs/configuration: 200 responses - Capability advertisement: 'logs' present in capabilities list for both components and apps
… traceability tags
Contributor
There was a problem hiding this comment.
Pull request overview
Adds communication log querying/configuration endpoints to the gateway REST API, backed by a /rosout-fed ring buffer with optional plugin delegation, and advertises the new logs capability on Apps/Components.
Changes:
- Introduces
LogManager(/rosout subscription, per-node buffers, config, query filtering) andLogProviderplugin interface. - Adds REST handlers and routes for
GET /{apps|components}/{id}/logsandGET/PUT /{apps|components}/{id}/logs/configuration. - Extends discovery responses to advertise
Capability::LOGSand adds unit/integration test coverage + docs.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_integration_tests/test/features/test_logging_api.test.py | New integration tests for logging endpoints and capability advertisement |
| src/ros2_medkit_integration_tests/ros2_medkit_test_utils/gateway_test_case.py | Adds put_raw() helper for integration tests needing raw responses |
| src/ros2_medkit_gateway/test/test_log_manager.cpp | New unit tests for severity mapping, normalization, ring buffer behavior, and config updates |
| src/ros2_medkit_gateway/test/test_log_handlers.cpp | New unit tests for handler guard paths (missing route matches) |
| src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp | Adds LogProvider discovery/caching and observer enumeration |
| src/ros2_medkit_gateway/src/log_manager.cpp | Implements /rosout subscription, ring-buffer storage, log querying, and config management |
| src/ros2_medkit_gateway/src/http/rest_server.cpp | Registers log routes and instantiates LogHandlers |
| src/ros2_medkit_gateway/src/http/handlers/log_handlers.cpp | Implements REST handlers for logs and logs configuration |
| src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp | Adds LOGS capability to component/app detail responses |
| src/ros2_medkit_gateway/src/http/handlers/capability_builder.cpp | Maps Capability::LOGS to "logs" |
| src/ros2_medkit_gateway/src/gateway_node.cpp | Instantiates LogManager and exposes accessor |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/log_provider.hpp | Defines plugin interface for log backends and observers |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp | Adds LogProvider plumbing to plugin manager API/state |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/log_types.hpp | Defines LogEntry and LogConfig types |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/log_manager.hpp | Declares LogManager API and static helpers |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp | Adds log_handlers_ member |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/log_handlers.hpp | Declares log REST handlers |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp | Includes log_handlers.hpp |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/capability_builder.hpp | Adds Capability::LOGS enum value |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp | Adds LogManager include/member/accessor |
| src/ros2_medkit_gateway/CMakeLists.txt | Builds new sources and adds new gtests |
| docs/api/rest.rst | Documents the new communication logs endpoints |
…ceability tags Fix RST definition list in docs/api/rest.rst (two consecutive terms sharing a definition are not valid RST - combined each pair onto a single term line).
- Use extern "C" get_log_provider() query function in PluginLoader instead of dynamic_cast across the dlopen boundary (mirrors UpdateProvider / IntrospectionProvider discovery mechanism) - Validate severity query parameter in GET /logs; return 400 for unknown values - Validate max_entries type and range explicitly in PUT /logs/configuration; return 400 when present but not a positive integer - Wrap LogProvider::on_log_entry() calls in try/catch to prevent plugin exceptions from crashing the ROS 2 subscription callback
…rocesses via ROS_DOMAIN_ID The FaultManagerNode unit tests (test_fault_manager, test_sqlite_storage, test_snapshot_capture, test_rosbag_capture) create in-process ROS 2 nodes on the default DDS domain. The launch_testing integration tests (test_integration, test_rosbag_integration) run concurrently and launch their own fault_manager_node subprocess on the same domain, causing service calls to occasionally route to the wrong server and making tests non-deterministically fail. Fix: assign a dedicated ROS_DOMAIN_ID to each GTest binary that uses ROS 2 nodes (62-65), keeping them isolated from integration test subprocesses that use the default domain (0).
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/log_handlers.hpp
Outdated
Show resolved
Hide resolved
…efaults - Add logs.buffer_size parameter (default: 200) to gateway_params.yaml and declare_parameter in GatewayNode; LogManager reads it at startup - Reduce kDefaultBufferSize from 10 000 to 200 (ring buffer per node) - Reduce LogConfig::max_entries default from 10 000 to 100 (GET /logs cap) - Add Logging Endpoints section to README (quick list + detailed reference) - Update test asserting the old max_entries default
- Remove REQ_INTEROP_062 (GET /logs/entries) and REQ_INTEROP_065 (DELETE /logs/config) - neither exists in ISO 17978-3 §7.21 - Fix REQ_INTEROP_061 description: endpoint returns log entries, not an overview - Fix REQ_INTEROP_063/064 URLs: /logs/config -> /logs/configuration - Mark REQ_INTEROP_061/063/064 as verified (tests exist) - Retag all @verifies REQ_INTEROP_062 -> REQ_INTEROP_061 in tests - Update rest.rst: 10 000 -> 200 (buffer), 100 (max_entries default) - Note 204 vs 200 deviation from SOVD spec for PUT /logs/configuration
- Fix GatewayPluginLoadResult move ctor/assignment to transfer log_provider (previously the field was leaked, leaving dangling pointer after move) - Fix disable_plugin() to null load_result.log_provider alongside other fields - Add 256-char length cap on context query parameter to prevent unbounded input - Add server-side 10000 max_entries cap in PUT /logs/configuration handler - Clamp logs.buffer_size parameter [1, 100000] in gateway_node with WARN on OOB - Add 4 missing integration tests: PUT /components/.../logs/configuration (204), context filter with nonexistent node (empty items), invalid JSON body (400), and entity severity_filter acting as server-side floor
Three integration test failures (jazzy, humble, rolling): 1. Log entries never appearing after startup: ROS 2 rosout messages carry logger names using '.' as separator (e.g. "powertrain.engine.temp_sensor") while entity FQNs use '/'. get_logs() now tries both slash-format and dot-format when matching buffer keys, so the default ring buffer resolves rosout entries correctly. Two unit tests added to cover this. 2. test_component_detail_includes_logs_capability failing: Component detail handler placed capabilities only under x-medkit, not at root level. Added response["capabilities"] at root level to match the app detail format (and the test expectation). Remaining review comments addressed: - All three 503 responses in log_handlers.cpp now use ERR_SERVICE_UNAVAILABLE (was ERR_INTERNAL_ERROR) - log_handlers.hpp comment: "store/return" -> "return in query results" - docs/api/rest.rst: "retain/return" -> "return in query results"
The two polling tests passed a condition that returned a plain bool
(True/False) instead of the response dict itself. poll_endpoint_until
returns whatever the condition callable returns, so data ended up as
True and data['items'] raised TypeError: 'bool' object is not
subscriptable.
Change condition to lambda d: d if d.get('items') else None so the
full response dict is returned and data['items'] works correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Adds communication log REST API endpoints for Apps and Components, backed by a per-node
/rosoutring buffer with configurable severity filter and buffer capacity.GET /apps/{id}/logs,GET /components/{id}/logs- query buffered log entries (severity and context filters supported)GET/PUT /apps/{id}/logs/configuration,GET/PUT /components/{id}/logs/configuration- read and update per-entity log configLogProviderplugin interface for third-party log backendsCapability::LOGSadvertised in App and Component discovery responsesIssue
Type
Testing
test_log_manager- 19 unit tests: severity mapping, FQN normalisation, ring-buffer eviction, config updates, prefix vs exact matching, plugin delegationtest_log_handlers- 9 unit tests: missing-matches guard paths for all three handler methodstest_logging_apiintegration test - 15 tests covering GET /logs (schema, severity filter, 404), GET/PUT /logs/configuration (defaults, updates, 400 validation), and capability advertisementAll
ros2_medkit_gatewayunit tests pass. One pre-existing flaky fault_manager test (FaultEventPublishingTest.NewFaultPublishesConfirmedEvent) is unrelated to these changes.Checklist