Skip to content
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

proposal: net: make TCPAddr support JSON serialization #23174

Closed
sothychan opened this issue Dec 19, 2017 · 4 comments
Closed

proposal: net: make TCPAddr support JSON serialization #23174

sothychan opened this issue Dec 19, 2017 · 4 comments

Comments

@sothychan
Copy link

sothychan commented Dec 19, 2017

What version of Go are you using (go version)?

1.9

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Mac OS X, amd64

What did you do?

The TCPAddr struct in the net package doesn't have the json struct tags so it doesn't get properly serialized for JSON when you want to add this information to the health check endpoint

What did you expect to see?

I expect the fields to be serialized into proper JSON

What did you see instead?

The fields weren't serialized properly

@dsnet dsnet changed the title net/tcpsock.go TCPAddr doesn't support JSON serialization proposal: net: make TCPAddr support JSON serialization Dec 19, 2017
@gopherbot gopherbot added this to the Proposal milestone Dec 19, 2017
@dsnet
Copy link
Member

dsnet commented Dec 19, 2017

No other standard library package has json field tags, so there doesn't seem to be any existing precedence to support this.

@flimzy
Copy link
Contributor

flimzy commented Dec 19, 2017

I don't know if this should actually be done, but if it is, would implementing the TextMarshaler interface (probably which just calls String()) not be a better solution? Then it would work for much more than just JSON--and it wouldn't produce cryptic JSON structs that only make sense in a Go context.

@bradfitz
Copy link
Contributor

I don't think we want to decide which style of JSON our users which to use for such low-level types.

I think you'll need to write your own wrapper types if you which to serialize these in the JSON style you prefer.

But more generally for this issue, maybe Go needs a policy of when we use JSON struct tags in the standard library and when we don't. https://golang.org/pkg/time/#Time.MarshalJSON seems pretty unoffensive just being a string in a super standard & neutral format. https://golang.org/pkg/math/big/#Int.MarshalJSON also seems basically fine. But that's all we have in the standard library really.

@sothychan
Copy link
Author

Thank you for the quick comments. Your guys' thoughts and suggestions make absolute sense. I do think it's better to implement the appropriate interfaces so that it doesn't bloat up the standard library.

@agl agl closed this as completed Dec 31, 2017
@golang golang locked and limited conversation to collaborators Dec 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants