Conversation
Replace FetchServerByID/FetchServers with FetchServerByIDContext/ FetchServerListContext so the caller's context deadline and cancellation propagates into server-fetch HTTP requests, not just the ping/download/upload test phase. Also add a defensive len(targets)==0 guard after FindServer, and defer s.Context.Reset() so DataManager snapshot state is always cleared on any return path (error or cancellation), preventing stale data bleeding into the next Run cycle. Update MockSpeedtestClient and all mock.On() call sites to match the new interface signatures.
Add a nil-result guard in Collect so a (nil, nil) return from Runner
no longer panics at the field access on line 89; emit zeroed metrics
with speedtest_up=0 instead.
Distinguish context.DeadlineExceeded from other errors in log output
('Speedtest timed out' vs 'Speedtest failed') to help operators tell
a collection timeout apart from a network failure.
Add 14 new tests across runner_test.go and collector_test.go: runner_test.go (8 tests): - ContextCancelledBefore/DeadlineBefore FetchServerByID and FetchServers: verify cancellation and timeout propagate end-to-end through the new context-aware interface methods - ContextForwardedTo FetchServerByID and FetchServers: assert the exact context identity is forwarded, not a detached background context collector_test.go (6 tests): - DeadlineExceededLogsTimedOut, WrappedDeadlineExceededLogsTimedOut: verify the new errors.Is branch fires for direct and wrapped deadlines - NonDeadlineErrorLogsFailed: verify non-deadline errors still log 'failed' - BothResultAndErrorTakesErrorPath: (result, err) both non-nil takes error path - ContextHasDeadline: Collect creates a deadline-bearing context before Run - NilResultNilErrorDoesNotPanic: (nil, nil) from runner emits zeroed metrics Also merge registerer_test.go into collector_test.go since RegisterSpeedtestCollector lives in collector.go; delete the orphan file.
- Fix SPEEDTEST_EXPORTER_DEBUG default: any non-empty string enables it (not a boolean false), matching the os.Getenv(...) != "" check in main.go - Add Unit column to metrics table (milliseconds / bits per second / dimensionless) - Document that no labels are present on any metric - Add verbatim example Prometheus exposition output - Name the debug collectors explicitly (go_* and process_*)
Add Makefile with targets: build, test, lint (golangci-lint with go vet fallback), run (respects SPEEDTEST_PORT/SPEEDTEST_SERVER/ SPEEDTEST_EXPORTER_DEBUG), clean, and help (default goal). Bump go directive from 1.25 to 1.26 to pick up security fixes in net/url, crypto/tls, html/template, and os included in the 1.26 line. The installed toolchain (go1.26.1) already satisfies this requirement.
v2.5.0 was built with Go 1.25 and refuses to lint modules targeting Go 1.26, causing the lint job to exit with code 3. v2.11.3 is the latest release and supports Go 1.26.
zakame
force-pushed
the
chore/prerelease
branch
from
240c240 to
97e36d1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-release audit and gap fixes identified by a full team review (Copilot baseline audit, Tester adversarial review, Language Lawyer, Program Clerk, Editor, Toolsmith).
Changes
refactor
SpeedtestClientinterface to context-aware fetch methods (FetchServerByIDContext,FetchServerListContext) so cancellation and deadlines propagate into server-fetch HTTP requests, not just the test phases.Context.Reset()so DataManager snapshot state is always cleared on any return path, preventing stale data bleeding into the next scrape cyclelen(targets) == 0guard afterFindServerfix
(nil, nil)return from Runner — previously panicked inCollect; now emits zeroed metrics withspeedtest_up=0context.DeadlineExceededfrom other errors in log output: "Speedtest timed out" vs "Speedtest failed"test
registerer_test.gointocollector_test.go(orphan filename — no matching source file); deleteregisterer_test.goDefaultCollectTimeoutis wired into the collector registered byNewSpeedtestRunner"Speedtest returned no result"is logged on(nil, nil)runner returnrunner_test.goreferencing old method names (FetchServerByID→FetchServerByIDContext,FetchServers→FetchServerListContext)docs
SPEEDTEST_EXPORTER_DEBUGdefault (any non-empty string, notfalse)build
Makefilewithbuild,test,cover,lint,vet,fmt,run,clean,helptargetsgodirective from 1.25 → 1.26 for security fixes innet/url,crypto/tls,html/template,osci
golangci-lintfrom v2.5.0 → v2.11.3; v2.5.0 was built with Go 1.25 and hard-rejects modules targeting Go 1.26