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

Change client.port from recommended to opt-in on HTTP server spans #472

Merged
merged 4 commits into from
Oct 31, 2023
Merged

Change client.port from recommended to opt-in on HTTP server spans #472

merged 4 commits into from
Oct 31, 2023

Conversation

trask
Copy link
Member

@trask trask commented Oct 29, 2023

Fixes #470

Changes

Changes client.port from recommended to opt-in on HTTP server spans.

Merge requirement checklist

@trask trask force-pushed the client-port-opt-in branch from 7e1f585 to f4a6731 Compare October 29, 2023 16:56
@trask trask marked this pull request as ready for review October 29, 2023 16:57
@trask trask requested review from a team October 29, 2023 16:57
joaopgrassi
joaopgrassi approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@joaopgrassi joaopgrassi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There are some warnings in http.yaml
image

@trask
Copy link
Member Author

trask commented Oct 30, 2023 •
edited
Loading

There are some warnings in http.yaml

Those warnings are in a file (model/registry/http.yaml) that isn't changed in this PR

EDIT sent #475 to fix the warning

This was referenced Oct 30, 2023
Merged
Merged
@AlexanderWert AlexanderWert merged commit 3b76932 into open-telemetry:main Oct 31, 2023
8 checks passed
@trask trask mentioned this pull request Oct 31, 2023
Closed
4 tasks
@mateuszrzeszutek mateuszrzeszutek mentioned this pull request Oct 31, 2023
Merged
@trask trask deleted the client-port-opt-in branch October 14, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mateuszrzeszutek mateuszrzeszutek mateuszrzeszutek approved these changes

@joaopgrassi joaopgrassi joaopgrassi approved these changes

@lmolkova lmolkova lmolkova approved these changes

@AlexanderWert AlexanderWert AlexanderWert approved these changes

Assignees

@reyang reyang

Labels
None yet
Projects
Semantic Conventions Triage
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should client.port be changed to opt-in on HTTP server spans?
6 participants
@trask @AlexanderWert @lmolkova @joaopgrassi @mateuszrzeszutek @reyang