fix(worker): populate metering nspect_id from NVCF_NSPECT_ID env#285
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 (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughInitializer and worker metering configurations now populate ChangesNspect ID metering propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 main module or its selected dependencies" 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: 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/worker-init/internal/workerinit/initializer.go`:
- Line 146: The constructors lack coverage for propagating their newly added
values. Add a test for NewInitializer in
src/compute-plane-services/worker-init/internal/workerinit/initializer.go:146-146
that verifies NspectId propagation, and a test for NewNVCTWorker in
src/compute-plane-services/worker-task/internal/worker/worker.go:128-128 that
verifies task propagation; otherwise document the omission in the PR
description.
🪄 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: 4dafe444-1f35-4b91-8a9c-632c94a9f974
📒 Files selected for processing (4)
src/compute-plane-services/worker-init/internal/workerinit/initializer.gosrc/compute-plane-services/worker-task/internal/worker/worker.gosrc/compute-plane-services/worker-utils/worker/newworker_test.gosrc/compute-plane-services/worker-utils/worker/worker.go
Metering events (NVCF_Invocation and NVCF_Infrastructure) were emitted with an empty data.nspect_id, causing UAM ingestion to reject them for failing schema validation. During the OSS split nspect_id moved to the NVCF_NSPECT_ID env var, but the worker only consumed it for the NVCF-NSPECTID inference header and never populated metering.Config.NspectId. Wire metering.NspectIdFromEnv() into the metering.Config built by the function worker, worker-init initializer, and task worker so metering events carry the nspect_id again. Signed-off-by: shobham <shobham@nvidia.com>
722d784 to
7b873bb
Compare
|
🎉 This PR is included in version nvcf-worker-init-v1.0.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version nvcf-worker-utils-v1.0.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version nvcf-worker-task-v1.0.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
Summary
NVCF worker metering events (
NVCF_InvocationandNVCF_Infrastructure) were being emitted with an emptydata.nspect_id, causing the downstream metering ingestion pipeline to reject them for failing mandatory-field schema validation.During the open-source split,
nspect_idwas moved to theNVCF_NSPECT_IDenv var, but the worker only consumed it for theNVCF-NSPECTIDinference header and never populatedmetering.Config.NspectId(serialized inmetering.goandinfra.go). This wiresmetering.NspectIdFromEnv()into the metering config so the events carry thenspect_idagain.Changes
NspectId: metering.NspectIdFromEnv()in the threemetering.Configconstructors:src/compute-plane-services/worker-utils/worker/worker.go(function worker — emits both affected event types)src/compute-plane-services/worker-init/internal/workerinit/initializer.go(init-phase infra events)src/compute-plane-services/worker-task/internal/worker/worker.go(task infra events)NVCF_NSPECT_IDflows intometeringConfig.NspectId.Testing
gofmtclean.TestNewNVCFWorker_Normalization/"nspect id is populated from environment for metering events".nspect_idserialization fromConfig.NspectId.Issues
NO-REF
Summary by CodeRabbit
New Features
Tests