Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Shall we change peer.ipv4 to String tag in golang impl? #154

Closed
NeoCN opened this issue Jul 27, 2017 · 2 comments
Closed

Shall we change peer.ipv4 to String tag in golang impl? #154

NeoCN opened this issue Jul 27, 2017 · 2 comments

Comments

@NeoCN
Copy link
Contributor

NeoCN commented Jul 27, 2017

I noticed that peer.ipv4 tag defined in ext package is uint32 type

// PeerHostIPv4 records IP v4 host address of the peer
PeerHostIPv4 = uint32TagName("peer.ipv4")

but , according to Standard Span tags and log fields, the peer.ipv4 tag is described as

-- -- --
peer.ipv4 string Remote IPv4 address as a .-separated tuple. E.g., "127.0.0.1"
-- -- --

So, follow the conventions or keep the current code?

@yurishkuro
Copy link
Member

we should not change the existing tag, it will be a breaking change that requires a new major version. We can add something like PeerHostIPv4Str with the same peer.ipv4 key but as a string tag, and add comments to PeerHostIPv4 to explain the reasoning.

@NeoCN are you ok to do a pull request?

@NeoCN
Copy link
Contributor Author

NeoCN commented Jul 27, 2017

@yurishkuro done!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants