-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
expvar: document what expvar.Var's String() interface method requires you to return #15088
Comments
Fixed by https://go-review.googlesource.com/#/c/21525/ ( e8f01e5 ) |
Is it worth explicitly mentioning that this is not necessarily compatible with fmt.Stringer and thus common .String() usage (either in the Var comments or in general package comments)? One can work it out from the current documentation for String(), but I'm not sure it's going to be clear or obvious to people. (Specifically, I'm not sure I'd have realized that eg time.Time was not compatible from this documentation alone. But on the other hand, the Go documentation style tends towards terse.) |
Or maybe it can use it if it's valid JSON, else fall back to escaping it as a string. /cc @adg |
That seems like a fine solution to me. |
CL https://golang.org/cl/23068 mentions this issue. |
CL https://golang.org/cl/23232 mentions this issue. |
Please answer these questions before submitting your issue. Thanks!
go version
)?go version devel +b836fe6 Sat Apr 2 18:25:27 2016 -0400 linux/amd64
go env
)?linux/amd64
Currently, the expvar.Var interface looks like it is the well known fmt.Stringer interface; both have a single String() method in them. However, this is not the case, because expvar.Var's String() requires you to return a proper JSON object, with the correct quoting and all. This means that not all fmt.Stringer methods will do; for instance, time.Time's String() method returns an unquoted string that will cause JSON parsing failures. That means that naively writing something like:
will cause mysterious downstream failures in things that attempt to consume expvar variables (because time.Time returns an unquoted string, which then winds up in the JSON for /debug/vars).
This programmer mistake could be somewhat avoided if the expvar documentation noted the JSON formatting requirement and also that String() functions in other packages that are intended to satisfy fmt.Stringer will often not work to satisfy expvar.Var.
The text was updated successfully, but these errors were encountered: