Add resource stats gRPC API #3243
Conversation
📝 WalkthroughWalkthroughA new Stats module is added to expose resource statistics (CPU and memory cgroup metrics) via gRPC service. The implementation spans proto definitions for payload types and the Stats service, Go code for cgroup measurement across v1/v2 modes, generated gRPC/Rust bindings, integration with the server, error definitions, tests, documentation, and CI workflow trigger updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StatsServer as Stats gRPC Server
participant CgroupMetrics as Cgroup Measurement
participant Response
Client->>StatsServer: ResourceStats(Empty)
activate StatsServer
rect rgb(200, 220, 240)
Note over StatsServer,CgroupMetrics: Collect Resource Stats
StatsServer->>CgroupMetrics: measureCgroupStats()
activate CgroupMetrics
CgroupMetrics->>CgroupMetrics: detectCgroupMode()
alt CGv2 Detected
CgroupMetrics->>CgroupMetrics: readCgroupV2Metrics()
Note over CgroupMetrics: Parse memory.current/max<br/>cpu.stat/cpu.max
else CGv1 Detected
CgroupMetrics->>CgroupMetrics: readCgroupV1Metrics()
Note over CgroupMetrics: Parse memory_usage_in_bytes<br/>cpuacct.usage, cfs_quota_us
end
CgroupMetrics->>CgroupMetrics: Sample 1: collect metrics
CgroupMetrics->>CgroupMetrics: Sleep 100ms
CgroupMetrics->>CgroupMetrics: Sample 2: collect metrics
CgroupMetrics->>CgroupMetrics: calculateCpuUsageCores(sample1, sample2)
deactivate CgroupMetrics
end
StatsServer->>Response: Populate Info_ResourceStats<br/>name, ip, cgroupStats
StatsServer-->>Client: Return ResourceStats
deactivate StatsServer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Breakdown: Generated proto/gRPC artifacts and workflow updates are low-effort repetitive changes. Core implementation complexity is moderate: the Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (46)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
.github/workflows/dockers-gateway-mirror-image.yaml (1)
219-219: PR target: internal stats — duplicate of generator noteSame generator-update verification as noted in agent-ngt workflow.
.github/workflows/dockers-agent-faiss-image.yaml (1)
219-219: PR target: internal stats — duplicate of generator noteMatches earlier workflows; ensure generator template was updated.
.github/workflows/dockers-readreplica-rotate-image.yaml (1)
205-205: PR target: internal stats glob — duplicate of generator noteConfirm
hack/docker/gen/main.goincludes these paths..github/workflows/dockers-gateway-lb-image.yaml (1)
216-216: PR target: internal stats — duplicate of generator noteSame generator verification as above.
.github/workflows/dockers-index-creation-image.yaml (1)
207-207: PR target: internal stats glob — duplicate of generator noteConfirm the generator includes these entries.
.github/workflows/dockers-index-save-image.yaml (1)
207-207: PR target: internal stats glob — duplicate of generator noteSame generator-update verification applies.
.github/workflows/dockers-index-operator-image.yaml (1)
207-207: PR target: internal stats — duplicate of generator noteEnsure
hack/docker/gen/main.gochanges are committed to keep these globs on regeneration.
🧹 Nitpick comments (5)
internal/net/grpc/stats/doc.go (1)
17-18: Nit: capitalize “gRPC” in the package comment.Tiny doc polish for GoDoc consistency.
-// Package stats provides generic functionality for grpc stats. +// Package stats provides generic functionality for gRPC stats.apis/proto/v1/payload/payload.proto (1)
651-657: Align field naming and add minimal validation to new ResourceStats.Proto style in this file favors snake_case (e.g.,
app_name,cluster_ip). Consider renaming fields and adding basic validations while the API is new.Apply:
message ResourceStats { - string name = 1; - string ip = 2 [(buf.validate.field).string.ipv4 = true]; - double cpuUsage = 3; - double memoryUsage = 4; + string name = 1 [(buf.validate.field).string.min_len = 1]; + string ip = 2 [(buf.validate.field).string.ipv4 = true]; + double cpu_usage = 3 [(buf.validate.field).double.gte = 0]; // define as ratio or cores; adjust if percent + double memory_usage = 4 [(buf.validate.field).double.gte = 0]; // define unit (bytes or ratio); adjust as needed }If you intend percent values, also cap at 100 with
lte = 100. If you prefer richer detail, you could reuse existingCPU/Memorymessages to stay consistent withInfo.Pod.Please confirm the intended units (ratio/percent/cores; bytes/ratio) so we can finalize validation and docs updates (apis/docs, swagger, Go/Rust bindings).
apis/docs/v1/payload.md.tmpl (1)
1583-1601: Document units and semantics for ResourceStats fieldsPlease add concise descriptions so users know these are percentages and what the IP represents.
Apply this diff to enrich the field table:
{{- define "_field:payload.v1.Info.ResourceStats" }} - Info.ResourceStats | field | type | label | description | | :---: | :--- | :---- | :---------- | - | name | string | | | - | ip | string | | | - | cpuUsage | double | | | - | memoryUsage | double | | | + | name | string | | Hostname of the node/pod reporting stats. | + | ip | string | | Local IP address (IPv4 or IPv6) of the node/pod; may be "unknown" if not resolvable. | + | cpuUsage | double | | Host CPU utilization percentage [0-100], sampled over ~100 ms. | + | memoryUsage | double | | System memory utilization percentage [0-100], computed as (MemTotal - MemAvailable)/MemTotal. | {{- end -}}apis/docs/v1/docs.md (1)
44-45: Clarify Info.ResourceStats field descriptions and unitsAdd brief, user‑facing descriptions so consumers know these are host‑level percentages and what “ip” represents.
Apply this diff:
### Info.ResourceStats -Represent the resource stats +Represent host resource utilization percentages. | Field | Type | Label | Description | | ----------- | ----------------- | ----- | ----------- | -| name | [string](#string) | | | -| ip | [string](#string) | | | -| cpuUsage | [double](#double) | | | -| memoryUsage | [double](#double) | | | +| name | [string](#string) | | Hostname of the reporting node/pod. | +| ip | [string](#string) | | Local IP address (IPv4/IPv6); may be "unknown". | +| cpuUsage | [double](#double) | | CPU utilization [%] sampled over ~100 ms. | +| memoryUsage | [double](#double) | | System memory utilization [%] = (MemTotal - MemAvailable)/MemTotal. |Also applies to: 567-579
apis/swagger/v1/rpc/stats/stats.swagger.json (1)
72-91: Add property descriptions to v1InfoResourceStatsSmall UX win: describe each field and clarify units.
Apply this diff:
"v1InfoResourceStats": { "type": "object", "properties": { "name": { - "type": "string" + "type": "string", + "description": "Hostname of the reporting node/pod." }, "ip": { - "type": "string" + "type": "string", + "description": "Local IP address (IPv4 or IPv6); may be \"unknown\"." }, "cpuUsage": { "type": "number", - "format": "double" + "format": "double", + "description": "CPU utilization percentage [0-100], sampled over ~100 ms." }, "memoryUsage": { "type": "number", - "format": "double" + "format": "double", + "description": "System memory utilization percentage [0-100] = (MemTotal - MemAvailable)/MemTotal." } }, "title": "Represent the resource stats" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
apis/grpc/v1/agent/core/agent.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/agent/sidecar/sidecar.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/agent/sidecar/sidecar_vtproto.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/discoverer/discoverer.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/filter/egress/egress_filter.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/filter/ingress/ingress_filter.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/meta/meta.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/mirror/mirror.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/payload/payload.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/payload/payload.pb.json.gois excluded by!**/*.pb.json.goapis/grpc/v1/payload/payload_vtproto.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/rpc/errdetails/error_details.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/rpc/stats/stats.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/rpc/stats/stats_vtproto.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/filter.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/flush.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/index.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/insert.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/object.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/remove.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/search.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/update.pb.gois excluded by!**/*.pb.go,!**/*.pb.goapis/grpc/v1/vald/upsert.pb.gois excluded by!**/*.pb.go,!**/*.pb.go
📒 Files selected for processing (30)
.gitfiles(5 hunks).github/workflows/dockers-agent-faiss-image.yaml(4 hunks).github/workflows/dockers-agent-ngt-image.yaml(4 hunks).github/workflows/dockers-agent-sidecar-image.yaml(4 hunks).github/workflows/dockers-benchmark-job-image.yaml(4 hunks).github/workflows/dockers-benchmark-operator-image.yaml(4 hunks).github/workflows/dockers-discoverer-k8s-image.yaml(4 hunks).github/workflows/dockers-gateway-filter-image.yaml(4 hunks).github/workflows/dockers-gateway-lb-image.yaml(4 hunks).github/workflows/dockers-gateway-mirror-image.yaml(4 hunks).github/workflows/dockers-index-correction-image.yaml(4 hunks).github/workflows/dockers-index-creation-image.yaml(4 hunks).github/workflows/dockers-index-deletion-image.yaml(4 hunks).github/workflows/dockers-index-exportation-image.yaml(4 hunks).github/workflows/dockers-index-operator-image.yaml(4 hunks).github/workflows/dockers-index-save-image.yaml(4 hunks).github/workflows/dockers-manager-index-image.yaml(4 hunks).github/workflows/dockers-readreplica-rotate-image.yaml(4 hunks)CHANGELOG.md(4 hunks)apis/docs/v1/docs.md(5 hunks)apis/docs/v1/payload.md.tmpl(2 hunks)apis/proto/v1/payload/payload.proto(1 hunks)apis/proto/v1/rpc/stats/stats.proto(1 hunks)apis/swagger/v1/rpc/stats/stats.swagger.json(1 hunks)charts/vald/templates/gateway/ing.yaml(1 hunks)internal/net/grpc/stats/doc.go(1 hunks)internal/net/grpc/stats/stats.go(1 hunks)internal/servers/server/server.go(2 hunks)rust/libs/proto/src/payload.v1.rs(1 hunks)rust/libs/proto/src/rpc.v1.tonic.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-06-10T19:25:49.832Z
Learnt from: kpango
PR: vdaas/vald#2491
File: pkg/gateway/lb/handler/grpc/handler.go:306-316
Timestamp: 2024-06-10T19:25:49.832Z
Learning: User: kpango
PR: vdaas/vald#2491
File: pkg/gateway/lb/handler/grpc/handler.go:306-316
Timestamp: 2024-05-07T04:15:11.886Z
Learning: In this project, vtprotobuf is used, which provides enhanced methods for protobuf objects: CloneVT (object cloning), EqualVT (object comparison), SizeVT (returns object size), MarshalVT (faster version of proto.Marshal), and UnmarshalVT (faster version of proto.Unmarshal). These methods offer better performance than usual protobuf usage.
Applied to files:
.github/workflows/dockers-gateway-lb-image.yaml.github/workflows/dockers-gateway-mirror-image.yaml.gitfiles.github/workflows/dockers-agent-ngt-image.yaml.github/workflows/dockers-gateway-filter-image.yaml
📚 Learning: 2025-05-01T08:44:07.726Z
Learnt from: kpango
PR: vdaas/vald#0
File: :0-0
Timestamp: 2025-05-01T08:44:07.726Z
Learning: PR #2927 addressed gRPC connection panic issues by removing the singleflight.Group[pool.Conn] field from the gRPCClient struct in internal/net/grpc/client.go and introducing a centralized handleError function for consistent error processing in both Connect and Disconnect methods.
Applied to files:
.github/workflows/dockers-benchmark-job-image.yaml
🧬 Code graph analysis (3)
internal/servers/server/server.go (2)
internal/net/grpc/stats/stats.go (1)
Register(42-45)internal/net/grpc/health/health.go (1)
Register(27-35)
internal/net/grpc/stats/stats.go (4)
internal/servers/server/server.go (2)
Server(47-52)New(138-265)internal/os/hostname.go (1)
Hostname(49-54)internal/net/net.go (1)
LoadLocalIP(362-377)internal/errors/errors.go (2)
Wrapf(112-120)Wrap(99-107)
rust/libs/proto/src/payload.v1.rs (1)
rust/libs/proto/src/rpc.v1.rs (28)
full_name(48-48)full_name(72-72)full_name(87-87)full_name(131-131)full_name(136-136)full_name(175-175)full_name(180-180)full_name(242-242)full_name(247-247)full_name(265-265)full_name(295-295)full_name(324-324)full_name(329-329)full_name(347-347)type_url(48-48)type_url(72-72)type_url(87-87)type_url(131-131)type_url(136-136)type_url(175-175)type_url(180-180)type_url(242-242)type_url(247-247)type_url(265-265)type_url(295-295)type_url(324-324)type_url(329-329)type_url(347-347)
🪛 Checkov (3.2.334)
apis/swagger/v1/rpc/stats/stats.swagger.json
[high] 1-93: Ensure that security requirement defined in securityDefinitions - version 2.0 files
(CKV_OPENAPI_6)
[high] 1-93: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-93: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 61-68: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 1-93: Ensure that securityDefinitions is defined and not empty - version 2.0 files
(CKV_OPENAPI_1)
🔇 Additional comments (49)
charts/vald/templates/gateway/ing.yaml (1)
333-333: No-op formatting change.Trailing newline added; fine to merge.
.github/workflows/dockers-agent-sidecar-image.yaml (1)
41-41: CI triggers expanded appropriately for Stats and Payload.Globs added for apis/grpc/v1/payload and stats plus internal/net/grpc/stats in both PR and PR_TARGET; looks good.
Also applies to: 43-43, 111-111, 179-179, 181-181, 249-249
.github/workflows/dockers-index-exportation-image.yaml (1)
42-42: CI path filters updated for Stats; LGTM.Both stats paths are covered in PR and PR_TARGET blocks.
Also applies to: 96-96, 153-153, 207-207
.github/workflows/dockers-manager-index-image.yaml (1)
44-44: Stats path globs added consistently.Change matches the pattern used in other docker workflows.
Also applies to: 100-100, 165-165, 221-221
.github/workflows/dockers-benchmark-operator-image.yaml (1)
40-40: Benchmark operator CI now watches Stats and Payload; OK.Additions mirror other workflows.
Also applies to: 42-42, 97-97, 158-158, 160-160, 215-215
.github/workflows/dockers-discoverer-k8s-image.yaml (1)
43-43: Discoverer CI triggers include Stats paths; OK.Consistent with the rest of the repo updates.
Also applies to: 99-99, 163-163, 219-219
.github/workflows/dockers-gateway-filter-image.yaml (1)
44-44: Confirm both stats globs appear twice in every dockers- workflow*New stats path globs were added for pull_request and pull_request_target — ensure each dockers-*.yaml contains >=2 occurrences of both:
- apis/grpc/v1/rpc/stats/*.go
- internal/net/grpc/stats/*.go
#!/bin/bash set -euo pipefail cd "$(git rev-parse --show-toplevel 2>/dev/null || printf '%s' .)" shopt -s nullglob missing=0 for f in .github/workflows/dockers-*.yaml .github/workflows/dockers-*.yml; do [ -f "$f" ] || continue c_rpc=$(rg -nF 'apis/grpc/v1/rpc/stats/*.go' "$f" || true | wc -l | tr -d ' ') c_int=$(rg -nF 'internal/net/grpc/stats/*.go' "$f" || true | wc -l | tr -d ' ') if [ "$c_rpc" -lt 2 ] || [ "$c_int" -lt 2 ]; then echo "WARN: $f -> rpc/stats: $c_rpc, internal/net/grpc/stats: $c_int" missing=1 fi done exit $missing.github/workflows/dockers-agent-ngt-image.yaml (4)
43-43: Add stats RPC glob — LGTMIncluding
apis/grpc/v1/rpc/stats/*.goensures image builds trigger when the new gRPC surface changes.
99-99: Watch internal stats implementation — LGTMAdding
internal/net/grpc/stats/*.gocovers server-side updates for the new API.
168-168: Mirror PR target trigger for stats RPC — LGTMKeeps PR/PR_TARGET filters consistent.
224-224: Mirror PR target trigger for internal stats — LGTM; ensure generator updatedSince this file is generated, confirm
hack/docker/gen/main.goincludes these new globs so future regenerations don’t drop them..github/workflows/dockers-gateway-mirror-image.yaml (3)
43-43: Add stats RPC glob — LGTM
99-99: Add internal stats glob — LGTM
163-163: PR target: stats RPC — LGTM.github/workflows/dockers-agent-faiss-image.yaml (3)
43-43: Add stats RPC glob — LGTM
97-97: Add internal stats glob — LGTM
165-165: PR target: stats RPC — LGTM.github/workflows/dockers-readreplica-rotate-image.yaml (5)
40-40: Add payload glob — good catch
ResourceStatsadditions in payloads will now trigger this image build.
42-42: Add stats RPC glob — LGTM
95-95: Add internal stats glob — LGTM
150-150: PR target: payload glob — LGTM
152-152: PR target: stats RPC glob — LGTM.github/workflows/dockers-gateway-lb-image.yaml (3)
43-43: Add stats RPC glob — LGTM
98-98: Add internal stats glob — LGTM
161-161: PR target: stats RPC — LGTM.github/workflows/dockers-index-creation-image.yaml (3)
44-44: Add stats RPC glob — LGTM
96-96: Add internal stats glob — LGTM
155-155: PR target: stats RPC glob — LGTM.github/workflows/dockers-index-save-image.yaml (3)
44-44: Add stats RPC glob — LGTM
96-96: Add internal stats glob — LGTM
155-155: PR target: stats RPC glob — LGTM.github/workflows/dockers-index-operator-image.yaml (5)
40-40: Add payload glob — LGTMCovers changes to new/updated payload types (e.g., ResourceStats).
42-42: Add stats RPC glob — LGTM
96-96: Add internal stats glob — LGTM
151-151: PR target: payload glob — LGTM
153-153: PR target: stats RPC — LGTMCHANGELOG.md (2)
429-446: Changelog formatting tweak is harmless.No content changes; safe to keep.
511-548: OK to keep autogenerated “Summary by CodeRabbit” spacing edits.No action needed.
.github/workflows/dockers-index-deletion-image.yaml (1)
43-43: Additions match the new Stats surface; good to go.Both globs (apis/grpc/v1/rpc/stats/.go and internal/net/grpc/stats/.go) appear in both pull_request and pull_request_target sections — occurrences at lines 43, 96, 154, 207. Same note on generator parity and repo-wide consistency for all docker workflows.
.github/workflows/dockers-benchmark-job-image.yaml (1)
42-42: CI path filters: globs added across dockers workflows; generator must provide missing filesBoth globs (apis/grpc/v1/rpc/stats/.go and internal/net/grpc/stats/.go) are present across .github/workflows/dockers-*.yaml; the referenced source directories/files do not exist in the repo — confirm the generator will produce these paths or commit the generated files.
internal/servers/server/server.go (2)
38-39: Import of stats package — correct placement and naming.
247-249: Register Stats before Health — correct ordering.Registering stats prior to health ensures Health sets SERVING for the Stats service via GetServiceInfo().
rust/libs/proto/src/payload.v1.rs (1)
1357-1373: ResourceStats message: prost::Name metadata verified — no issues.impl ::prost::Name for ResourceStats contains full_name "payload.v1.Info.ResourceStats" and type_url "/payload.v1.Info.ResourceStats"; field types match the proto. Regenerate this file from the proto (do not edit manually).
apis/proto/v1/rpc/stats/stats.proto (1)
17-38: Do not change this single route to /v1/resource/stats — follow the repo's unversioned REST convention
- Repo uses unversioned top-level HTTP bindings; examples: apis/proto/v1/vald/object.proto:122 -> /object/list, apis/proto/v1/vald/index.proto:35 -> /index/info, apis/proto/v1/agent/core/agent.proto:38 -> /index/save, and apis/swagger/v1/rpc/stats/stats.swagger.json:15 -> /resource/stats. If you want versioned paths, do a repo-wide migration (update proto HTTP bindings + generated swagger) rather than changing just this endpoint.
- Optional refactor: replace payload.v1.Empty with a dedicated request message to allow future filters (scope/format); update apis/proto/v1/rpc/stats/stats.proto accordingly.
Likely an incorrect or invalid review comment.
.gitfiles (1)
212-214: No action needed — .gitfiles is alphabetically sorted.Verification: the provided script printed "OK: .gitfiles is sorted". The "/bin/bash: line 5: !: command not found" is an execution-environment artifact and can be ignored.
apis/docs/v1/payload.md.tmpl (1)
267-272: OK to wire ResourceStats into the docs templatesThe scheme/field template hooks look consistent with the surrounding patterns. No issues.
apis/docs/v1/docs.md (1)
144-146: Include the new Stats service in ToC and keep the wording tightWording is fine; ensure docs generation keeps this section stable when templates regenerate.
If docs.md is generated, please re-run docs generation after template changes to keep them in sync.
Also applies to: 1642-1657
rust/libs/proto/src/rpc.v1.tonic.rs (1)
100-124: Generated gRPC client/server surface looks goodClient/server scaffolding for ResourceStats is standard Prost/Tonic output. No changes needed.
Also applies to: 217-299
apis/swagger/v1/rpc/stats/stats.swagger.json (1)
15-35: Add security to this operation or explicitly mark it publicI searched apis/swagger/v1 and found no securityDefinitions/security entries. If this endpoint requires auth, add a root-level securityDefinitions (Swagger 2.0) and include a "security" array on paths./resource/stats (GET) in apis/swagger/v1/rpc/stats/stats.swagger.json; otherwise document the endpoint as public.
Signed-off-by: Jumpei Tanikawa <[email protected]>
Deploying vald with
|
| Latest commit: |
a249751
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f02ff1ae.vald.pages.dev |
| Branch Preview URL: | https://feature-internal-add-resourc.vald.pages.dev |
github-actions
bot
added
area/discoverer
area/manager/index
area/filter/ingress
area/filter/egress
area/internal
type/ci
area/helm
area/agent/sidecar
area/gateway/lb
area/gateway/filter
area/agent/core/faiss
area/agent/core/ngt
area/gateway/mirror
area/index/job/correction
area/index/job/creation
area/index/job/save
area/index/job/readreplica
actions/backport/release/v1.7
size/XL
type/feature
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3243 +/- ##
==========================================
+ Coverage 17.74% 18.53% +0.79%
==========================================
Files 126 128 +2
Lines 13647 12193 -1454
==========================================
- Hits 2421 2260 -161
+ Misses 10949 9640 -1309
- Partials 277 293 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jumpei527
changed the title
Profile Report
|
Matts966
dismissed stale reviews from coderabbitai[bot] and themself
via
d1e4666
Matts966
force-pushed
the
feature/internal/add-resource-stats-grpc-api
branch
from
d1e4666 to
5f7e780
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/e2e.v2.yaml (2)
95-97: Minor: Redundant null check in combo function.The check
o.environment === null(line 96) is unnecessary because baseInclude consistently uses string literals ("null","management", etc.) rather than actual null values. Consider simplifying toreturn o.scenario + '-' + o.deployment + '-' + o.cluster + '-' + o.environment;or adding a clarifying comment if the null check is intended for forward compatibility.
262-262: Note: Debug kubectl logs command with --tail=-1 may produce large output.The
kubectl logs -l app.kubernetes.io/component=helm-operator --tail=-1command at line 262 will fetch all logs (no limit), which could produce significant output if the helm-operator has been running for a while. This is acceptable for debugging, but consider monitoring CI log size limits if this becomes problematic in practice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e.v2.yaml(2 hunks)
🔇 Additional comments (3)
.github/workflows/e2e.v2.yaml (3)
46-115: Well-implemented dynamic matrix generation with robust fallback logic.The build-matrix job design is solid. The substring matching approach (line 101) provides flexible label-based filtering, and the fallback mechanism (line 105-107) ensures the workflow never gets stuck with an empty matrix. The label extraction safely handles undefined values (line 84), and the resulting JSON is correctly formatted for consumption by
fromJson().
44-44: Condition update properly gates detect-ci-container to workflow triggers.The updated condition correctly checks for label prefix
'actions/e2e-v2'alongside existing tag and dispatch triggers. The short-circuit AND logic at line 44 prevents evaluation of undefinedgithub.event.label.nameon non-labeled triggers.
120-123: Dynamic matrix dependency chain is correctly configured.The e2e job properly depends on both detect-ci-container (for image TAG) and build-matrix (for matrix definition), and the
fromJson()parsing of the string output is appropriate given theresult-encoding: stringsetting on the upstream github-script step.
5 participants
Description
This PR implements a new gRPC API for retrieving system resource statistics (CPU and memory usage) to enable better monitoring of Vald components.
What Changed
This implementation adds a
ResourceStatsgRPC service within theinternal/net/grpc/statspackage, providing comprehensive resource monitoring capabilities.Related Issue
#3215
Versions
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation