Skip to content

Commit 26bcf0a

Browse files
committed
fix(telemetry): consistent invocation ID per run and add telemetry test infra
- root.go: set invocation ID only when empty so all API requests in one CLI run share the same invocation_id (fixes multiple IDs when gateway PreRun calls ValidateAPIKey then connection PreRun overwrote telemetry) - pkg/cmd: add TestInvocationIDPersistsAcrossMultipleInitTelemetryCalls (fails without fix, passes with it) - test/acceptance: recording proxy (StartRecordingProxy, AssertTelemetryConsistent, RunListenWithTimeout, RunWithEnv, NewCLIRunnerWithConfigPath/NoCI), telemetry_test.go proxy-based tests, TestTelemetryInvocationIDConsistentWhenValidateAPIKeyInPreRun, TestTelemetryListenProxy in listen_test.go, README section Made-with: Cursor
1 parent 444dcba commit 26bcf0a

6 files changed

Lines changed: 1663 additions & 1 deletion

File tree

pkg/cmd/root.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ func initTelemetry(cmd *cobra.Command) {
5151
tel.SetEnvironment(hookdeck.DetectEnvironment())
5252
tel.SetCommandContext(cmd)
5353
tel.SetDeviceName(Config.DeviceName)
54-
tel.SetInvocationID(hookdeck.NewInvocationID())
54+
if tel.InvocationID == "" {
55+
tel.SetInvocationID(hookdeck.NewInvocationID())
56+
}
5557
}
5658

5759
// RootCmd returns the root command for use by tools (e.g. generate-reference).

pkg/cmd/telemetry_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,39 @@ func TestInitTelemetryResetBetweenCalls(t *testing.T) {
8686
require.NotEqual(t, id1, tel2.InvocationID)
8787
}
8888

89+
// TestInvocationIDPersistsAcrossMultipleInitTelemetryCalls reproduces the v2.0.0 bug: when
90+
// initTelemetry is called multiple times in one process (e.g. root PreRun, then gateway PreRun,
91+
// then connection PreRun), the invocation_id must stay the same so all API requests in that run
92+
// share one ID. Without the fix, each initTelemetry call overwrites with a new ID; this test
93+
// asserts the ID is unchanged after a second call. On v2.0.0 (repro branch) it fails.
94+
func TestInvocationIDPersistsAcrossMultipleInitTelemetryCalls(t *testing.T) {
95+
hookdeck.ResetTelemetryInstanceForTesting()
96+
97+
root := &cobra.Command{Use: "hookdeck"}
98+
gateway := &cobra.Command{Use: "gateway"}
99+
connection := &cobra.Command{Use: "connection"}
100+
root.AddCommand(gateway)
101+
gateway.AddCommand(connection)
102+
103+
Config.DeviceName = "test-device"
104+
initTelemetry(root)
105+
tel := hookdeck.GetTelemetryInstance()
106+
firstID := tel.InvocationID
107+
require.NotEmpty(t, firstID, "first initTelemetry must set invocation_id")
108+
109+
// Second call simulates gateway PreRun; must not overwrite invocation_id.
110+
initTelemetry(gateway)
111+
tel = hookdeck.GetTelemetryInstance()
112+
require.Equal(t, firstID, tel.InvocationID,
113+
"invocation_id must persist across initTelemetry calls in the same process (fix: set only when empty)")
114+
115+
// Third call simulates connection PreRun; must still not overwrite.
116+
initTelemetry(connection)
117+
tel = hookdeck.GetTelemetryInstance()
118+
require.Equal(t, firstID, tel.InvocationID,
119+
"invocation_id must persist after third initTelemetry call")
120+
}
121+
89122
// TestInitTelemetryWhenDisabled verifies that initTelemetry always populates the
90123
// singleton (Source, CommandPath, etc.) even when telemetry is disabled. The
91124
// call must happen for every command; PerformRequest later skips sending the

test/acceptance/README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ These tests require browser-based authentication via `hookdeck login` and must b
1818

1919
**Why Manual?** These tests access endpoints (like `/teams`) that require CLI authentication keys obtained through interactive browser login, which aren't available to CI service accounts.
2020

