Uh oh!
There was an error while loading. Please reload this page.
[TOW-1424] Add runner-id to telemetry - #192
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a mandatory runner_id field to the telemetry context to help identify and diagnose issues by tracking which runner instance is executing a run.
Changes:
- Modified the telemetry
Contextstruct to require arunner_idparameter - Updated the logging macro to include
runner_idin all telemetry events - Refactored the
ContextAPI with a builder pattern (new()→with_runid())
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tower-telemetry/src/context.rs | Added mandatory runner_id field and refactored API to use builder pattern |
| crates/tower-telemetry/src/logging.rs | Updated logging macro to emit runner_id in all telemetry events |
| crates/tower-cmd/src/run.rs | Updated CLI to use new Context API with "local-runner" identifier |
| crates/tower-runtime/tests/subprocess_test.rs | Updated test to provide required runner_id parameter |
| crates/tower-runtime/tests/local_test.rs | Updated all test cases to provide required runner_id parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ExecutionSpec { | ||
| id, | ||
| telemetry_ctx: tower_telemetry::Context::new(), | ||
| telemetry_ctx: tower_telemetry::Context::new("runner-id".to_string()), |
There was a problem hiding this comment.
The hardcoded test value 'runner-id' is ambiguous. Consider using a more descriptive identifier like 'test-subprocess-runner' to clearly indicate this is a test context.
| telemetry_ctx: tower_telemetry::Context::new("runner-id".to_string()), | |
| telemetry_ctx: tower_telemetry::Context::new("test-subprocess-runner".to_string()), |
| // We need to create the package, which will load the app | ||
| let opts = StartOptions { | ||
| ctx: tower_telemetry::Context::new(), | ||
| ctx: tower_telemetry::Context::new("runner-id".to_string()), |
There was a problem hiding this comment.
The hardcoded test value 'runner-id' is ambiguous and repeated across multiple tests. Consider using a more descriptive identifier like 'test-local-runner' or defining a test constant to maintain consistency.
Uh oh!
There was an error while loading. Please reload this page.
Add runner-id to telemetry as a mandatory part of telemetry context to understand issues