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

Updating Sockpeer service. #7888

Merged
merged 17 commits into from
Apr 28, 2023
Merged

Conversation

rahuldimri
Copy link
Contributor

No description provided.

@rahuldimri rahuldimri requested a review from a team February 23, 2023 08:54
@mateuszrzeszutek
Copy link
Member

Hey @rahuldimri ,
Can you make sure the code is properly formatted and add some tests covering the new path?

@rahuldimri
Copy link
Contributor Author

Working on this.

@mateuszrzeszutek mateuszrzeszutek dismissed their stale review March 28, 2023 10:46

PR does not compile

@trask trask added the needs author feedback Waiting for additional feedback from the author label Apr 13, 2023
…trumentation/api/instrumenter/net/PeerServiceAttributesExtractor.java

Co-authored-by: Mateusz Rzeszutek <[email protected]>
@github-actions
Copy link
Contributor

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@github-actions github-actions bot added the stale label Apr 24, 2023
Copy link
Contributor Author

@rahuldimri rahuldimri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing the }

@github-actions github-actions bot removed the stale label Apr 27, 2023
rahuldimri and others added 5 commits April 27, 2023 14:14
…trumentation/api/instrumenter/net/PeerServiceAttributesExtractor.java

Co-authored-by: Mateusz Rzeszutek <[email protected]>
…trumentation/api/instrumenter/net/PeerServiceAttributesExtractorTest.java

Co-authored-by: Mateusz Rzeszutek <[email protected]>
…trumentation/api/instrumenter/net/PeerServiceAttributesExtractorTest.java

Co-authored-by: Mateusz Rzeszutek <[email protected]>
Copy link
Member

@mateuszrzeszutek mateuszrzeszutek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rahuldimri !

@mateuszrzeszutek mateuszrzeszutek removed the needs author feedback Waiting for additional feedback from the author label Apr 28, 2023
@mateuszrzeszutek mateuszrzeszutek merged commit 26a00e0 into open-telemetry:main Apr 28, 2023
@rahuldimri
Copy link
Contributor Author

@mateuszrzeszutek thanks for your support in this PR. Do you mind to add labels on this issue ?.

@mateuszrzeszutek
Copy link
Member

I'm not sure I'm following; what labels do you have in mind?

@trask
Copy link
Member

trask commented Apr 28, 2023

hey @rahuldimri, could you elaborate on why you needed this? Is net.peer.name not being populated by some instrumentation? thx!

@trask
Copy link
Member

trask commented May 8, 2023

@rahuldimri fyi, I suspect the need for this will go away with open-telemetry/opentelemetry-specification#3402, where it's clarified that we should be populating (the equivalent of) net.peer.name, and we should only be populating (the equivalent of) net.sock.peer.name when going through an intermediary (e.g. proxy server)

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

Successfully merging this pull request may close these issues.

3 participants