21+
### Recording proxy (telemetry tests)
22+
23+
Some tests (e.g. `TestTelemetryGatewayConnectionListProxy` in `telemetry_test.go`) use a **recording proxy**: the CLI is run with `--api-base` pointing at a local HTTP server that forwards every request to the real Hookdeck API and records method, path, and the `X-Hookdeck-CLI-Telemetry` header. The same `CLIRunner` and `go run main.go` flow are used as in other acceptance tests; only the API base URL is overridden so traffic goes through the proxy. This verifies that a single CLI run sends consistent telemetry (same `invocation_id` and `command_path`) on all API calls. Helpers: `StartRecordingProxy`, `AssertTelemetryConsistent`.
24+
25+
**Login telemetry test (TestTelemetryLoginProxy)** uses the same proxy approach: it runs `hookdeck login --api-key KEY` with `--api-base` set to the proxy, and asserts exactly one recorded request (GET `/2025-07-01/cli-auth/validate`) with consistent telemetry. It uses **HOOKDECK_CLI_TESTING_CLI_KEY** (not the API/CI key), because the validate endpoint accepts CLI keys from interactive login; if unset, the test is skipped. Other telemetry tests still use the normal API key via `NewCLIRunner`.
26+
2127
## Setup
2228

2329
### Local Development

test/acceptance/helpers.go

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,162 @@ import (
55
"bytes"
66
"encoding/json"
77
"fmt"
8+
"io"
89
"net/http"
10+
"net/http/httptest"
11+
"net/url"
912
"os"
1013
"os/exec"
1114
"path/filepath"
1215
"strings"
16+
"sync"
1317
"testing"
1418
"time"
1519

1620
"github.com/stretchr/testify/assert"
1721
"github.com/stretchr/testify/require"
1822
)
1923

