-
Notifications
You must be signed in to change notification settings - Fork 838
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
Follow spec defined port 4318 for HTTP exporter #2395
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2395 +/- ##
=======================================
Coverage 92.78% 92.78%
=======================================
Files 145 145
Lines 5226 5226
Branches 1071 1071
=======================================
Hits 4849 4849
Misses 377 377 |
Should we hold off on this until the collector itself is updated? |
Looks like the collector PR to change the port has been merged. In terms of releases it looks like the collector does monthly releases in the first week on the month so the August one should be here soon? As long as users update their collector version quickly it won't be a problem. But we could be safe and let this sit for a while since the collector will still support the old |
97407c5
to
085ed57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general looks fine, but in the example we are still using version 0.25 which has been released in April - 4 months ago so I can assume that the example will not work anymore ?
@NathanielRN friendly ping |
Just a heads up that port number may still be in flux. Merged reverted PR: open-telemetry/opentelemetry-specification#1847 The draft PR with possible upcoming change: open-telemetry/opentelemetry-collector#3765 I’m sharing as I stumbled upon and seems related, but I’m not sure how it impacts all this. I’ll let y’all decide. |
Thanks for the heads up. We've been moving slowly with this change intentionally since we know the collector will continue to support the existing port for quite a long time so we have no real reason to force this change through until the dust is settled. |
I'll close this PR, the call out about the collector going back to 1 port was really useful! Agree that we can update when the dust has settled, and the old port will continue to work for a while! |
What do you mean going back to 1 port? |
@dyladan in open-telemetry/opentelemetry-specification#1847 which @longility posted above they said
Originally they had said they need to use port |
Which problem is this PR solving?
Follow up to #2331 , the specifications just updated the HTTP port to be
4318
in this PR: open-telemetry/opentelemetry-specification#1839Likewise, the Collector also plans to updated from
55681
to4318
, but will support both for some time.Short description of the changes
4318