Repository navigation
Data Flow Metrics - #3903
Open
ryanmelt wants to merge 4 commits into
Open
Data Flow Metrics#3903ryanmelt wants to merge 4 commits into
ryanmelt wants to merge 4 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3903 +/- ##
==========================================
+ Coverage 80.28% 80.62% +0.33%
==========================================
Files 909 915 +6
Lines 70551 70675 +124
Branches 2741 2774 +33
==========================================
+ Hits 56643 56980 +337
+ Misses 13237 12992 -245
- Partials 671 703 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
jmthomas
previously approved these changes
Sep 26, 2026
jmthomas
left a comment
jmthomas
left a comment
Member
There was a problem hiding this comment.
Cool new feature. I added a playwright test and bumped the storage down to 20MB from 100MB to prevent huge overruns with a large number of interfaces. This could especially become a problem with a TCP server. That seemed resonable since the red limit was 10MB.
…etails Simplify nested ternaries in flowMetrics, guard the details dialog against missing details, and add a Data Flows Playwright test for INST2 processing and INST2_INT metrics. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ryanmelt
force-pushed
the
fall_behind_detection
branch
from
October 11, 2026 23:48
d9dcb04 to
e566cee
Compare
Member
Author
|
#3902 needs to be merged first and then this can be reviewed. |
update() handles all errors internally; void makes the intentionally ignored promise explicit to satisfy SonarCloud rule S9383. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Detailed metrics API openc3-cosmos-init/plugins/packages/openc3-js-common/src/services/openc3Api.js, openc3/lib/openc3/api/metrics_api.rb, openc3/spec/api/metrics_api_spec.rb |
get_metrics accepts a detailed option. Detailed mode returns scoped per-service reports; default mode keeps the existing aggregation. API specs cover both modes and authorization. |
Metric grouping, health, and polling openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/src/tools/CmdTlmServer/InterfaceComponents/flowMetrics.js, .../useFlowMetrics.js, openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/tests/flowMetrics.test.js, openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/package.json |
Metric helpers group reports and assign health states using delay, duration, buffer, and age thresholds. The composable polls for updates, and the package adds a test script. Unit tests cover states, formatting, and report handling. |
Graph health indicators and details routing openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/src/tools/CmdTlmServer/InterfaceComponents/InterfaceFlowChart.vue, .../CosmosNode.vue, .../InterfaceNode.vue, .../RouterNode.vue, .../TargetNode.vue, .../FlowHealthIndicator.vue, openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/src/tools/CmdTlmServer/InterfacesTab.vue, .../RoutersTab.vue |
The flow chart passes health data to interface, router, and COSMOS nodes. Their indicators show health states and open the relevant details view. Node sizing and dialog rendering are updated. |
Metric details, pause state, and validation openc3-cosmos-init/plugins/packages/openc3-cosmos-tool-cmdtlmserver/src/tools/CmdTlmServer/InterfaceComponents/DetailsTable.vue, .../FlowMetricsPanel.vue, .../TargetProcessingDetails.vue, playwright/tests/cmd-tlm-server/data-flows.p.spec.ts, docs.openc3.com/docs/tools/cmd-tlm-server.md |
The details panels display metric values, status, service, and report age. Pause controls retain a snapshot of displayed health. Playwright tests cover target and interface dialogs, and the documentation describes metrics and status behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
actor User
participant InterfaceFlowChart
participant useFlowMetrics
participant OpenC3Api
participant OpenC3ApiServer as OpenC3::Api
participant MetricModel
participant FlowHealthIndicator
User->>InterfaceFlowChart: Open Data Flows view
InterfaceFlowChart->>useFlowMetrics: Start metrics composable
useFlowMetrics->>OpenC3Api: Request detailed metrics
OpenC3Api->>OpenC3ApiServer: get_metrics with detailed option
OpenC3ApiServer->>MetricModel: Load scoped service reports
MetricModel-->>OpenC3ApiServer: Return service reports
OpenC3ApiServer-->>OpenC3Api: Return detailed metrics
OpenC3Api-->>useFlowMetrics: Return reports
useFlowMetrics-->>InterfaceFlowChart: Provide grouped health
InterfaceFlowChart-->>FlowHealthIndicator: Render node health
User->>FlowHealthIndicator: Open metric details
FlowHealthIndicator-->>InterfaceFlowChart: Emit details event
InterfaceFlowChart-->>User: Show selected flow details
Suggested reviewers: emilyragan, clayandgen
Merge Risk: 
7577d
-
The new Data Flows metrics view has no established merge-blocking issue in the supplied evidence. Complete the normal test checks before merging.
Security architecture risk: 
7577d
-
The new view exposes more granular service information through the existing metrics permissions. No introduced authorization bypass was established, but the deployed restrictions on access to individual scopes could not be verified.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- observed — Detailed mode increases information granularity for callers passing the existing metrics authorization check: it exposes individual service identifiers, metric reports, and update timestamps instead of only aggregates. Retrieval remains scope-selected through MetricModel; detailed mode does not introduce a separate datastore lookup or new scope selector.
Trust Boundaries and Controls
- observed — Compared with the PR base, get_metrics retains authorization before scoped model access. The inspected RPC path retains whitelist dispatch and authorization-header token propagation. These controls do not establish the deployed role-to-scope policy: the enterprise authorization override was unavailable, so absolute cross-scope entitlement enforcement remains unverified rather than an observed PR regression.
Hardening Proposals
- proposed — Validate both response modes against the deployed scope-authorization policy, including a token authorized for one scope requesting another scope. This would resolve the remaining entitlement uncertainty; it is not evidence of a newly introduced bypass.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (13 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: adding metrics to Data Flow diagrams. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (13 skipped: 13 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
ryanmelt
added
the
review-and-merge-ready
This branch has not been deployed
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.
What changed
Add new metrics view for internal processing and Interface in the Data Flow Diagrams
Why it changed
Better visibility if anything is falling behind
Testing strategy
Manually tested
Summary by CodeRabbit
New Features
Documentation