opentelemetry-go-contrib icon indicating copy to clipboard operation
opentelemetry-go-contrib copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

[otelmongo] should use `AttributeNetPeerIP` when hostname is a IP

Open jixiuf opened this issue 2 years ago • 8 comments

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/1e34aa359ff6cfe4ba2a44775d261311ea024197/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go#L54

should use semconv.NetPeerIPKey when hostname is a IP

jixiuf avatar Apr 07 '22 08:04 jixiuf

Hi @jixiuf can I take this up please

DiptoChakrabarty avatar Apr 12 '22 16:04 DiptoChakrabarty

@jixiuf This looks abandoned. Can I take this up? Please assign this to me. I will open a PR along with the relevant unit tests.

alamzeeshan avatar Aug 21 '23 06:08 alamzeeshan

@evantorrie @jmacd @XSAM Can I take this up?

alamzeeshan avatar Sep 07 '23 14:09 alamzeeshan

Feel free to open a PR and link this issue on it.

dmathieu avatar Sep 07 '23 15:09 dmathieu

@pellared, @dmathieu, I can pick this up since it looks abandoned, but a few questions:

  1. is this still needed?
  2. Based on what I see in the changelog, and the NOTE here it looks like we'd set NetSockPeerAddr if hostname is an IP address, otherwise set NetPeerName?
  3. which unit tests are you referring to?

https://github.com/open-telemetry/opentelemetry-specification/blob/bb3d0a0d14f41ccd3f78852316c77705e722edfa/CHANGELOG.md?plain=1#L1126-L1127

https://github.com/open-telemetry/opentelemetry-go/blob/d37d851bbce65577ab340849ec6c540fdc6e3096/semconv/v1.17.0/trace.go#L1115-L1117

carrbs avatar Dec 07 '23 00:12 carrbs

There is a plan to deprecate the module and move the ownership to MongoDB team.

I do not think it is worth to take the time in fix as there are no codeowners for this module so there is low chance that the PR will get a review.

Reference: https://jira.mongodb.org/browse/GODRIVER-2938

pellared avatar Dec 07 '23 13:12 pellared

Blocked by https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4678 (no codeowners).

pellared avatar Dec 07 '23 13:12 pellared