24+
// defaultAPIUpstream is the real Hookdeck API base URL used by the recording proxy.
25+
const defaultAPIUpstream = "https://api.hookdeck.com"
26+
27+
// RecordedRequest holds a single HTTP request as captured by the recording proxy.
28+
type RecordedRequest struct {
29+
Method string
30+
Path string
31+
Telemetry string
32+
}
33+
34+
// RecordingProxy is an HTTP server that forwards requests to the real API and
35+
// records each request (including X-Hookdeck-CLI-Telemetry). Use it to run the
36+
// CLI with --api-base pointing at the proxy so all API traffic is captured
37+
// while still hitting the real backend.
38+
type RecordingProxy struct {
39+
t *testing.T
40+
server *httptest.Server
41+
upstream string
42+
mu sync.Mutex
43+
recorded []RecordedRequest
44+
}
45+
46+
// URL returns the proxy base URL (e.g. http://127.0.0.1:port). Pass this to
47+
// the CLI as --api-base so requests go through the proxy.
48+
func (p *RecordingProxy) URL() string {
49+
return p.server.URL
50+
}
51+
52+
// Recorded returns a copy of the slice of recorded requests. Safe to call
53+
// after the CLI command has finished.
54+
func (p *RecordingProxy) Recorded() []RecordedRequest {
55+
p.mu.Lock()
56+
defer p.mu.Unlock()
57+
out := make([]RecordedRequest, len(p.recorded))
58+
copy(out, p.recorded)
59+
return out
60+
}
61+
62+
// Close shuts down the proxy server.
63+
func (p *RecordingProxy) Close() {
64+
p.server.Close()
65+
}
66+
67+
// StartRecordingProxy starts an httptest.Server that acts as a reverse proxy to
68+
// upstreamBase (e.g. https://api.hookdeck.com). Every request is recorded
69+
// (method, path, X-Hookdeck-CLI-Telemetry) and then forwarded to the upstream;
70+
// the upstream response is returned to the client. Use with CLIRunner.Run("--api-base", proxy.URL(), "gateway", ...).
71+
func StartRecordingProxy(t *testing.T, upstreamBase string) *RecordingProxy {
72+
t.Helper()
73+
upstream, err := url.Parse(strings.TrimSuffix(upstreamBase, "/"))
74+
require.NoError(t, err, "parse upstream URL")
75+
76+
p := &RecordingProxy{
77+
t: t,
78+
upstream: upstream.String(),
79+
recorded: make([]RecordedRequest, 0),
80+
}
81+
82+
p.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
83+
// Record before forwarding
84+
p.mu.Lock()
85+
p.recorded = append(p.recorded, RecordedRequest{
86+
Method: r.Method,
87+
Path: r.URL.Path,
88+
Telemetry: r.Header.Get("X-Hookdeck-CLI-Telemetry"),
89+
})
90+
p.mu.Unlock()
91+
92+
// Build upstream request: same method, path, query, body
93+
targetPath := r.URL.Path
94+
if r.URL.RawQuery != "" {
95+
targetPath += "?" + r.URL.RawQuery
96+
}
97+
dest, err := url.Parse(upstream.String() + targetPath)
98+
require.NoError(p.t, err)
99+
100+
var bodyReader io.Reader
101+
if r.Body != nil {
102+
bodyReader = r.Body
103+
}
104+
105+
req, err := http.NewRequest(r.Method, dest.String(), bodyReader)
106+
require.NoError(p.t, err)
107+
108+
// Copy headers that the API cares about
109+
for _, k := range []string{
110+
"Authorization", "Content-Type", "X-Team-ID", "X-Project-ID",
111+
"X-Hookdeck-CLI-Telemetry", "X-Hookdeck-Client-User-Agent", "User-Agent",
112+
} {
113+
if v := r.Header.Get(k); v != "" {
114+
req.Header.Set(k, v)
115+
}
116+
}
117+
118+
resp, err := http.DefaultClient.Do(req)
119+
require.NoError(p.t, err)
120+
defer resp.Body.Close()
121+
122+
// Copy response back
123+
for k, v := range resp.Header {
124+
for _, vv := range v {
125+
w.Header().Add(k, vv)
126+
}
127+
}
128+
w.WriteHeader(resp.StatusCode)
129+
_, _ = io.Copy(w, resp.Body)
130+
}))
131+
132+
return p
133+
}
134+
135+
// telemetryPayload is the structure of the X-Hookdeck-CLI-Telemetry header (JSON).
136+
type telemetryPayload struct {
137+
CommandPath string `json:"command_path"`
138+
InvocationID string `json:"invocation_id"`
139+
}
140+
141+
// AssertTelemetryConsistent checks that every recorded request that has a
142+
// telemetry header shares the same invocation_id and command_path, and that
143+
// command_path equals expectedCommandPath.
144+
func AssertTelemetryConsistent(t *testing.T, recorded []RecordedRequest, expectedCommandPath string) {
145+
t.Helper()
146+
var invocationID, commandPath string
147+
for i, r := range recorded {
148+
if r.Telemetry == "" {
149+
continue
150+
}
151+
var p telemetryPayload
152+
require.NoError(t, json.Unmarshal([]byte(r.Telemetry), &p), "request %d: invalid telemetry JSON: %s", i, r.Telemetry)
153+
if invocationID == "" {
154+
invocationID = p.InvocationID
155+
commandPath = p.CommandPath
156+
}
157+
require.Equal(t, invocationID, p.InvocationID, "request %d (%s %s): invocation_id should be consistent", i, r.Method, r.Path)
158+
require.Equal(t, commandPath, p.CommandPath, "request %d (%s %s): command_path should be consistent", i, r.Method, r.Path)
159+
}
160+
require.Equal(t, expectedCommandPath, commandPath, "command_path should match expected")
161+
require.NotEmpty(t, invocationID, "at least one request should have invocation_id")
162+
}
163+
20164
func init() {
21165
// Attempt to load .env file from test/acceptance/.env for local development
22166
// In CI, the environment variable will be set directly
@@ -86,6 +230,43 @@ func NewCLIRunner(t *testing.T) *CLIRunner {
86230
return runner
87231
}
88232

233+
// NewCLIRunnerWithConfigPath creates a CLI runner that uses the given config file path.
234+
// It runs "ci --api-key" so the config is populated (api_key, project_id, project_mode, etc.).
235+
// Use this when a test needs a known config path (e.g. to write a minimal variant for another run).
236+
func NewCLIRunnerWithConfigPath(t *testing.T, configPath string) *CLIRunner {
237+
t.Helper()
238+
apiKey := getAcceptanceAPIKey(t)
239+
require.NotEmpty(t, apiKey, "HOOKDECK_CLI_TESTING_API_KEY must be set")
240+
projectRoot, err := filepath.Abs("../..")
241+
require.NoError(t, err, "Failed to get project root path")
242+
runner := &CLIRunner{
243+
t: t,
244+
apiKey: apiKey,
245+
projectRoot: projectRoot,
246+
configPath: configPath,
247+
}
248+
stdout, stderr, err := runner.Run("ci", "--api-key", apiKey)
249+
require.NoError(t, err, "Failed to authenticate CLI with config at %s: stdout=%s stderr=%s", configPath, stdout, stderr)
250+
return runner
251+
}
252+
253+
// NewCLIRunnerWithConfigPathNoCI creates a CLI runner that uses the given config file path,
254+
// without running "ci". Use when the config file is already populated (e.g. a minimal config
255+
// written by the test). The runner will pass HOOKDECK_CONFIG_FILE to all Run() calls.
256+
func NewCLIRunnerWithConfigPathNoCI(t *testing.T, configPath string) *CLIRunner {
257+
t.Helper()
258+
apiKey := getAcceptanceAPIKey(t)
259+
require.NotEmpty(t, apiKey, "HOOKDECK_CLI_TESTING_API_KEY must be set")
260+
projectRoot, err := filepath.Abs("../..")
261+
require.NoError(t, err, "Failed to get project root path")
262+
return &CLIRunner{
263+
t: t,
264+
apiKey: apiKey,
265+
projectRoot: projectRoot,
266+
configPath: configPath,
267+
}
268+
}
269+
89270
// getAcceptanceAPIKey returns the API key for the current acceptance slice.
90271
// When ACCEPTANCE_SLICE=1 and HOOKDECK_CLI_TESTING_API_KEY_2 is set, use it; when ACCEPTANCE_SLICE=2 and HOOKDECK_CLI_TESTING_API_KEY_3 is set, use that; else HOOKDECK_CLI_TESTING_API_KEY.
91272
func getAcceptanceAPIKey(t *testing.T) string {
@@ -198,6 +379,80 @@ func (r *CLIRunner) Run(args ...string) (stdout, stderr string, err error) {
198379
return stdoutBuf.String(), stderrBuf.String(), err
199380
}
200381

382+
// RunWithEnv is like Run but merges extraEnv into the process environment (e.g. for HOOKDECK_CLI_USE_SYSTEM_BINARY).
383+
// When extraEnv["HOOKDECK_CLI_USE_SYSTEM_BINARY"] == "1", runs the installed "hookdeck" binary on PATH instead of "go run main.go"
384+
// (e.g. to run tests against 2.0.0 or another installed version).
385+
func (r *CLIRunner) RunWithEnv(extraEnv map[string]string, args ...string) (stdout, stderr string, err error) {
386+
r.t.Helper()
387+
388+
env := os.Environ()
389+
if r.configPath != "" {
390+
env = appendEnvOverride(env, "HOOKDECK_CONFIG_FILE", r.configPath)
391+
}
392+
for k, v := range extraEnv {
393+
env = appendEnvOverride(env, k, v)
394+
}
395+
396+
var cmd *exec.Cmd
397+
if extraEnv != nil && extraEnv["HOOKDECK_CLI_USE_SYSTEM_BINARY"] == "1" {
398+
hookdeckPath, lookErr := exec.LookPath("hookdeck")
399+
if lookErr != nil {
400+
return "", "", fmt.Errorf("HOOKDECK_CLI_USE_SYSTEM_BINARY=1 but hookdeck not on PATH: %w", lookErr)
401+
}
402+
cmd = exec.Command(hookdeckPath, args...)
403+
cmd.Dir = r.projectRoot
404+
} else {
405+
mainGoPath := filepath.Join(r.projectRoot, "main.go")
406+
cmdArgs := append([]string{"run", mainGoPath}, args...)
407+
cmd = exec.Command("go", cmdArgs...)
408+
cmd.Dir = r.projectRoot
409+
}
410+
cmd.Env = env
411+
412+
var stdoutBuf, stderrBuf bytes.Buffer
413+
cmd.Stdout = &stdoutBuf
414+
cmd.Stderr = &stderrBuf
415+
err = cmd.Run()
416+
return stdoutBuf.String(), stderrBuf.String(), err
417+
}
418+
419+
// RunListenWithTimeout starts the CLI with the given args (e.g. "--api-base", proxyURL,
420+
// "listen", port, sourceName, "--output", "compact"), lets it run for runDuration, then
421+
// kills the process. Uses a pre-built binary so we terminate the actual listen process
422+
// (not a "go run" parent). Returns stdout, stderr, and the error from Wait (often
423+
// non-nil because the process was killed). Uses the same project root and config env as Run().
424+
func (r *CLIRunner) RunListenWithTimeout(args []string, runDuration time.Duration) (stdout, stderr string, err error) {
425+
r.t.Helper()
426+
tmpBinary := filepath.Join(r.projectRoot, "hookdeck-listen-test-"+generateTimestamp())
427+
defer os.Remove(tmpBinary)
428+
429+
buildCmd := exec.Command("go", "build", "-o", tmpBinary, ".")
430+
buildCmd.Dir = r.projectRoot
431+
if err := buildCmd.Run(); err != nil {
432+
return "", "", fmt.Errorf("build CLI for listen test: %w", err)
433+
}
434+
435+
cmd := exec.Command(tmpBinary, args...)
436+
cmd.Dir = r.projectRoot
437+
if r.configPath != "" {
438+
cmd.Env = appendEnvOverride(os.Environ(), "HOOKDECK_CONFIG_FILE", r.configPath)
439+
}
440+
var stdoutBuf, stderrBuf bytes.Buffer
441+
cmd.Stdout = &stdoutBuf
442+
cmd.Stderr = &stderrBuf
443+
if err := cmd.Start(); err != nil {
444+
return "", "", err
445+
}
446+
timer := time.AfterFunc(runDuration, func() {
447+
if cmd.Process != nil {
448+
_ = cmd.Process.Kill()
449+
}
450+
})
451+
defer timer.Stop()
452+
waitErr := cmd.Wait()
453+
return stdoutBuf.String(), stderrBuf.String(), waitErr
454+
}
455+
201456
// RunFromCwd executes the CLI from the current working directory.
202457
// This is useful for tests that need to test --local flag behavior,
203458
// which creates config in the current directory.

test/acceptance/listen_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,36 @@ func TestListenCommandWithContext(t *testing.T) {
163163

164164
t.Logf("Listen command terminated via context cancellation")
165165
}
166+
167+
// TestTelemetryListenProxy runs "listen" with the CLI pointed at a recording proxy
168+
// that forwards to the real API. It lets listen run for a few seconds (so it can
169+
// perform initial API calls: get sources, get connections, create session), then
170+
// stops it. Asserts that every API request in that run has the same invocation_id
171+
// and command_path "hookdeck listen".
172+
func TestTelemetryListenProxy(t *testing.T) {
173+
if testing.Short() {
174+
t.Skip("Skipping acceptance test in short mode")
175+
}
176+
177+
cli := NewCLIRunner(t)
178+
proxy := StartRecordingProxy(t, defaultAPIUpstream)
179+
defer proxy.Close()
180+
181+
timestamp := generateTimestamp()
182+
sourceName := "test-telemetry-" + timestamp
183+
184+
_, _, _ = cli.RunListenWithTimeout([]string{
185+
"--api-base", proxy.URL(),
186+
"listen", "9999", sourceName,
187+
"--output", "compact",
188+
}, 6*time.Second)
189+
// Process is killed after 6s; we don't assert on exit error
190+
191+
recorded := proxy.Recorded()
192+
require.GreaterOrEqual(t, len(recorded), 1,
193+
"expected at least one API request from listen (sources, connections, or cli-sessions)")
194+
for i, req := range recorded {
195+
t.Logf("listen API request %d: %s %s (telemetry: %s)", i+1, req.Method, req.Path, req.Telemetry)
196+
}
197+
AssertTelemetryConsistent(t, recorded, "hookdeck listen")
198+
}

0 commit comments

Comments
 (0)