feat(byoo-perf): add OTLP sink and telemetrygen load generation - #624
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe BYOO OTEL performance suite now deploys an in-cluster OTLP sink, generates telemetry with ChangesBYOO performance suite
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant perf as run command
participant k3d as k3d package
participant deploy as deploy package
participant sink as OTLP sink
participant loadgen as telemetrygen Jobs
perf->>k3d: create or reuse cluster
perf->>deploy: deploy sink
deploy->>sink: create sink resources
perf->>deploy: deploy collector with credentials
perf->>loadgen: run profile load
perf->>deploy: clean up labeled resources
Possibly related PRs
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
approved these changes
Aug 5, 2026
shobham-nv
force-pushed
the
shobham/418-byoo-perf-deploy
branch
3 times, most recently
from
August 10, 2026 14:59
435be42 to
a5a2164
Compare
Add the in-cluster OTLP sink (pkg/sink) and telemetrygen load generator (pkg/loadgen), and wire `perf run` end-to-end: deploy the sink, deploy the authentic collector with its export redirected at the sink (backed by an export-credentials Secret over the collector secrets volume), drive load at the profile's rates, then clean up. - pkg/sink: stock collector-contrib sink accepting OTLP over gRPC/HTTP, discarding via the debug exporter, exposing receiver counters on a Prometheus endpoint, and gating readiness with health_check. - pkg/loadgen: single-shot telemetrygen Jobs (backoffLimit 0) per enabled signal targeting the collector receiver. - pkg/labels: shared labels so cleanup is scoped to suite-created objects. - pkg/deploy: DeploySink, RunLoad, export-credentials Secret + secrets-volume override, and Cleanup extended to jobs/configmaps/secrets. - Unit tests for all new packages and the run flag surface; make perf-test passes. Measurement and reporting land in the next milestone; the sink already exposes its counters for it to read. Signed-off-by: shobham <shobham@nvidia.com>
--mode k3d (the default) now provisions a dedicated local k3d cluster, runs the suite against it, and deletes it afterwards (unless --retain, which keeps the resources and the cluster). --mode remote keeps the previous behavior of using the ambient kubeconfig/context. - pkg/k3d: thin, unit-testable wrapper over the k3d CLI (create/delete/list/ image-import) with an injectable command runner. Reuses an existing cluster of the same name so reruns are cheap. - run: provisions/tears down the cluster around the shape loop and points the deploy client at the k3d-<name> context. New flags --k3d-cluster and --import-images (load collector/sink/loadgen images from local Docker for non-pullable/local builds). Signed-off-by: shobham <shobham@nvidia.com>
shobham-nv
force-pushed
the
shobham/419-byoo-perf-loadgen-sink
branch
from
August 10, 2026 15:51
89ae4fa to
833b809
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: 4
🧹 Nitpick comments (3)
src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go (1)
349-364: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare one deadline across all Job waits.
RunLoadapplies every Job at the same time, so the Jobs run concurrently. The wait loop then gives each Job its own freshtimeout. With two signals the worst-case wall time is twice the caller's budget. A single deadline matches the concurrent execution model.♻️ Proposed refactor to use one deadline
func (c *Client) RunLoad(ctx context.Context, namespace string, jobs []*batchv1.Job, timeout time.Duration) error { for _, j := range jobs { if err := c.applyJob(ctx, namespace, j); err != nil { return err } } + deadline := time.Now().Add(timeout) for _, j := range jobs { - if err := c.waitJobComplete(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", j.Name) + } + if err := c.waitJobComplete(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 349 - 364, Update RunLoad to create one shared deadline from the caller’s timeout after applying all Jobs, then pass the remaining time to each waitJobComplete call instead of giving each Job a fresh timeout. Preserve the existing early-return behavior while ensuring the total waiting period stays within the original timeout budget.src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go (1)
201-214: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd signal handling so teardown runs on interrupt.
ctxcomes fromcontext.Background(). If the user presses Ctrl-C during a run, the process exits beforedefer teardown()runs. The managed k3d cluster and the deployed namespace then stay behind.signal.NotifyContextlets the in-flight waits cancel and the deferred teardown run.♻️ Proposed refactor
- ctx := context.Background() + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop()🤖 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 201 - 214, Update the run flow around ctx and ensureK3dCluster to derive the context with signal.NotifyContext for interrupt and termination signals, and defer its cancellation. Pass this signal-aware context through cluster provisioning and the subsequent in-flight waits so interruption cancels them and allows the existing deferred teardown to execute.src/compute-plane-services/byoo-otel-collector/perf/pkg/k3d/k3d.go (1)
150-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse Go’s built-in
maxand removemaxInt. Theperfmodule targets Go 1.25.0, somaxis available.🤖 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/k3d/k3d.go` around lines 150 - 155, Replace all uses of maxInt with Go’s built-in max function, then remove the maxInt helper entirely. Keep the existing maximum-value behavior unchanged.
🤖 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 304-314: Update the render-and-validate flow around render.Render
and validate.Render to call the existing cleanupAfterErr path before returning
either error. Preserve the current wrapped render error and validation error
while ensuring the deployed sink resources are removed on both failure paths.
- Around line 239-273: Track cluster ownership across
src/compute-plane-services/byoo-otel-collector/perf/pkg/k3d/k3d.go:103-119 by
adding a Reused indicator to Cluster and setting it from the existing Exists
result. In ensureK3dCluster at
src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go:239-273,
report when the cluster is reused and skip both image-import-failure deletion
and teardown deletion for reused clusters; retain deletion only for clusters
created by this run. Add a unit test covering reused-cluster teardown behavior.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/sink/sink.go`:
- Line 141: Update the sink Pod configuration to use corev1.RestartPolicyAlways
instead of RestartPolicyNever so it remains recoverable after collector
termination. In TestPodExposesReceiverAndMetricsPorts, assert the selected
restart policy.
In `@src/compute-plane-services/byoo-otel-collector/perf/README.md`:
- Around line 109-115: Update the changed documentation in the README to remove
Markdown bold emphasis around k3d and replace em dashes in the affected lines
near the mode and retain descriptions with standard punctuation, while
preserving the existing meaning and concise prose.
---
Nitpick comments:
In `@src/compute-plane-services/byoo-otel-collector/perf/cmd/perf/main.go`:
- Around line 201-214: Update the run flow around ctx and ensureK3dCluster to
derive the context with signal.NotifyContext for interrupt and termination
signals, and defer its cancellation. Pass this signal-aware context through
cluster provisioning and the subsequent in-flight waits so interruption cancels
them and allows the existing deferred teardown to execute.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/deploy/deploy.go`:
- Around line 349-364: Update RunLoad to create one shared deadline from the
caller’s timeout after applying all Jobs, then pass the remaining time to each
waitJobComplete call instead of giving each Job a fresh timeout. Preserve the
existing early-return behavior while ensuring the total waiting period stays
within the original timeout budget.
In `@src/compute-plane-services/byoo-otel-collector/perf/pkg/k3d/k3d.go`:
- Around line 150-155: Replace all uses of maxInt with Go’s built-in max
function, then remove the maxInt helper entirely. Keep the existing
maximum-value behavior unchanged.
🪄 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: a62a03c4-99b1-41eb-8d36-b65c0342f775
📒 Files selected for processing (13)
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/k3d/k3d.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/k3d/k3d_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/labels/labels.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/loadgen/loadgen.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/loadgen/loadgen_test.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/sink/sink.gosrc/compute-plane-services/byoo-otel-collector/perf/pkg/sink/sink_test.go
- sink: run the OTLP sink Pod with RestartPolicyAlways so the kubelet restarts it if its container dies, keeping it in the Service for the whole load run. - run: clean up the deployed sink when collector render or validation fails, matching the other failure paths so nothing leaks in remote mode. - k3d: track cluster ownership via Cluster.Reused and never delete a pre-existing cluster of the same name on teardown or image-import failure; add a unit test covering the reused-cluster teardown path. - docs: drop bold emphasis and em dashes from the changed k3d README lines. Signed-off-by: shobham <shobham@nvidia.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 10, 2026
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.
TL;DR
Adds load generation and an in-cluster OTLP sink to the BYOO collector performance suite, and wires
perf runend-to-end.runnow deploys a sink, deploys the authentic collector with its export redirected at that sink, and drivestelemetrygenload at the selected profile's rates before cleaning up. Stacked on #623.Additional Details
The suite must drive real telemetry through the collector and let it drain, otherwise the collector backs up against the unreachable placeholder export endpoints and the numbers are meaningless. This PR adds the two building blocks and the wiring:
pkg/sink— a stockcollector-contribsink that accepts OTLP over gRPC (4317) and HTTP (4318), discards payloads via thedebugexporter, exposes itsotelcol_receiver_accepted_*counters on a Prometheus telemetry endpoint (8888), and gates readiness withhealth_check(13133). This is the destination the collector under test exports to.pkg/loadgen— single-shottelemetrygenJobs (one per enabled signal,backoffLimit: 0so a retry never replays load) that send OTLP into the collector receiver at the profile's rates forwarmup + window.pkg/labels— shared labels sodeploy,sink, andloadgentag objects identically and cleanup stays scoped.pkg/deploy—DeploySink,RunLoad(create + wait for completion), an export-credentialsSecretthat backs the collector's/etc/byoo-otel-collector/secretsvolume (so the generated${file:...}exporter config resolves and the collector can actually start), andCleanupextended to also remove jobs, config maps, and secrets.runredirects export viaProvider=OTEL_COLLECTOR+ endpoints pointed at the sink; new flags--sink-image,--loadgen-image,--skip-load.Limitations / to validate on a cluster
For the Reviewer
pkg/deploy/deploy.go(credentials Secret +mountSecretOverPath,RunLoad, broadenedCleanup) andcmd/perf/main.gorunShape.For QA
make perf-test(build + vet + unit tests, cluster-free) passes.perf run --shape container) should be exercised against a target cluster.Issues
NO-REF
Checklist
Summary by CodeRabbit
New Features
Documentation
Chores