Add OpenTelemetry-native host and process monitoring - #14051
Add OpenTelemetry-native host and process monitoring#14051mike-realuptime wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Several YAML files have an incomplete Apache 2.0 license header (missing the leading “Licensed to the Apache Software Foundation (ASF)…” line).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds OpenTelemetry-native host and process monitoring support (Linux + Windows) by extending existing MAL rules to accept either exporter-based metrics or OTel hostmetrics, and introduces a new E2E case that validates the OTel hostmetrics + pre-aggregated process pipeline end-to-end.
Changes:
- Extend Linux
vm.yamland Windowswindows.yamlMAL rules to map both exporter and OTel hostmetrics sources into the same canonical SkyWalking meter targets (avoiding rule ownership collisions). - Add new process MAL rules (
process-hostmetrics-{linux,windows}.yaml) modeling normalized process groups as logical SkyWalking instances. - Add a new E2E case (
vm/otel-hostmetrics) including collector config, compose wiring, and workflow integration.
File summaries
| File | Description |
|---|---|
| test/e2e-v2/cases/vm/prometheus-node-exporter/otel-rules/vm.yaml | Extends Linux VM MAL to accept both node-exporter and OTel hostmetrics under vm-monitoring. |
| test/e2e-v2/cases/vm/otel-hostmetrics/otel-rules/windows.yaml | Adds Windows VM MAL rules for the OTel-hostmetrics E2E case. |
| test/e2e-v2/cases/vm/otel-hostmetrics/otel-rules/vm.yaml | Adds Linux VM MAL rules for the OTel-hostmetrics E2E case. |
| test/e2e-v2/cases/vm/otel-hostmetrics/otel-rules/process-hostmetrics-windows.yaml | Adds Windows process MAL rules mapping normalized process groups to instances. |
| test/e2e-v2/cases/vm/otel-hostmetrics/otel-rules/process-hostmetrics-linux.yaml | Adds Linux process MAL rules mapping normalized process groups to instances. |
| test/e2e-v2/cases/vm/otel-hostmetrics/otel-collector-config.yaml | Adds an otelcol-contrib config for host + process collection, normalization, grouping, and aggregation. |
| test/e2e-v2/cases/vm/otel-hostmetrics/expected/process-count-3.yml | Adds an assertion verifying grouped process count aggregation (=3). |
| test/e2e-v2/cases/vm/otel-hostmetrics/entrypoint.sh | Starts 3 sleep processes and runs the collector in the E2E container. |
| test/e2e-v2/cases/vm/otel-hostmetrics/e2e.yaml | Defines the new E2E verification steps for hostmetrics + process metrics. |
| test/e2e-v2/cases/vm/otel-hostmetrics/Dockerfile.otelcol | Builds a minimal image bundling otelcol-contrib plus the E2E entrypoint. |
| test/e2e-v2/cases/vm/otel-hostmetrics/docker-compose.yml | Wires OAP + BanyanDB + collector container; mounts MAL rules and collector config. |
| oap-server/server-starter/src/main/resources/otel-rules/windows.yaml | Updates production Windows infrastructure MAL to accept both windows-exporter and OTel hostmetrics sources. |
| oap-server/server-starter/src/main/resources/otel-rules/vm.yaml | Updates production Linux infrastructure MAL to accept both node-exporter and OTel hostmetrics sources. |
| oap-server/server-starter/src/main/resources/otel-rules/process-hostmetrics-windows.yaml | Adds production Windows process MAL rules for normalized process groups. |
| oap-server/server-starter/src/main/resources/otel-rules/process-hostmetrics-linux.yaml | Adds production Linux process MAL rules for normalized process groups. |
| .github/workflows/skywalking.yaml | Adds the new “VM OpenTelemetry Hostmetrics” E2E job entry and pins otelcol-contrib version via env. |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -1,4 +1,3 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one or more | |||
| # contributor license agreements. See the NOTICE file distributed with | |||
| @@ -0,0 +1,257 @@ | |||
| # contributor license agreements. See the NOTICE file distributed with | |||
| @@ -1,4 +1,3 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one or more | |||
| # contributor license agreements. See the NOTICE file distributed with | |||
|
Thanks for the contribution. I reviewed the current head and reproduced the Collector path with
The |
3a763b8 to
fe5d5ff
Compare
|
Thank you very much for the detailed review. I have addressed the points you raised and pushed an updated version of the PR. This review has been a particularly valuable learning experience for me. Several of the issues you pointed out helped me better understand both SkyWalking's metric contracts and the OpenTelemetry Collector processing model. I sincerely appreciate the time you spent reproducing the setup and explaining the problems so precisely. The changes are summarized below:
The final validation is green, including:
Thank you again for the thorough review and for the opportunity to improve both the implementation and my understanding of SkyWalking's architecture. |
| system.cpu.utilization: | ||
| enabled: true |
There was a problem hiding this comment.
[P1] Preserve the existing Linux CPU metric scale
Collector Contrib 0.158.0 defaults system.cpu.utilization to averaging across cores. The new MAL expressions treat this value as summed CPU utilization, changing the existing meter_vm_cpu_total_percentage and meter_vm_cpu_average_used semantics. Four fully busy cores must report 400%, but this configuration produces 100%. The new normalized metric then divides again and reports 25% instead of 100%.
Please retain per-core attributes or configure sum aggregation, update the corresponding E2E configuration, and assert these values on a multi-core input. This enhancement should preserve the existing metric scale.
| (system_cpu_time * 100) | ||
| .tagNotEqual('mode','idle') | ||
| .sum(['node_identifier_host_name']) | ||
| .rate('PT1M') |
There was a problem hiding this comment.
[P2] Avoid overlapping CPU states in the new Windows normalized metric
Native Windows system CPU time already includes interrupt time. Summing every non-idle state adds interrupt again, inflating the new meter_win_cpu_norm_percentage and potentially exceeding 100%.
Please calculate this new normalized metric from non-overlapping user and system time. The existing total-CPU rule already sums all non-idle modes; this finding concerns the new normalized metric, rather than a regression in the existing exporter path.
| | Sockets Used | count | `meter_vm_sockets_used` | Kernel sockets currently in use | Yes | No | Yes | | ||
| | UDP In Use | count | `meter_vm_udp_inuse` | UDP sockets currently in use | Yes | No | Yes | | ||
| | Filefd Allocated | count | `meter_vm_filefd_allocated` | Host-level allocated file descriptors from Linux `/proc/sys/fs/file-nr` | Yes | No | — | | ||
| | Network Connections | count | `meter_vm_network_connections` | TCP connections grouped by protocol and state | — | Yes | — | |
There was a problem hiding this comment.
[P3] Remove or implement the advertised network-connections metric
The supported-metrics table lists meter_vm_network_connections, but no corresponding MAL rule is included. Users following this documentation cannot query that metric. Please remove the entry or provide the rule that produces it.
Summary
This PR adds OpenTelemetry Collector hostmetrics support for Linux and Windows infrastructure monitoring, including host-level metrics and logical process monitoring.
Key changes:
process-hostmetrics-linuxandprocess-hostmetrics-windows.state -> modeandprocess.handles -> process.open_handles.The final test suite is green across MAL runtime tests, backend build, Linux host/process E2E, Windows OTLP fixture coverage, raw Collector assertions, and the existing node-exporter regression E2E.
No Horizon/UI changes are included in this PR.