otelmongo: set attribute based on hostname or IP address #2182
Conversation
DiptoChakrabarty
requested review from
jmacd,
MrAlias,
Aneurysm9,
evantorrie,
XSAM,
dashpole,
paivagustavo,
MadVikingGod,
pellared and
hanyuancheung
as code owners
Codecov Report
@@ Coverage Diff @@
## main #2182 +/- ##
=======================================
+ Coverage 74.2% 75.7% +1.5%
=======================================
Files 144 145 +1
Lines 6556 6567 +11
=======================================
+ Hits 4867 4977 +110
+ Misses 1546 1442 -104
- Partials 143 148 +5
|
| @@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | |||
|
|
|||
| ### Fixed | |||
|
|
|||
| - Set attributes based on whether hostname provided is an IP Address or hostname (#2182) | |||
There was a problem hiding this comment.
Please reference the full package name that this change is related to.
| semconv.NetPeerPortKey.Int(port), | ||
| semconv.NetTransportTCP, | ||
| } | ||
|
|
||
| addr := net.ParseIP(hostname) |
There was a problem hiding this comment.
I wonder if this wouldn't be more efficient with a regex?
There was a problem hiding this comment.
I prefer net.ParseIP especially that it is already used in other places like here: https://github.com/open-telemetry/opentelemetry-go/blob/db7fd1bb51ce6ed1171cac15eeecb6871dbbb80a/semconv/internal/http.go#L123
|
|
||
| addr := net.ParseIP(hostname) | ||
| if addr != nil { | ||
| attrs = append(attrs, semconv.NetPeerIPKey.String(hostname)) |
There was a problem hiding this comment.
Can you please cover this scenario in the unit tests? It would be good to cover both IPv4 and IPv6.
pellared
changed the title
pellared
changed the title
|
@DiptoChakrabarty are you still able to work on this? It looks abandoned. If so, please address the feedback that has been given. If not, please close the PR. |
Fixes #2165
Set the appropriate attribute based on whether hostname is an Ip address or not