From 7f78b690c9b260959686e5d739e124cc242f4808 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 16 May 2026 15:06:20 +0000 Subject: [PATCH] add hermetic tests for EscapeTag, statsd, LoadConfig, HTTPS, UrlString Adds table-driven tests for pure functions that previously had no coverage, all hermetic per AGENTS.md (no real network beyond loopback httptest, no filesystem outside t.TempDir). - statsd_test.go: EscapeTag for :, |, ,, @ plus a refs-#14 case pinning the missing newline/CR handling. Count/Timer/Gauge wire-format assertions with queue draining to keep package state clean between cases. - config_test.go: LoadConfig with t.TempDir fixtures (empty path, minimal YAML, multi-URL), a refs-#6 case encoding the map[string]string loader bug, and a subprocess-based check pinning the log.Fatalf-on-missing-file boundary. - https_test.go: HTTPS against httptest.NewServer covering 2xx success, refs-#7 (5xx still reports success), refs-#15 (silently follows redirects), and a dial-failure case that's already correct. - destinations_test.go: TestUrlString table-driven cases pinning password redaction as "[...]" (refs #12), userinfo variants, custom paths, the icmp port=-1 omission, and the scheme!=protocol parens annotation. Tests for known bugs (#6, #7, #12, #14, #15) document current behavior so future fixes have a failing target to flip; each is marked "Refs #N -- flip when fixed". Fixes #47 https://claude.ai/code/session_01WjHPSobuzrRkjwUgjAJWMk --- config_test.go | 209 ++++++++++++++++++++++++++++++++++++ destinations_test.go | 142 +++++++++++++++++++++++++ https_test.go | 115 ++++++++++++++++++++ statsd_test.go | 247 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 713 insertions(+) create mode 100644 config_test.go create mode 100644 https_test.go create mode 100644 statsd_test.go diff --git a/config_test.go b/config_test.go new file mode 100644 index 0000000..2962741 --- /dev/null +++ b/config_test.go @@ -0,0 +1,209 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// writeConfig writes content to a config file in a hermetic temp dir and +// returns the path. The file (and its parent dir) are torn down by +// t.TempDir's cleanup. +func writeConfig(t *testing.T, content string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "connectivity.yml") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("os.WriteFile(%q): %v", path, err) + } + return path +} + +// findURL returns the Url matching label, or nil if not found. It exists so +// table-driven assertions can look up entries without depending on map +// iteration order (the loader is `for k, v := range configMap` over the YAML +// keys). +func findURL(urls []Url, label string) *Url { + for i := range urls { + if urls[i].Label == label { + return &urls[i] + } + } + return nil +} + +func TestLoadConfig_EmptyPathReturnsEmptyConfig(t *testing.T) { + cfg := LoadConfig("") + if cfg == nil { + t.Fatalf("LoadConfig(\"\") = nil; want non-nil *Config") + } + if len(cfg.URLs) != 0 { + t.Errorf("LoadConfig(\"\").URLs = %v; want empty", cfg.URLs) + } + if cfg.StatsdHost != "" { + t.Errorf("LoadConfig(\"\").StatsdHost = %q; want empty (defaults are only applied when a path is given)", cfg.StatsdHost) + } + if cfg.StatsdPort != 0 { + t.Errorf("LoadConfig(\"\").StatsdPort = %d; want 0", cfg.StatsdPort) + } + if cfg.StatsdProtocol != "" { + t.Errorf("LoadConfig(\"\").StatsdProtocol = %q; want empty", cfg.StatsdProtocol) + } +} + +func TestLoadConfig_MinimalYAMLAppliesDefaults(t *testing.T) { + path := writeConfig(t, "example: http://example.com\n") + cfg := LoadConfig(path) + + if cfg.StatsdHost != "127.0.0.1" { + t.Errorf("StatsdHost = %q; want %q (default)", cfg.StatsdHost, "127.0.0.1") + } + if cfg.StatsdPort != 8125 { + t.Errorf("StatsdPort = %d; want 8125 (default)", cfg.StatsdPort) + } + if cfg.StatsdProtocol != "udp" { + t.Errorf("StatsdProtocol = %q; want %q (default)", cfg.StatsdProtocol, "udp") + } + if len(cfg.URLs) != 1 { + t.Fatalf("len(URLs) = %d; want 1", len(cfg.URLs)) + } + got := findURL(cfg.URLs, "example") + if got == nil { + t.Fatalf("URL with label %q not found; URLs = %+v", "example", cfg.URLs) + } + if got.Url != "http://example.com" { + t.Errorf("URLs[example].Url = %q; want %q", got.Url, "http://example.com") + } +} + +func TestLoadConfig_MultipleURLs(t *testing.T) { + yaml := "" + + "a: http://a.example.com\n" + + "b: https://b.example.com\n" + + "c: tcp://c.example.com:1234\n" + path := writeConfig(t, yaml) + cfg := LoadConfig(path) + + if len(cfg.URLs) != 3 { + t.Fatalf("len(URLs) = %d; want 3 — URLs = %+v", len(cfg.URLs), cfg.URLs) + } + want := map[string]string{ + "a": "http://a.example.com", + "b": "https://b.example.com", + "c": "tcp://c.example.com:1234", + } + for label, wantURL := range want { + got := findURL(cfg.URLs, label) + if got == nil { + t.Errorf("URL with label %q not found; URLs = %+v", label, cfg.URLs) + continue + } + if got.Url != wantURL { + t.Errorf("URLs[%s].Url = %q; want %q", label, got.Url, wantURL) + } + } +} + +// TestLoadConfig_StatsdKeysBecomeURLs documents the #6 bug: the loader +// unmarshals into map[string]string and treats every YAML key as a URL label, +// so typed config keys like statsd_host / statsd_port / statsd_protocol end up +// in the URLs slice instead of populating the Config struct. +// +// Refs #6 — flip when fixed: once the loader honors the Config struct, these +// keys should populate StatsdHost/StatsdPort/StatsdProtocol and NOT appear in +// URLs. Note that statsd_port's YAML value is an int and would fail to +// unmarshal into map[string]string today, so we use a string-shaped fixture +// the current loader can parse (otherwise the bug manifests as a log.Fatalf +// in LoadConfig rather than a misclassified URL). +func TestLoadConfig_StatsdKeysBecomeURLs(t *testing.T) { + yaml := "" + + "statsd_host: \"statsd.example.com\"\n" + + "statsd_protocol: \"tcp\"\n" + + "example: \"http://example.com\"\n" + path := writeConfig(t, yaml) + cfg := LoadConfig(path) + + // Bug: statsd_host and statsd_protocol are treated as URL labels. + if got := findURL(cfg.URLs, "statsd_host"); got == nil { + t.Errorf("expected URL with label %q to be present (current buggy behavior — #6); URLs = %+v", "statsd_host", cfg.URLs) + } else if got.Url != "statsd.example.com" { + t.Errorf("URLs[statsd_host].Url = %q; want %q (current buggy behavior — #6)", got.Url, "statsd.example.com") + } + if got := findURL(cfg.URLs, "statsd_protocol"); got == nil { + t.Errorf("expected URL with label %q to be present (current buggy behavior — #6); URLs = %+v", "statsd_protocol", cfg.URLs) + } + + // Bug: defaults are applied because the typed Config fields were never + // populated from YAML — the statsd_host value above is silently dropped. + if cfg.StatsdHost != "127.0.0.1" { + t.Errorf("StatsdHost = %q; want %q (current buggy behavior — #6: typed YAML keys are dropped, so defaults kick in)", cfg.StatsdHost, "127.0.0.1") + } + if cfg.StatsdProtocol != "udp" { + t.Errorf("StatsdProtocol = %q; want %q (current buggy behavior — #6: typed YAML keys are dropped, so defaults kick in)", cfg.StatsdProtocol, "udp") + } +} + +// TestLoadConfig_MissingFileFatals pins the current log.Fatalf-on-read-error +// behavior. The check uses the helper subprocess pattern: this test re-execs +// the test binary with an environment variable that triggers the helper +// branch to invoke LoadConfig on a non-existent path. We assert the +// subprocess exits non-zero and prints a message naming the file. +// +// The log.Fatalf paths in LoadConfig violate the AGENTS.md guidance +// ("log.Fatalf is acceptable only at process startup; library-level code +// returns errors"). Pinning the current behavior in a test makes future +// refactoring observable. +func TestLoadConfig_MissingFileFatals(t *testing.T) { + if os.Getenv("CONNECTIVITY_TEST_LOADCONFIG_HELPER") == "1" { + // Child process: invoke LoadConfig on a path guaranteed not to + // exist. log.Fatalf will call os.Exit(1). + LoadConfig(filepath.Join(t.TempDir(), "does-not-exist.yml")) + // Unreachable when the bug/behavior is intact. + return + } + + cmd := exec.Command(os.Args[0], "-test.run=^TestLoadConfig_MissingFileFatals$") + cmd.Env = append(os.Environ(), "CONNECTIVITY_TEST_LOADCONFIG_HELPER=1") + out, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("subprocess exited with status 0; want non-zero. output:\n%s", out) + } + exitErr, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("subprocess err = %v (%T); want *exec.ExitError", err, err) + } + if exitErr.ExitCode() == 0 { + t.Errorf("subprocess exit code = 0; want non-zero") + } + if !strings.Contains(string(out), "Failed to open config file") { + t.Errorf("subprocess output = %q; want it to contain %q", string(out), "Failed to open config file") + } +} + +func TestFindConfig_ReturnsErrorWhenNoneExist(t *testing.T) { + // Run in a temp dir so the current working directory does not contain + // any of the relative ConfigPaths (connectivity.yml, etc). + dir := t.TempDir() + prev, err := os.Getwd() + if err != nil { + t.Fatalf("os.Getwd: %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(prev); err != nil { + t.Fatalf("os.Chdir(%q): %v", prev, err) + } + }) + if err := os.Chdir(dir); err != nil { + t.Fatalf("os.Chdir(%q): %v", dir, err) + } + + path, err := FindConfig() + if err == nil { + t.Errorf("FindConfig() = (%q, nil); want non-nil error when no config file exists", path) + } + if path != "" { + t.Errorf("FindConfig() path = %q; want empty when error is non-nil", path) + } +} diff --git a/destinations_test.go b/destinations_test.go index db9f4aa..dcc52ec 100644 --- a/destinations_test.go +++ b/destinations_test.go @@ -262,3 +262,145 @@ func TestUdpUrlWithPort(t *testing.T) { assertHostEquals(t, got.Host, "host") assertPortEquals(t, got.Port, 123) } + +// TestUrlString table-tests Destination.UrlString. The key invariant is that +// when a password is set on the URL, it is redacted as `[...]` in the +// formatted string (refs #12). A regression here would leak credentials into +// logs at INFO level — every Check call passes the destination through +// LogDestination, which formats with %s and thus calls String() -> +// UrlString(). +// +// Cases exercise the cross-product of {no userinfo, username only, username +// +password} × {no port, with port} × {default vs custom path} × {scheme == +// protocol vs scheme != protocol}. +func TestUrlString(t *testing.T) { + cases := []struct { + name string + dest Destination + want string + }{ + { + name: "tcp_scheme_equals_protocol_no_parens", + dest: Destination{Scheme: "tcp", Protocol: "tcp", Host: "example.com", Port: 1234, Path: ""}, + want: "tcp://example.com:1234", + }, + { + name: "http_appends_tcp_in_parens_because_scheme_differs_from_protocol", + dest: Destination{Scheme: "http", Protocol: "tcp", Host: "example.com", Port: 80, Path: ""}, + want: "http://example.com:80 (tcp)", + }, + { + name: "https_with_path_appends_tcp_in_parens", + dest: Destination{Scheme: "https", Protocol: "tcp", Host: "example.com", Port: 443, Path: "/health"}, + want: "https://example.com:443/health (tcp)", + }, + { + name: "username_only_no_password_set", + dest: Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "alice", + PasswordSet: false, + Host: "example.com", + Port: 443, + }, + want: "https://alice@example.com:443 (tcp)", + }, + { + name: "username_with_password_redacts_as_brackets_ellipsis", + dest: Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "alice", + Password: "hunter2", + PasswordSet: true, + Host: "example.com", + Port: 443, + }, + want: "https://alice:[...]@example.com:443 (tcp)", + }, + { + name: "username_with_empty_password_still_redacts", + dest: Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "alice", + Password: "", + PasswordSet: true, + Host: "example.com", + Port: 443, + }, + want: "https://alice:[...]@example.com:443 (tcp)", + }, + { + name: "username_with_password_and_custom_path", + dest: Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "alice", + Password: "hunter2", + PasswordSet: true, + Host: "example.com", + Port: 8443, + Path: "/api/v1/health", + }, + want: "https://alice:[...]@example.com:8443/api/v1/health (tcp)", + }, + { + name: "icmp_omits_port_when_minus_one_and_no_parens_when_scheme_equals_protocol", + dest: Destination{Scheme: "icmp", Protocol: "icmp", Host: "example.com", Port: -1, Path: ""}, + want: "icmp://example.com", + }, + { + name: "mysql_appends_tcp_in_parens", + dest: Destination{Scheme: "mysql", Protocol: "tcp", Host: "example.com", Port: 3306, Path: ""}, + want: "mysql://example.com:3306 (tcp)", + }, + { + name: "no_userinfo_when_username_empty_even_if_password_set", + dest: Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "", + Password: "hunter2", + PasswordSet: true, + Host: "example.com", + Port: 443, + }, + want: "https://example.com:443 (tcp)", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := tc.dest.UrlString() + if got != tc.want { + t.Errorf("UrlString() = %q; want %q", got, tc.want) + } + }) + } +} + +// TestUrlString_RedactedDoesNotContainPassword belt-and-suspenders for #12: +// no matter the input shape, the formatted string must not contain the raw +// password value. This catches regressions where a refactor swaps the order +// of the userinfo branches and accidentally drops the redaction. +func TestUrlString_RedactedDoesNotContainPassword(t *testing.T) { + const secret = "s3cretP@ss" + dest := Destination{ + Scheme: "https", + Protocol: "tcp", + Username: "alice", + Password: secret, + PasswordSet: true, + Host: "example.com", + Port: 443, + Path: "/", + } + got := dest.UrlString() + if strings.Contains(got, secret) { + t.Errorf("UrlString() = %q; must not contain password %q", got, secret) + } + if !strings.Contains(got, "[...]") { + t.Errorf("UrlString() = %q; want it to contain redaction marker %q", got, "[...]") + } +} diff --git a/https_test.go b/https_test.go new file mode 100644 index 0000000..ec3c4ed --- /dev/null +++ b/https_test.go @@ -0,0 +1,115 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +// newTestDestination returns a *Destination wired to the given URL with a +// label so log calls inside HTTPS don't panic. The Scheme/Host/Port fields +// aren't read by HTTPS itself (it uses dest.URL), but they're populated for +// consistency with what NewDestination would produce. +func newTestDestination(t *testing.T, url string) *Destination { + t.Helper() + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + return &Destination{ + Label: "test", + URL: url, + Protocol: "tcp", + Scheme: "http", + Host: "example.com", + Port: 80, + } +} + +func TestHTTPS_Returns200(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + dest := newTestDestination(t, srv.URL) + if !HTTPS(dest) { + t.Errorf("HTTPS(2xx) = false; want true") + } +} + +// TestHTTPS_Returns500ButReportsSuccess documents that HTTPS does not check +// the response status code, so a 5xx response still returns true. The Go +// stdlib http.Get only returns an error for transport-level failures (DNS, +// dial, TLS, etc.), not for HTTP error statuses. +// +// Refs #7 — flip when fixed: once HTTPS checks status codes, a 5xx response +// should return false. To flip this test then, change `want true` to +// `want false` and update the test name. +func TestHTTPS_Returns500ButReportsSuccess(t *testing.T) { + cases := []struct { + name string + status int + }{ + {name: "internal_server_error", status: http.StatusInternalServerError}, + {name: "bad_gateway", status: http.StatusBadGateway}, + {name: "service_unavailable", status: http.StatusServiceUnavailable}, + {name: "not_found", status: http.StatusNotFound}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.status) + })) + t.Cleanup(srv.Close) + + dest := newTestDestination(t, srv.URL) + if !HTTPS(dest) { + t.Errorf("HTTPS(%d) = false; want true (current buggy behavior — #7: no status-code check)", tc.status) + } + }) + } +} + +// TestHTTPS_FollowsRedirects documents that HTTPS follows redirects (up to +// Go's default of 10 hops) without restriction. A check tool that silently +// follows a redirect to a different host can mask the very routing / +// availability issues it's meant to detect. +// +// Refs #15 — flip when fixed: once HTTPS disables redirect following (or +// records the redirect chain), this test should assert that the destination +// behind a 3xx is NOT considered reachable, or that the redirect target was +// recorded. +func TestHTTPS_FollowsRedirects(t *testing.T) { + var finalHits int32 + final := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&finalHits, 1) + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(final.Close) + + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, final.URL, http.StatusFound) + })) + t.Cleanup(redirector.Close) + + dest := newTestDestination(t, redirector.URL) + if !HTTPS(dest) { + t.Errorf("HTTPS(redirect) = false; want true (current buggy behavior — #15: redirects are followed)") + } + if got := atomic.LoadInt32(&finalHits); got != 1 { + t.Errorf("final server hit count = %d; want 1 (current buggy behavior — #15: redirects are followed)", got) + } +} + +// TestHTTPS_DialFailureReturnsFalse pins the only error path HTTPS currently +// surfaces: a transport-level dial failure. This is the one input where the +// current implementation behaves correctly, so the assertion is `want false` +// outright. +func TestHTTPS_DialFailureReturnsFalse(t *testing.T) { + // 127.0.0.1:1 is a port that's vanishingly unlikely to have a + // listener; the connection refuses immediately on Linux. + dest := newTestDestination(t, "http://127.0.0.1:1/") + if HTTPS(dest) { + t.Errorf("HTTPS(unreachable) = true; want false") + } +} diff --git a/statsd_test.go b/statsd_test.go new file mode 100644 index 0000000..1042a85 --- /dev/null +++ b/statsd_test.go @@ -0,0 +1,247 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// drainQueue removes all pending messages from the package-level statsd queue +// so each test starts and ends with an empty channel. Tests that don't enqueue +// anything still call drainQueue via t.Cleanup as a defensive measure against +// state leaking between cases. +func drainQueue(t *testing.T) { + t.Helper() + for { + select { + case <-queue: + default: + return + } + } +} + +// recvQueue reads one message from the queue with a short timeout. A timeout +// indicates the function under test did not enqueue anything, which is a test +// failure rather than a hang. +func recvQueue(t *testing.T) string { + t.Helper() + select { + case s := <-queue: + return s + case <-time.After(100 * time.Millisecond): + t.Fatalf("timed out waiting for message on statsd queue") + return "" + } +} + +func TestEscapeTag(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {name: "colon", in: "a:b", want: "a-b"}, + {name: "pipe", in: "a|b", want: "a-b"}, + {name: "comma", in: "a,b", want: "a-b"}, + {name: "at", in: "a@b", want: "a-b"}, + {name: "all_specials_combined", in: "a:b|c,d@e", want: "a-b-c-d-e"}, + {name: "no_specials_unchanged", in: "plain.tag_value-1", want: "plain.tag_value-1"}, + {name: "empty_string", in: "", want: ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := EscapeTag(tc.in) + if got != tc.want { + t.Errorf("EscapeTag(%q) = %q; want %q", tc.in, got, tc.want) + } + }) + } +} + +// TestEscapeTag_DoesNotEscapeNewlineOrCR documents the missing newline/CR +// escaping in EscapeTag (#14). The wire-protocol injection risk is that a tag +// containing a newline ends the current statsd message and starts a new one +// the collector parses separately. +// +// Refs #14 — flip when fixed: once EscapeTag also rewrites \n and \r, the +// expected values below should change to "a-b" and the test name's assertion +// flipped to require sanitization. +func TestEscapeTag_DoesNotEscapeNewlineOrCR(t *testing.T) { + cases := []struct { + name string + in string + want string // current (buggy) behavior: passes the newline/CR through + }{ + {name: "newline_passes_through", in: "a\nb", want: "a\nb"}, + {name: "carriage_return_passes_through", in: "a\rb", want: "a\rb"}, + {name: "crlf_passes_through", in: "a\r\nb", want: "a\r\nb"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := EscapeTag(tc.in) + if got != tc.want { + t.Errorf("EscapeTag(%q) = %q; want %q (current buggy behavior — #14)", tc.in, got, tc.want) + } + }) + } +} + +func TestCount_WireFormat(t *testing.T) { + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + + cases := []struct { + name string + metric string + value int + tags []string + want string + }{ + { + name: "single_tag", + metric: "connectivity.check", + value: 1, + tags: []string{"dest_host:example.com"}, + want: "connectivity.check:1|c|#dest_host:example.com", + }, + { + name: "multiple_tags_comma_joined", + metric: "connectivity.check", + value: 3, + tags: []string{"dest_host:example.com", "dest_port:443"}, + want: "connectivity.check:3|c|#dest_host:example.com,dest_port:443", + }, + { + name: "zero_value", + metric: "m", + value: 0, + tags: []string{"t:v"}, + want: "m:0|c|#t:v", + }, + { + name: "negative_value", + metric: "m", + value: -5, + tags: []string{"t:v"}, + want: "m:-5|c|#t:v", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + Count(tc.metric, tc.value, tc.tags) + got := recvQueue(t) + if got != tc.want { + t.Errorf("Count enqueued %q; want %q", got, tc.want) + } + }) + } +} + +func TestIncrement_EnqueuesCountOfOne(t *testing.T) { + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + + Increment("connectivity.check", []string{"dest_host:example.com"}) + got := recvQueue(t) + want := "connectivity.check:1|c|#dest_host:example.com" + if got != want { + t.Errorf("Increment enqueued %q; want %q", got, want) + } +} + +func TestTimer_WireFormat(t *testing.T) { + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + + cases := []struct { + name string + metric string + took time.Duration + tags []string + want string + }{ + { + name: "whole_millisecond", + metric: "connectivity.lookup", + took: 5 * time.Millisecond, + tags: []string{"dest_host:example.com"}, + want: "connectivity.lookup:5|ms|#dest_host:example.com", + }, + { + name: "sub_millisecond_truncates_to_zero", + metric: "connectivity.lookup", + took: 500 * time.Microsecond, + tags: []string{"t:v"}, + want: "connectivity.lookup:0|ms|#t:v", + }, + { + name: "second_converts_to_1000ms", + metric: "m", + took: time.Second, + tags: []string{"t:v"}, + want: "m:1000|ms|#t:v", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + Timer(tc.metric, tc.took, tc.tags) + got := recvQueue(t) + if got != tc.want { + t.Errorf("Timer enqueued %q; want %q", got, tc.want) + } + }) + } +} + +func TestGauge_WireFormat(t *testing.T) { + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + + cases := []struct { + name string + metric string + value int + tags []string + want string + }{ + { + name: "single_tag", + metric: "connectivity.confidence", + value: 7, + tags: []string{"dest_host:example.com"}, + want: "connectivity.confidence:7|g|#dest_host:example.com", + }, + { + name: "multiple_tags", + metric: "g", + value: 10, + tags: []string{"a:1", "b:2", "c:3"}, + want: "g:10|g|#a:1,b:2,c:3", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + Gauge(tc.metric, tc.value, tc.tags) + got := recvQueue(t) + if got != tc.want { + t.Errorf("Gauge enqueued %q; want %q", got, tc.want) + } + }) + } +} + +// TestCount_TagSeparatorIsComma verifies the dogstatsd contract that tags are +// comma-separated after `#`. A regression to space-separated tags would still +// look syntactically plausible in logs but silently drop tag parsing at the +// collector. +func TestCount_TagSeparatorIsComma(t *testing.T) { + t.Cleanup(func() { drainQueue(t) }) + drainQueue(t) + + Count("m", 1, []string{"a:1", "b:2"}) + got := recvQueue(t) + if !strings.Contains(got, "#a:1,b:2") { + t.Errorf("Count enqueued %q; want it to contain %q", got, "#a:1,b:2") + } +}