feat(byoo-otel-collector): add debug and log chunking flags#194
Open
kristinapathak wants to merge 4 commits into
Open
feat(byoo-otel-collector): add debug and log chunking flags#194kristinapathak wants to merge 4 commits into
kristinapathak wants to merge 4 commits into
Conversation
Contributor
Author
|
CI note after migration:
|
8 tasks
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
from
July 21, 2026 20:53
a4cb944 to
a11fd25
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBYOO configuration now supports explicit log chunking, debug mode, SRE metrics, and OpenTelemetry Collector overrides. NVCA aggregates and injects these settings into collector containers, while rendering resolves chunking defaults, applies overrides, and configures debug pipelines. ChangesBYOO observability configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentConfig
participant NVCA
participant BYOOCollector
AgentConfig->>NVCA: BYOOOTelCollectorEnvVars()
NVCA->>BYOOCollector: inject chunking, debug, SRE metrics, and collector override variables
BYOOCollector->>BYOOCollector: render collector configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
kristinapathak
changed the title
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
from
July 21, 2026 20:57
a11fd25 to
810f6bb
Compare
4 tasks
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
from
July 21, 2026 21:24
810f6bb to
f78d458
Compare
|
🌿 Preview your docs: https://nvidia-preview-ci-byoo-otel-config-knobs.docs.buildwithfern.com/nvcf |
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
2 times, most recently
from
July 21, 2026 21:38
9e1019a to
01fb8b6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/compute-plane-services/byoo-otel-collector/internal/otelconfig/render.go (1)
615-670: 📐 Maintainability & Code Quality | 🔵 TrivialDebug-exporter wiring duplicated with
collector_config.go.
applyDebugMode's block that ensures a"debug"exporter and appends it to every pipeline (lines 627-636) is duplicated almost verbatim inapplyDebugExporterConfigincollector_config.go(lines 410-419). Extract a shared helper (e.g.ensureDebugExporter(otelConfig)) to avoid drift between the two debug-enabling paths (TemplateConfig.DebugModevsOTelCollectorConfig.DebugExporter.Enabled).See consolidated comment.
🤖 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/internal/otelconfig/render.go` around lines 615 - 670, The debug exporter wiring is duplicated between applyDebugMode and applyDebugExporterConfig, risking inconsistent behavior. Extract the shared exporter initialization and pipeline-appending logic into an ensureDebugExporter helper, then call it from both debug-enabling paths while keeping their existing log-level and configuration responsibilities unchanged.src/compute-plane-services/byoo-otel-collector/internal/otelconfig/collector_config.go (2)
392-420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
applyDebugExporterConfigduplicatesapplyDebugMode's debug-exporter logic.
See consolidated comment.🤖 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/internal/otelconfig/collector_config.go` around lines 392 - 420, The debug exporter setup in applyDebugExporterConfig duplicates the logic already handled by applyDebugMode. Consolidate the behavior by reusing applyDebugMode and remove the duplicated exporter initialization and pipeline-update logic from applyDebugExporterConfig, preserving the existing enabled guard.
325-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
applyBatchConfigandapplyLogBatchConfigduplicate the same field-setting logic.Both functions set
timeout,send_batch_size,send_batch_max_size,metadata_keys, andmetadata_cardinality_limiton a processor map with identical logic (lines 330-344 vs 358-372). Consider extracting a shared(c BatchConfig) applyTo(out map[string]interface{})helper (mirroring the pattern already used forRetryOnFailureConfig/SendingQueueConfig/SendingQueueBatchConfig), and haveapplyLogBatchConfigcall it after resolving/cloning the base processor map. This avoids the two copies drifting if a newBatchConfigfield is added later.♻️ Suggested consolidation
+func (c BatchConfig) applyTo(out map[string]interface{}) { + if c.Timeout != "" { + out["timeout"] = c.Timeout + } + if c.SendBatchSize != nil { + out["send_batch_size"] = *c.SendBatchSize + } + if c.SendBatchMaxSize != nil { + out["send_batch_max_size"] = *c.SendBatchMaxSize + } + if len(c.MetadataKeys) > 0 { + out["metadata_keys"] = c.MetadataKeys + } + if c.MetadataCardinalityLimit != nil { + out["metadata_cardinality_limit"] = *c.MetadataCardinalityLimit + } +} + func applyBatchConfig(otelConfig *OpenTelemetryConfig, processorID string, cfg BatchConfig) { if cfg.IsZero() { return } processor := mapFromInterface(otelConfig.Processors[processorID]) - if cfg.Timeout != "" { - processor["timeout"] = cfg.Timeout - } - if cfg.SendBatchSize != nil { - processor["send_batch_size"] = *cfg.SendBatchSize - } - if cfg.SendBatchMaxSize != nil { - processor["send_batch_max_size"] = *cfg.SendBatchMaxSize - } - if len(cfg.MetadataKeys) > 0 { - processor["metadata_keys"] = cfg.MetadataKeys - } - if cfg.MetadataCardinalityLimit != nil { - processor["metadata_cardinality_limit"] = *cfg.MetadataCardinalityLimit - } + cfg.applyTo(processor) otelConfig.Processors[processorID] = processor }🤖 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/internal/otelconfig/collector_config.go` around lines 325 - 390, Extract the duplicated BatchConfig field assignments from applyBatchConfig and applyLogBatchConfig into a BatchConfig.applyTo(out map[string]interface{}) helper, following the existing configuration helper pattern. Have both functions resolve their processor map as they currently do, then call applyTo and preserve their existing processor and pipeline updates.
🤖 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.
Nitpick comments:
In
`@src/compute-plane-services/byoo-otel-collector/internal/otelconfig/collector_config.go`:
- Around line 392-420: The debug exporter setup in applyDebugExporterConfig
duplicates the logic already handled by applyDebugMode. Consolidate the behavior
by reusing applyDebugMode and remove the duplicated exporter initialization and
pipeline-update logic from applyDebugExporterConfig, preserving the existing
enabled guard.
- Around line 325-390: Extract the duplicated BatchConfig field assignments from
applyBatchConfig and applyLogBatchConfig into a BatchConfig.applyTo(out
map[string]interface{}) helper, following the existing configuration helper
pattern. Have both functions resolve their processor map as they currently do,
then call applyTo and preserve their existing processor and pipeline updates.
In
`@src/compute-plane-services/byoo-otel-collector/internal/otelconfig/render.go`:
- Around line 615-670: The debug exporter wiring is duplicated between
applyDebugMode and applyDebugExporterConfig, risking inconsistent behavior.
Extract the shared exporter initialization and pipeline-appending logic into an
ensureDebugExporter helper, then call it from both debug-enabling paths while
keeping their existing log-level and configuration responsibilities unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d2cf1582-1005-4361-b387-e7e624b46fe5
⛔ Files ignored due to path filters (3)
src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/types.gois excluded by!**/vendor/**src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/zz_generated.deepcopy.gois excluded by!**/vendor/**,!**/zz_generated.*src/libraries/go/lib/pkg/types/nvca/config/zz_generated.deepcopy.gois excluded by!**/zz_generated.*
📒 Files selected for processing (18)
deploy/helm/nvca-operator/nvca-operator/values.yamlsrc/compute-plane-services/byoo-otel-collector/README.mdsrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/BUILD.bazelsrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/collector_config.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/embed_config.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig_test.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/render.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/render_test.gosrc/compute-plane-services/nvca/deployments/nvca-operator/values.yamlsrc/compute-plane-services/nvca/internal/miniservice/reconcile_test.gosrc/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars_test.gosrc/compute-plane-services/nvca/pkg/nvca/cli_test.gosrc/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.gosrc/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.gosrc/libraries/go/lib/pkg/types/nvca/config/config_test.gosrc/libraries/go/lib/pkg/types/nvca/config/types.gosrc/libraries/go/lib/pkg/types/nvca/config/types_test.go
🚧 Files skipped from review as they are similar to previous changes (13)
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/embed_config.go
- src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go
- src/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars_test.go
- src/libraries/go/lib/pkg/types/nvca/config/types_test.go
- src/libraries/go/lib/pkg/types/nvca/config/config_test.go
- src/compute-plane-services/byoo-otel-collector/README.md
- src/compute-plane-services/nvca/pkg/nvca/cli_test.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig_test.go
- deploy/helm/nvca-operator/nvca-operator/values.yaml
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig.go
- src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go
- src/compute-plane-services/nvca/internal/miniservice/reconcile_test.go
- src/libraries/go/lib/pkg/types/nvca/config/types.go
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
from
July 21, 2026 21:46
01fb8b6 to
038d7b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go`:
- Around line 2840-2844: Update the test containing the BYOOLogChunking
configuration to assert that the reconciled result’s
got.Agent.BYOOLogChunking.Enabled is true, alongside the existing merged-field
assertions. Keep the test setup unchanged and add only the verification needed
to catch loss of this feature flag.
🪄 Autofix (Beta)
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: 7aea0267-61a0-4ed4-adc9-ac521b0d0e06
⛔ Files ignored due to path filters (3)
src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/types.gois excluded by!**/vendor/**src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config/zz_generated.deepcopy.gois excluded by!**/vendor/**,!**/zz_generated.*src/libraries/go/lib/pkg/types/nvca/config/zz_generated.deepcopy.gois excluded by!**/zz_generated.*
📒 Files selected for processing (27)
deploy/helm/nvca-operator/nvca-operator/values.yamldocs/ngc-managed/cluster-management/configuration.mddocs/user/cluster-management/configuration.mdsrc/compute-plane-services/byoo-otel-collector/README.mdsrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/BUILD.bazelsrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/collector_config.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/embed_config.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig_test.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/render.gosrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/render_test.gosrc/compute-plane-services/nvca/deployments/nvca-operator/values.yamlsrc/compute-plane-services/nvca/internal/miniservice/reconcile.gosrc/compute-plane-services/nvca/internal/miniservice/reconcile_test.gosrc/compute-plane-services/nvca/internal/util/k8sutil/BUILD.bazelsrc/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars.gosrc/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars_test.gosrc/compute-plane-services/nvca/pkg/nvca/cli_test.gosrc/compute-plane-services/nvca/pkg/nvca/k8scomputebackend.gosrc/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_task_container.gosrc/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.gosrc/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.gosrc/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.gosrc/libraries/go/lib/pkg/types/nvca/config/config_test.gosrc/libraries/go/lib/pkg/types/nvca/config/deepcopy_test.gosrc/libraries/go/lib/pkg/types/nvca/config/types.gosrc/libraries/go/lib/pkg/types/nvca/config/types_test.go
🚧 Files skipped from review as they are similar to previous changes (23)
- src/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars.go
- docs/user/cluster-management/configuration.md
- src/compute-plane-services/nvca/internal/util/k8sutil/BUILD.bazel
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/BUILD.bazel
- src/libraries/go/lib/pkg/types/nvca/config/deepcopy_test.go
- src/compute-plane-services/byoo-otel-collector/README.md
- src/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_task_container.go
- docs/ngc-managed/cluster-management/configuration.md
- src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook.go
- deploy/helm/nvca-operator/nvca-operator/values.yaml
- src/compute-plane-services/nvca/internal/util/k8sutil/byoo_env_vars_test.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/embed_config.go
- src/compute-plane-services/nvca/pkg/nvca/cli_test.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig_test.go
- src/libraries/go/lib/pkg/types/nvca/config/types_test.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/collector_config.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/render_test.go
- src/compute-plane-services/nvca/pkg/webhook/miniservice_mutating_webhook_test.go
- src/compute-plane-services/byoo-otel-collector/internal/otelconfig/render.go
- src/compute-plane-services/nvca/internal/miniservice/reconcile_test.go
- src/libraries/go/lib/pkg/types/nvca/config/config_test.go
- src/libraries/go/lib/pkg/types/nvca/config/types.go
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
3 times, most recently
from
July 21, 2026 22:33
557f484 to
d16d4f7
Compare
Add NVCA agent config for BYOO SRE metrics collector settings and pass the resulting environment variables through the existing BYOO OTel collector-only injection path. This lets Helm users configure the SRE metrics pipeline through agentConfig.mergeConfig without adding dedicated chart values. Document the mergeConfig shape and extend focused tests for config decoding, env rendering, operator config merging, and collector-only pod mutation. Closes #325 Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
kristinapathak
force-pushed
the
ci/byoo-otel-config-knobs
branch
from
July 21, 2026 22:35
d16d4f7 to
13b89a4
Compare
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
1 participant
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
Add BYOO OTel collector feature flags for common debug and log chunking cases, while preserving advanced collector overrides for settings such as exporter timeout.
Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
agent.byooLogChunking.enabled, which injectsBYOO_LOG_CHUNKING_ENABLED=true. The collector enableslogchunk/byooand defaultsmax_body_bytesto262144anddry_runtofalsewhen those fields are unset.byooLogChunking.enableddoes not enable exporterhelper byte batching by itself. Byte batching is configured through advancedagent.byooOtelCollector.exporterHelper.sendingQueue.batchoverrides.agent.byooOtelCollectoradvanced rendering overrides, encoded intoBYOO_OTEL_COLLECTOR_CONFIG_B64, for exporter helper, memory limiter, batch, and log batch settings.agent.byooDebugMode.enabled, which injectsBYOO_DEBUG_MODE=trueinto the BYOO OTel collector and renders collector telemetry logs at debug/development mode plus debug exporter fanout.agentConfig.mergeConfigpath and only adds commented chart examples for the narrow flags and exporter timeout override.main, then applies this PR's debug/log chunking changes.For the Reviewer
Start with
src/libraries/go/lib/pkg/types/nvca/config/types.gofor the NVCA config surface, thensrc/compute-plane-services/byoo-otel-collector/internal/otelconfig/otelconfig.go,collector_config.go, andrender.gofor collector-side defaulting and advanced override application.For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
git diff --check origin/main..HEADGOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path ./tools/ci/check-go-codegen src/libraries/go/lib --install k8s.io/code-generator/cmd/deepcopy-gen@v0.34.2 --command 'make codegen-update'GOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test ./pkg/types/nvca/configGOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test ./internal/otelconfigGOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test -vet=off ./internal/util/k8sutil -run TestAddBYOOEnvVarsToPodSpecMutatesOnlyBYOOCollectorContainer -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0'GOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test -vet=off ./pkg/webhook -run TestMiniserviceMutatePodSpec_BYOOOTelCollectorEnvVarsOnlyCollector -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0'GOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test -vet=off ./pkg/operator/reconcile -run TestEncodeAgentConfig_MergesBYOOConfig -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0'GOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test -vet=off ./pkg/nvca -run 'TestCLI|TestNewCobraCommand|Test.*Config' -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0'GOCACHE=/private/tmp/nvcf-pr194-go-cache GOPATH=/private/tmp/nvcf-pr194-go-path go test -vet=off ./internal/miniservice -run TestReconcile_Function -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0'Issues
NO-REF
Checklist