bench: skip non-finite metric samples when parsing exposition output

[anxkhn]

What's wrong

parseExposition in internal/bench/scrape.go reads each sample value with strconv.ParseFloat and skips the line only when parsing fails. The comment right below the check says NaN and infinity carry no information and should be dropped, but strconv.ParseFloat happily accepts NaN, +Inf and -Inf with no error, so those values are kept as series samples.

That matters at the end of a run, not during parsing. Snapshot.Flatten copies the samples into a map[string]float64, cmd/sam-bench embeds that map in the observation, and write hands it to json.MarshalIndent, which returns an UnsupportedValueError for non-finite floats. The practical effect: if any scraped endpoint emits a single non-finite sample (an unavailable summary quantile, for example), the whole observation fails to serialize and nothing gets written. The workload has already run at that point, so the finite metrics and the benchmark report are lost along with the one unsupported value.

The fix

internal/bench/scrape.go now rejects the sample when math.IsNaN(value) or math.IsInf(value, 0) in addition to the parse error. This matches the intent the existing comment already stated, and it drops the sample rather than substituting zero, so an unavailable measurement never turns into a misleading data point. The comment was reworded to say "infinity" since negative infinity is covered too. Standard library only, no new dependencies.

Two tests come with it:

• TestParseExpositionNonFinite in internal/bench/scrape_test.go feeds NaN, +Inf and -Inf alongside 3, 0 and -2.5, checks that only the three finite samples survive Flatten, and confirms json.Marshal succeeds. Zero and an ordinary negative are included on purpose so the filter can't quietly widen.
• TestWriteObservationWithNonFiniteMetrics in cmd/sam-bench/report_test.go serves the same body from an httptest server, runs scrapeAll, and calls write against a t.TempDir path. Before the fix write returns an error and leaves no file behind; now it writes valid JSON containing both the report and finite=3.

Testing

go test ./internal/bench ./cmd/sam-bench -run 'TestParseExpositionNonFinite|TestWriteObservationWithNonFiniteMetrics' -count=1 -timeout=10s

Both packages pass.

[gemini-code-assist]

Code Review

This pull request introduces changes to filter out non-finite metrics (such as NaN and infinity) during scraping, ensuring only finite values are processed. It includes new unit and integration tests to verify this behavior. The review feedback notes that the new integration test in report_test.go uses an unbounded context, violating the repository style guide's requirement for time-bounded integration tests, and suggests using a context with a timeout instead.

[gemini-code-assist]

3. Test coverage must follow the pyramid

Import time to allow setting a timeout on the context used in the integration test, adhering to Rule 3.5 of the Repository Style Guide.

Suggested change

	"context"
	"encoding/json"
	"net/http"
	"net/http/httptest"
	"os"
	"path/filepath"
	"strings"
	"testing"
	"context"
	"encoding/json"
	"net/http"
	"net/http/httptest"
	"os"
	"path/filepath"
	"strings"
	"testing"
	"time"

References

1. Rule 3.5: Integration tests must be time bounded. Look for unbounded waits, time.Sleep longer than a few hundred milliseconds, or polling without a deadline. Prefer context.WithTimeout and t.Deadline(). ^(link)

[gemini-code-assist]

3. Test coverage must follow the pyramid

According to Rule 3.5 of the Repository Style Guide, integration tests must be time-bounded to avoid unbounded waits. Since scrapeAll performs HTTP requests, use a context with a timeout instead of context.Background().

Suggested change

	metrics, err := scrapeAll(context.Background(), []string{server.URL})
	if err != nil {
	t.Fatalf("scrapeAll: %v", err)
	}
	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancel()
	metrics, err := scrapeAll(ctx, []string{server.URL})
	if err != nil {
	t.Fatalf("scrapeAll: %v", err)
	}

References

1. Rule 3.5: Integration tests must be time bounded. Look for unbounded waits, time.Sleep longer than a few hundred milliseconds, or polling without a deadline. Prefer context.WithTimeout and t.Deadline(). ^(link)