feat(byoo-perf): measure a baseline and report results - #637
Open
shobham-nv wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe performance suite now executes repeated load and measurement cycles. It parses collector and sink metrics, computes baseline reports with pod health, records partial or invalid results, and optionally persists JSON output. ChangesPerformance baseline measurement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PerfRunner
participant DeployClient
participant Collector
participant Sink
participant ReportBuilder
participant ResultsDir
PerfRunner->>DeployClient: StartLoad jobs
PerfRunner->>Collector: Scrape collector metrics
PerfRunner->>Sink: Scrape sink metrics
PerfRunner->>DeployClient: WaitLoad jobs
PerfRunner->>DeployClient: Collect pod health
PerfRunner->>ReportBuilder: Build baseline report
ReportBuilder->>ResultsDir: Persist JSON result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Comment |
sbaum1994
reviewed
Aug 5, 2026
sbaum1994
left a comment
sbaum1994
left a comment
Contributor
There was a problem hiding this comment.
Go review findings from the performance-reporting changes.
shobham-nv
force-pushed
the
shobham/419-byoo-perf-loadgen-sink
branch
from
August 10, 2026 15:51
89ae4fa to
833b809
Compare
shobham-nv
force-pushed
the
shobham/420-byoo-perf-measure
branch
from
August 11, 2026 04:53
c64a39d to
01f5652
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go`:
- Line 162: Update the help text for the results-dir flag in the command’s flag
definition to state that structured JSON output includes one file per shape and
repetition, matching the filenames generated by the multi-repetition write path.
- Around line 372-383: Update the repetition flow around StartLoad and measure
so it waits for every load-generator pod to reach started/running state before
measurement warmup begins. Use a bounded timeout context for this readiness
wait, propagate timeout or startup errors, and add a delayed-start test covering
pods that become ready after job creation.
- Around line 441-453: Update the flow around PodHealth and report.Build to
record health-query errors in Notes and mark the report partial when health is
unavailable, instead of passing an indistinguishable zero-value health result.
Ensure a later load-completion failure still takes precedence and preserves
invalid status, and add a test covering PodHealth failure.
- Around line 419-439: Update the snap function so Snapshot.At reflects when the
metric samples are captured, not when scraping begins. Perform the collector and
sink scrapes concurrently, then apply an explicit common timestamp after
successful responses are received; preserve warning handling for scrape errors.
Add coverage for delayed successful scrapes verifying the resulting window
duration and throughput denominator use the aligned snapshot timestamps.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/report/report.go`:
- Around line 164-176: Update Build in
src/compute-plane-services/byoo-otel-collector/perf/pkg/report/report.go (lines
164-176) to capture and pass the ok result from counterDelta to note for every
refused, sent, and failed logs and metrics counter. In
src/compute-plane-services/byoo-otel-collector/perf/pkg/report/report_test.go
(lines 69-112), add all required reported counters to the complete fixture or
assert the resulting notes and partial status; in lines 129-140, require every
reported counter for completeness.
In `@src/compute-plane-services/byoo-otel-collector/perf/README.md`:
- Around line 10-12: Replace the em dash in the README sentence describing
pass/fail thresholds with ASCII punctuation, such as a period or semicolon,
while preserving the sentence’s meaning.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5e8e3159-f345-4146-bff0-ed433eb4e2a4
📒 Files selected for processing (11)
src/compute-plane-services/byoo-otel-collector/VERSIONsrc/compute-plane-services/byoo-otel-collector/perf/README.mdsrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.gosrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/prom.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/prom_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/report.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/report_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/sink/sink.go
shobham-nv
force-pushed
the
shobham/420-byoo-perf-measure
branch
from
August 11, 2026 13:06
01f5652 to
d6ca79e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go (1)
274-284: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueJoin the pod-creation goroutine before the subtest ends.
The goroutine writes to the fake clientset without synchronization with the test body. If
WaitLoadStartedfails, the subtest can finish while the goroutine still creates pods. That leaks a goroutine into the next subtest and can log after the test completes. Use a channel orsync.WaitGroupto join before returning.♻️ Proposed change
+ done := make(chan struct{}) go func() { + defer close(done) time.Sleep(50 * time.Millisecond) for _, j := range jobs { _, _ = cs.CoreV1().Pods("byoo-perf").Create(ctx, runningLoadPod("byoo-perf", j.Name, jobLabelKey), metav1.CreateOptions{}) } }() + defer func() { <-done }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go` around lines 274 - 284, Synchronize the pod-creation goroutine in the test around WaitLoadStarted by adding a completion channel or sync.WaitGroup, and wait for it before the subtest can return, including when WaitLoadStarted fails. Keep the existing delayed creation behavior and ensure the fake clientset writes finish before test cleanup.src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go (1)
581-589: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
sleephides context cancellation.If the context is cancelled,
sleepreturns at once andmeasurecontinues to the next snapshot. The report is then built from a truncated window but still carriesstatus="ok". Return the cancellation to the caller and mark the report invalid.♻️ Proposed change
-func sleep(ctx context.Context, d time.Duration) { +func sleep(ctx context.Context, d time.Duration) error { t := time.NewTimer(d) defer t.Stop() select { case <-ctx.Done(): + return ctx.Err() case <-t.C: + return nil } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go` around lines 581 - 589, Update sleep to return the context cancellation error when ctx.Done() fires, while returning nil after the timer completes. In measure, propagate that error and mark the generated report invalid instead of continuing with the truncated snapshot window or leaving status="ok".src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go (1)
378-385: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
WaitLoadStartedappliestimeoutper Job, not to the whole wait.The loop calls
waitJobPodStartedfor each Job with the fulltimeout. With two generator Jobs the worst-case wait is2 * timeout. The caller passescfg.readyTimeout(default 3m), so a stuck run can block for 6 minutes before the measurement window starts. Derive one deadline for the whole set if you want a bounded startup wait.♻️ Proposed change to bound the total wait
func (c *Client) WaitLoadStarted(ctx context.Context, namespace string, jobs []*batchv1.Job, timeout time.Duration) error { + deadline := time.Now().Add(timeout) for _, j := range jobs { - if err := c.waitJobPodStarted(ctx, namespace, j.Name, timeout); err != nil { + remaining := time.Until(deadline) + if remaining <= 0 { + return fmt.Errorf("timed out waiting for load generator job %q pod to start", j.Name) + } + if err := c.waitJobPodStarted(ctx, namespace, j.Name, remaining); err != nil { return err } } return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go` around lines 378 - 385, Update Client.WaitLoadStarted to enforce timeout across all jobs rather than per job: derive a single deadline from the initial timeout, pass each waitJobPodStarted call only the remaining duration, and return the existing error when the overall deadline is exhausted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go`:
- Around line 370-394: Adjust the load duration used by the generator Jobs in
the baseline and corresponding measurement paths so it includes the startup
delay between client.StartLoad and WaitLoadStarted, ensuring measure’s warmup
and measurement window finish before generators stop. Preserve the existing
profile warmup and measurement values, and add a test covering that the measured
window ends before generator termination.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go`:
- Around line 397-415: Update waitJobPodStarted to ignore pods belonging to
previous Job runs while polling. Use the current Job’s UID as the pod owner
filter, or otherwise restrict pods by creation time, before evaluating phases;
only return failure for a Failed pod owned by the current job while continuing
to detect Running or Succeeded pods.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/render/render.go`:
- Around line 160-167: Add regression tests covering the Render-to-BenchPod
flow: verify owner labels and annotations propagate, suite labels override only
intended keys, and mutating the returned Pod does not mutate Result. Use the
existing Render and BenchPod test symbols and run the repository-native Go test
runner.
---
Nitpick comments:
In `@src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go`:
- Around line 581-589: Update sleep to return the context cancellation error
when ctx.Done() fires, while returning nil after the timer completes. In
measure, propagate that error and mark the generated report invalid instead of
continuing with the truncated snapshot window or leaving status="ok".
In
`@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go`:
- Around line 274-284: Synchronize the pod-creation goroutine in the test around
WaitLoadStarted by adding a completion channel or sync.WaitGroup, and wait for
it before the subtest can return, including when WaitLoadStarted fails. Keep the
existing delayed creation behavior and ensure the fake clientset writes finish
before test cleanup.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go`:
- Around line 378-385: Update Client.WaitLoadStarted to enforce timeout across
all jobs rather than per job: derive a single deadline from the initial timeout,
pass each waitJobPodStarted call only the remaining duration, and return the
existing error when the overall deadline is exhausted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 773a4179-4f0e-4293-a3cc-b175848c6dce
📒 Files selected for processing (9)
src/compute-plane-services/byoo-otel-collector/VERSIONsrc/compute-plane-services/byoo-otel-collector/perf/README.mdsrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.gosrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/render/render.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/report.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/report/report_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- src/compute-plane-services/byoo-otel-collector/VERSION
- src/compute-plane-services/byoo-otel-collector/perf/pkg/report/report.go
- src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main_test.go
- src/compute-plane-services/byoo-otel-collector/perf/README.md
run now measures the collector over the profile's window and emits a baseline (there are no pass/fail thresholds yet). - pkg/report: a small Prometheus text parser plus baseline computation. It matches candidate metric names per concept (suffixes vary across collector-contrib versions), computes per-signal throughput, drops, end-to-end delivery (sink accepted / collector accepted), collector CPU (avg cores) and RSS, and records pod restart/OOM health. Missing series are noted, not fatal. Emits a human summary and JSON. - deploy: ScrapePodMetrics via the API-server proxy (no metrics-server or port-forward needed), PodHealth, and a StartLoad/WaitLoad split so metrics can be sampled while load is in flight. - run: after warmup, snapshots the collector + sink at the start and end of the measurement window, builds the report, prints the summary, and writes <results-dir>/<shape>.json when --results-dir is set. Signed-off-by: shobham <shobham@nvidia.com>
shobham-nv
force-pushed
the
shobham/420-byoo-perf-measure
branch
from
August 11, 2026 13:21
d6ca79e to
4603cd3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/compute-plane-services/byoo-otel-collector/perf/pkg/render/render_test.go (1)
105-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the metrics egress label overlay.
common.BYOOMetricsEgressTargetLabelKeyis a suite override in Lines 105-109. The test only asserts the other two suite labels. Add an assertion for its expected value so a removed or incorrect metrics-egress overlay fails the test.Proposed test addition
if pod.Labels[common.K8sAppNameLabelKey] != common.ByooOTelCollectorPodNameBase { t.Errorf("suite app-name label = %q, want %q", pod.Labels[common.K8sAppNameLabelKey], common.ByooOTelCollectorPodNameBase) } + if pod.Labels[common.BYOOMetricsEgressTargetLabelKey] != common.BYOOMetricsEgressTargetLabelValue { + t.Errorf("suite metrics-egress label = %q, want %q", pod.Labels[common.BYOOMetricsEgressTargetLabelKey], common.BYOOMetricsEgressTargetLabelValue) + }As per coding guidelines, “Code changes must include tests,” and as per path instructions, “Add or update tests for code changes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/render/render_test.go` around lines 105 - 130, The test around the suite label assertions in the relevant render test must also validate the metrics egress overlay. Add an assertion for pod.Labels[common.BYOOMetricsEgressTargetLabelKey] against its expected suite value, alongside the existing app-name and part-of label checks, so an incorrect or missing overlay fails the test.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go`:
- Around line 82-90: Update the fake client setup used by the deployment test to
add a create reactor that converts Secret.StringData entries into Secret.Data
and clears StringData, matching Kubernetes API conversion behavior. In the
credential assertion around accountsSecretsFile, read and unmarshal
secret.Data[accountsSecretsFile] instead of secret.StringData.
---
Nitpick comments:
In
`@src/compute-plane-services/byoo-otel-collector/perf/pkg/render/render_test.go`:
- Around line 105-130: The test around the suite label assertions in the
relevant render test must also validate the metrics egress overlay. Add an
assertion for pod.Labels[common.BYOOMetricsEgressTargetLabelKey] against its
expected suite value, alongside the existing app-name and part-of label checks,
so an incorrect or missing overlay fails the test.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7e090cb9-8636-46c5-b1b5-ef19387799df
📒 Files selected for processing (7)
src/compute-plane-services/byoo-otel-collector/VERSIONsrc/compute-plane-services/byoo-otel-collector/perf/README.mdsrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.gosrc/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/render/render_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- src/compute-plane-services/byoo-otel-collector/perf/README.md
- src/compute-plane-services/byoo-otel-collector/VERSION
- src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main_test.go
- src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go
Comment on lines
+82
to
+90
| // The secret carries a single accounts-secrets.json holding the name->token | ||
| // map; the collector's extractor flattens it into per-signal token files. | ||
| raw, ok := secret.StringData[accountsSecretsFile] | ||
| if !ok { | ||
| t.Fatalf("secret missing %q key: %+v", accountsSecretsFile, secret.StringData) | ||
| } | ||
| var got map[string]string | ||
| if err := json.Unmarshal([]byte(raw), &got); err != nil { | ||
| t.Fatalf("accounts-secrets.json is not valid JSON: %v", err) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Kubernetes v1.34, does corev1.Secret.StringDatapersist in GET responses, or does the API server merge it intocorev1.Secret.Data?
💡 Result:
For Kubernetes v1.34, the corev1.Secret.StringData field does not persist in GET responses, and the API server merges it into the corev1.Secret.Data field at write time [1][2][3]. Key behaviors regarding this field include: - Write-Only Nature: StringData is a convenience field intended for write-only input [1][4]. It allows users to specify non-binary secret data in plaintext strings, which the API server then automatically encodes and merges into the Data field [5][6]. - No Persistence: The StringData field is not stored in etcd and is never returned in responses when reading (GET) from the API [1][2][7]. When you retrieve a Secret via a GET request, you will see the populated Data field with base64-encoded values, but the StringData field will be absent from the output [6][4]. - Merging Logic: During a create or update operation, if both Data and StringData are provided, the values in StringData take precedence and are merged into the Data field, overwriting any existing values for those keys [1][5][6]. Because of this one-way conversion and the lack of persistence, using StringData can be problematic for workflows that rely on observing the original input field, such as certain GitOps diffing or admission control processes [7][4].
Citations:
- 1: https://kubernetes.io/docs/reference/kubernetes-api/core/secret-v1/
- 2: https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/secret-v1/
- 3: https://arnavion.github.io/k8s-openapi/v0.26.x/src/k8s_openapi/v1_34/api/core/v1/secret.rs.html
- 4: https://hajnalmt.hu/posts/kubernetes-secret-handling-interesting-things/
- 5: https://kubernetes.io/docs/concepts/configuration/secret/
- 6: https://kubernetes.io/docs/tasks/configmap-secret/managing-secret-using-config-file/
- 7: Data lost when data is applied as data while not lost if data is applied as stringData in secret kubernetes/kubernetes#123843
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- guidance files ---'
find .. -name AGENTS.md -print
printf '%s\n' '--- target file outline ---'
ast-grep outline src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go
printf '%s\n' '--- target lines ---'
sed -n '1,150p' src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go
printf '%s\n' '--- Secret construction and fake-client setup ---'
rg -n -C 4 'StringData|accountsSecretsFile|fake\.NewSimpleClientset|NewClient|Secret' src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.goRepository: NVIDIA/nvcf
Length of output: 15111
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository guidance ---'
cat -n AGENTS.md
printf '%s\n' '--- byoo-otel-collector guidance ---'
cat -n src/compute-plane-services/byoo-otel-collector/AGENTS.md
printf '%s\n' '--- relevant deploy symbols ---'
rg -n -C 8 'WithExportCredentials|accountsSecretsFile|StringData|Secret\{' src/compute-plane-services/byoo-otel-collector/perf/pkg
printf '%s\n' '--- module and cached fake-client sources ---'
rg -n 'k8s.io/client-go|k8s.io/api' go.mod src/compute-plane-services/byoo-otel-collector/go.mod 2>/dev/null || true
fd -i 'fake' ../go/pkg/mod/k8s.io/client-go* 2>/dev/null | head -20 || trueRepository: NVIDIA/nvcf
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat >"$tmpdir/go.mod" <<'EOF'
module secret-field-probe
go 1.23
require (
k8s.io/api v0.34.2
k8s.io/client-go v0.34.2
)
EOF
cat >"$tmpdir/main.go" <<'EOF'
package main
import (
"context"
"fmt"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
)
func main() {
cs := fake.NewSimpleClientset()
in := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "credentials", Namespace: "test"},
StringData: map[string]string{"accounts-secrets.json": `{"perf-logs":"tok-a"}`},
}
if _, err := cs.CoreV1().Secrets("test").Create(context.Background(), in, metav1.CreateOptions{}); err != nil {
panic(err)
}
out, err := cs.CoreV1().Secrets("test").Get(context.Background(), "credentials", metav1.GetOptions{})
if err != nil {
panic(err)
}
fmt.Printf("StringData key present: %t\n", out.StringData["accounts-secrets.json"] != "")
fmt.Printf("Data key present: %t\n", len(out.Data["accounts-secrets.json"]) != 0)
}
EOF
(
cd "$tmpdir"
GOTOOLCHAIN=local GOPROXY=off go run .
)Repository: NVIDIA/nvcf
Length of output: 575
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- fake client create/get implementation ---'
rg -n -C 8 'func \(.*\) Create|func \(.*\) Get|ObjectTracker|Add\(obj' ../go/pkg/mod/k8s.io/client-go@v0.34.2/testing ../go/pkg/mod/k8s.io/client-go@v0.34.2/kubernetes/fake | head -240
printf '%s\n' '--- Secret StringData/Data definitions ---'
rg -n -C 6 'type Secret struct|StringData|Data map' ../go/pkg/mod/k8s.io/api@v0.34.2/core/v1/types.go ../go/pkg/mod/k8s.io/api@v0.34.2/core/v1/generated.proto 2>/dev/null | head -180
printf '%s\n' '--- cached module checksums ---'
find ../go/pkg/mod/cache/download/k8s.io -path '*api*0.34.2*' -o -path '*client-go*0.34.2*' | head -80Repository: NVIDIA/nvcf
Length of output: 28961
Model Secret conversion in the fake client before asserting credentials.
fake.NewSimpleClientset stores objects as-is, so this GET returns the StringData supplied by applyCredentialsSecret. Add a create reactor that merges StringData into Data and clears StringData, then assert secret.Data[accountsSecretsFile].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy_loadgen_test.go`
around lines 82 - 90, Update the fake client setup used by the deployment test
to add a create reactor that converts Secret.StringData entries into Secret.Data
and clears StringData, matching Kubernetes API conversion behavior. In the
credential assertion around accountsSecretsFile, read and unmarshal
secret.Data[accountsSecretsFile] instead of secret.StringData.
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
2 participants
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.
Summary
Adds the measurement + reporting milestone to the BYOO collector performance suite.
perf runnow drivestelemetrygenload against the authentic collector (rendered through the sharedicms-translatelibrary), scrapes the collector and in-cluster OTLP sink via the API-server proxy, and computes a reproducible baseline (throughput, drops, end-to-end delivery, CPU/memory, pod health).dev= 1,baseline= 3); each run is persisted separately (<shape>.json, or<shape>-run<N>.jsonfor several).status:ok,partial(a metric series or pod health was missing), orinvalid(load generators did not complete cleanly), so a failed run is never mistaken for a clean baseline.runwaits for the load-generator pods to start before the warmup so pod scheduling / image-pull latency stays out of the measurement window.Testing
GOWORK=off go build/vet/test ./...for theperfmodule: all green.cd src/compute-plane-services/byoo-otel-collector/perf GOWORK=off go run ./cmd/perf run --shape container --import-images --results-dir ./resultsResult: PASS (status
ok) — collector accepted the generated telemetry and delivered ~100% to the in-cluster sink, with no restarts and no OOM kills over a ~30s window (containershape,devprofile).okRaw baseline (perf/results/container.json)
{ "shape": "container", "profile": "dev", "run": 1, "repetitions": 1, "status": "ok", "window_seconds": 30.010872708, "logs": { "generated_expected": 30010.872708000003, "collector_accepted": 26083, "collector_refused": 0, "exporter_sent": 26124, "exporter_failed": 0, "sink_accepted": 26124, "throughput_per_sec": 870.4845158680148, "delivery_ratio": 1.0015719050722693 }, "metrics": { "generated_expected": 30010.872708000003, "collector_accepted": 26170, "collector_refused": 0, "exporter_sent": 26185, "exporter_failed": 0, "sink_accepted": 26185, "throughput_per_sec": 872.5171125403448, "delivery_ratio": 1.0005731753916698 }, "resources": { "cpu_cores_avg": 0.13628394081688028, "mem_rss_bytes": 123772928 }, "health": { "phase": "Running", "restarts": 0, "oom_killed": false } }Summary by CodeRabbit
New Features
Documentation
Chores