[tonytran-dd]

What

Nested NetworkPath message on Connection at field 65 (next identifier bumped 65 → 66).

message NetworkPath {
  bool testEligible = 1;     // policy approved a dynamic test
  string correlationKey = 2; // identity of that test
}

Why nested

Generated as a pointer, so three states survive the wire:

State	Meaning
nil	never evaluated (Network Path off, or older Agent)
set, empty	evaluated, no test
set, populated	evaluated, test scheduled

A flat bool collapses the first two.

Order

logs-backend → dd-go CNM → dd-go NPP → datadog-agent → web-ui

[Copilot]

Copilot review overview

🔵 Needs a closer look

The PR remains a draft pending human validation of the implementation plan and coordinated downstream rollout.

Review effort: Balanced
Findings: None

What changed in this PR

Adds Network Path metadata to CNM connections, enabling downstream consumers to distinguish evaluation states and correlate flows with Network Path events.

Changes:

• Adds field 65 with test eligibility and a correlation key.
• Preserves absent versus present-empty metadata.
• Regenerates Go message bindings and streaming builders.

File	Description
proto/​process/​connections.proto	Defines Network Path metadata and advances the field marker.
process/​connections.proto_builder.go	Adds builders for the new metadata.
process/​connections.pb.go	Adds generated types, descriptors, and serialization support.

Files not reviewed (1)

• process/connections.pb.go: Generated file

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The PR explicitly remains a not-for-merge draft pending human sign-off on the cross-service contract and coordinated rollout.

Review effort: Balanced
Findings: None

Files not reviewed (1)

• process/connections.pb.go: Generated file

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T14:53:10.531951Z	c0196dc	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[tonytran-dd]

@DataDog review

[tonytran-dd]

@DataDog review

[datadog-prod-us1-5]

More details

The generated message and builder encoders preserve absent, empty, and populated Network Path metadata. Field 65 is unused in the base schema, and the completed static reviewer reported no actionable regression.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Bits Code Review · Commit b12df20}

[AlexandreYang]

Review Summary

Reviewed the new NetworkPath message on Connection (field 65), the regenerated gogo code in connections.pb.go, and the new NetworkPathBuilder / ConnectionBuilder.SetNetworkPath.

Assessment: safe to merge. Nothing blocks this.

• Compatibility: the change only adds a field. 65 is unused and not reserved. Old receivers skip the field and old senders leave it nil.
• Builder tag 0x20a = (65<<3)|2 is correct, and SetNetworkPath follows the same pattern as SetLaddr and the other nested setters.
• invoke codegen CI passes, so the generated code matches the proto.
• I checked locally with a throwaway test that the three states survive builder → gogo Unmarshal → Marshal → Unmarshal. Not set gives nil. Set but empty gives a non-nil empty message. Populated gives testEligible:true correlationKey:"abc".
• The earlier nit about the next identifier comment has been fixed.

#	Priority	File	Finding
1		process/connections.proto_builder.go:1366	Optional: no test locks in the three-state presence contract

Test Coverage

Code path	Test	Status
NetworkPath gogo Marshal/Unmarshal	— (generated; invoke codegen CI)	Partial
SetNetworkPath nil / empty / populated presence	—	Missing (P3, verified manually)

Overall: Gaps found, but none block the merge. The repo has no per-field builder tests, so the generated code relies on codegen CI.

Positive Observations

• Making it a nested message so nil and "evaluated, no test" stay distinct is a good choice, and the PR description explains it clearly.
• The field comments spell out what testEligible does not mean and when correlationKey can be empty, which will save downstream consumers from guessing.

[AlexandreYang]

Optional: add a round-trip test for the three-state presence contract

The main point of this PR is that nil, set-but-empty, and populated stay distinct on the wire. Nothing tests that. I checked by hand and it works today. The risk is a later change, such as a generator option like nullable=false or making the field flat, which would silently merge nil and empty while CI stays green.

Suggested test in process/connections_test.go: build a Connection with ConnectionBuilder in three cases and Unmarshal it:

• SetNetworkPath not called → NetworkPath == nil
• SetNetworkPath(func(*NetworkPathBuilder){}) → NetworkPath != nil, TestEligible == false, CorrelationKey == ""
• SetTestEligible(true) and SetCorrelationKey("k") → both values come back

This is optional and fine to skip, since the repo doesn't test builders field by field.