go: add REMOTE_CONFIG DebugService ping handler for example client - #247
Conversation
Adds a handler to allow us to handle the ping/pong magic tunnel dispatch messages as an example of how to do this, and to also help enable our e2e PoC.
There was a problem hiding this comment.
🟡 Changes recommended
The new handler logs untrusted input without quoting (log forging risk) and leaves the correlation ID unused, which should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an example REMOTE_CONFIG DebugService ping handler to the testx509 Go example client so it can participate in magic-tunnel ping/pong dispatches (useful for the e2e PoC).
Changes:
- Add
handleDebugServicethat unmarshalsDebugServiceRequestand answers Ping with aPingResponsetimestamp. - Add unit tests covering Ping handling and unset/unsupported subtopic behavior.
- Register the REMOTE_CONFIG namespace handler from the
testx509example’smain.
File summaries
| File | Description |
|---|---|
| ffi-hosts/go/cmd/testx509/pingpong.go | Implements the REMOTE_CONFIG DebugService dispatch handler and response creation. |
| ffi-hosts/go/cmd/testx509/pingpong_test.go | Adds tests validating ping response behavior and unsupported/unset subtopic handling. |
| ffi-hosts/go/cmd/testx509/main.go | Registers the new handler for the REMOTE_CONFIG namespace in the example client. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The handler uses the expected DebugService request and response envelopes. It registers before the client starts.
🤖 Datadog Autotest · Commit 02ff775 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| // handleDebugService implements libddrcffi.HandlerFunc for the | ||
| // NAMESPACE_REMOTE_CONFIG namespace. It answers a Ping subtopic with a Pong | ||
| // carrying the current time. | ||
| func handleDebugService(correlationID uint64, payload []byte) ([]byte, error) { | ||
| var req remoteconfigv1.DebugServiceRequest | ||
| if err := proto.Unmarshal(payload, &req); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| switch subtopic := req.GetSubtopic().(type) { | ||
| case *remoteconfigv1.DebugServiceRequest_Ping: | ||
| ping := subtopic.Ping | ||
| log.Printf("received ping (correlation_id=%d) from connection %q: reason=%q", correlationID, ping.GetConnectionId(), ping.GetReason()) | ||
|
|
||
| resp := &remoteconfigv1.DebugServiceResponse{ | ||
| Subtopic: &remoteconfigv1.DebugServiceResponse_Ping{ | ||
| Ping: &remoteconfigv1.PingResponse{ | ||
| Now: timestamppb.Now(), | ||
| }, | ||
| }, | ||
| } | ||
| return proto.Marshal(resp) | ||
| default: | ||
| return nil, errUnsupportedSubtopic | ||
| } | ||
| } |
There was a problem hiding this comment.
So this is the amount of code needed to integrate a magic tunnel handler + response for external teams 👌
Top stuff - this has very nicely abstracted away all the connection complexity 🙏
There was a problem hiding this comment.
🟡 Changes recommended
The new tests will emit log output on every run due to the handler’s global log.Printf, which should be silenced in tests to avoid noisy CI output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
ffi-hosts/go/cmd/testx509/pingpong_test.go:45
- This test also triggers log output from handleDebugService; call the shared test helper to discard logs here as well.
func TestHandleDebugService_UnsetSubtopic(t *testing.T) {
req := &remoteconfigv1.DebugServiceRequest{}
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| import ( | ||
| "errors" | ||
| "testing" | ||
|
|
||
| remoteconfigv1 "github.com/DataDog/libdd-rc/ffi-hosts/go/rcproto/magic_tunnel/remote_config" | ||
| "google.golang.org/protobuf/proto" | ||
| ) | ||
|
|
||
| func TestHandleDebugService_Ping(t *testing.T) { | ||
| req := &remoteconfigv1.DebugServiceRequest{ |
| } | ||
| return proto.Marshal(resp) | ||
| default: | ||
| return nil, errUnsupportedSubtopic |
There was a problem hiding this comment.
nit: Should the error also contain which subtopic was received for debugging purposes?
There was a problem hiding this comment.
This error will include the correlation_id in the backend / client logs that specifies which exact request caused the error 👍
Additionally, this error message will be returned to the team sending the request in the backend as the response to their gRPC call!
Adds a handler to allow us to handle the ping/pong magic tunnel dispatch messages as an example of how to do this, and to also help enable our e2e PoC.