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.

Sign up for GitHub

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

Jump to bottom

[otelmongo] should use AttributeNetPeerIP when hostname is a IP #2165

Open
jixiuf opened this issue Apr 7, 2022 · 8 comments · May be fixed by #2182
Open

[otelmongo] should use AttributeNetPeerIP when hostname is a IP #2165

jixiuf opened this issue Apr 7, 2022 · 8 comments · May be fixed by #2182
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelmongo

Comments

@jixiuf
Copy link

jixiuf commented Apr 7, 2022 •
edited by MrAlias
Loading

opentelemetry-go-contrib/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go

Line 54 in 1e34aa3

semconv.NetPeerNameKey.String(hostname),

should use semconv.NetPeerIPKey when hostname is a IP
@jixiuf jixiuf added area: instrumentation Related to an instrumentation package enhancement New feature or request labels Apr 7, 2022
@MrAlias MrAlias added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 7, 2022
@DiptoChakrabarty
Copy link

Hi @jixiuf can I take this up please

@DiptoChakrabarty DiptoChakrabarty linked a pull request Apr 13, 2022 that will close this issue
Open
@alamzeeshan
Copy link

@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
Copy link

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

@dmathieu
Copy link
Member

dmathieu commented Sep 7, 2023

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

@carrbs
Copy link
Contributor

carrbs commented Dec 7, 2023

@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

@pellared
Copy link
Member

pellared commented Dec 7, 2023 •
edited
Loading

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 pellared removed good first issue Good for newcomers help wanted Extra attention is needed labels Dec 7, 2023
@pellared
Copy link
Member

pellared commented Dec 7, 2023

Blocked by #4678 (no codeowners).

@prestonvasquez
Copy link
Contributor

@pellared This issue can be closed as if #6172 is accepted. semconv/v1.26.0 deprecates net.peer.ip in favor of network.peer.address.

@prestonvasquez prestonvasquez mentioned this issue Oct 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelmongo
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

otelmongo: set attribute based on hostname or IP address DiptoChakrabarty/opentelemetry-go-contrib
8 participants
@dmathieu @carrbs @jixiuf @pellared @MrAlias @alamzeeshan @prestonvasquez @DiptoChakrabarty