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

remove legacy grpc receiver endpoint for GA release #3101

Closed

Conversation

bhautikpip
Copy link
Contributor

@bhautikpip bhautikpip commented May 5, 2021

Description: <Describe what has changed.
Removed OTLP receiver legacy endpoint which was still accepting data even after we switched to port 4317 for easy migration. Submitted a PR to remove legacy endpoint for GA release. Not sure when is the good time to merge this PR but thought this will be required before GA release.

Link to tracking Issue: #2565

Testing: Ran otlpreceiver package unit tests by running
go test -race go.opentelemetry.io/collector/receiver/otlpreceiver command

Documentation: < Describe the documentation added.>

Please delete paragraphs that you did not use before submitting.

@bhautikpip bhautikpip requested a review from a team May 5, 2021 09:26
@jpkrohling
Copy link
Member

Do we really want to do it before GA? There are so many tools, blog posts, and docs out there referencing this port that users will be frustrated when they follow those and things don't work as expected.

@bhautikpip
Copy link
Contributor Author

bhautikpip commented May 5, 2021

Do we really want to do it before GA? There are so many tools, blog posts, and docs out there referencing this port that users will be frustrated when they follow those and things don't work as expected.

In my opinion if we expect this change will break user's set up(in case if they are still using legacy port 55680) then might as well do it in GA release. Not sure what will change if we merge this after GA. I definitely agree on changing the documentation whenever we plan to merge this PR.

@jpkrohling
Copy link
Member

There are things we don't control, like external blog posts. I would rather release with silent support for the legacy port, and log a big warning if we see data getting in via the legacy port.

@bhautikpip
Copy link
Contributor Author

@bogdandrutu what is the best path forward for this issue? Not sure if there have been any discussions happening on this.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@mxiamxia
Copy link
Member

mxiamxia commented May 21, 2021

This PR can be closed and a new one - #3256 created for removing all the occurrences of the legacy port

@bogdandrutu
Copy link
Member

@mxiamxia have you created a tracking PR for the remove of the port as we discussed after some more releases?

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 3, 2021
@bogdandrutu bogdandrutu removed the Stale label Jun 7, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@mxiamxia
Copy link
Member

@mxiamxia have you created a tracking PR for the remove of the port as we discussed after some more releases?

Yes, created a tracking issue - #3368

I'll send code change for deprecating the legacy host when we're ready for v0.30.0 release.

@mxiamxia
Copy link
Member

@bhautikpip @bogdandrutu , pls close this PR.

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

Successfully merging this pull request may close these issues.

5 participants