Repository navigation
fix(phpfpm): emit request duration as a number, not a pointer address - #23
Merged
Merged
Conversation
requestDuration.MarshalJSON called fmt.Sprint on the *pointer* receiver rather than the value. fmt formats a pointer to a non-struct as its address, so every process entry carried something like "request duration":0xc000129a48 and the whole document produced by `get --out json` failed to parse. The sibling timestamp.MarshalJSON dereferences before formatting, which is why only this one was wrong. Both now use strconv.FormatInt on the value, so the two are symmetric and the wrong form cannot be copied back from its neighbour. That also leaves fmt unused in the file. Verified through the public API from a consuming module: a status payload unmarshalled into a Pool and re-marshalled now yields "request duration":295 and round-trips.
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
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.
Problem
php-fpm_exporter get --out jsonproduced output that does not parse. Every process entry carried a pointer address where a number belonged:requestDuration.MarshalJSONcalledfmt.Sprinton its pointer receiver:fmtformats a pointer to a non-struct type as its address. The siblingtimestamp.MarshalJSONdereferences first (time.Time(*t)), which is why only this one was affected.It is reachable:
cmd/get.gocallsjson.Marshal(pm), and althoughpmitself is not addressable,pm.Poolsis a slice — slice elements are addressable through the backing array, soencoding/jsondoes reach the pointer-receiver method on the nested process fields.Fix
Both methods now use
strconv.FormatInton the value. Keeping them symmetric matters here: the bug was one method drifting from its neighbour, and identical shape makes the wrong form harder to reintroduce.fmtis no longer used in the file.Tests
Written failing first. Before the fix:
TestRequestDurationMarshalJSONEmitsTheNumber— typical, zero and large valuesTestPoolManagerMarshalsToValidJSON— marshals a populatedPoolManagerthe wayget --out jsondoes, then unmarshals it back, asserting bothrequest durationandstart timesurvive. This covers the addressability path described above, so a regression in either method is caught.Verification
End-to-end from a separate consuming module using only the public API, unmarshalling a realistic PHP-FPM status payload into a
Pooland re-marshalling it:go test ./... -racepasses.golangci-lint(v2.12.2, as CI pins) reports 5 issues, unchanged from master, none new.Scope
This is upstream code and the bug predates the fork. Only the two
MarshalJSONmethods and the now-unusedfmtimport changed; no metric, label or scraping behaviour is touched.