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 default OTLP port number #1221

Merged
merged 2 commits into from
Nov 16, 2020
Merged

Change default OTLP port number #1221

merged 2 commits into from
Nov 16, 2020

Conversation

tigrannajaryan
Copy link
Member

Contributes to #1148

Note that a separate port is used for OTLP/HTTP for now. There is currently work
in progress to confirm that we can use the same port. Once we have the confirmation
I will update the spec again to use one port.

@tigrannajaryan tigrannajaryan requested review from a team November 10, 2020 17:17
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/otlp-portnum branch from cb01c3f to e92112e Compare November 10, 2020 17:17
arminru
arminru approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@arminru arminru left a comment

Choose a reason for hiding this comment

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

Please also update the default mentioned in specification/protocol/exporter.md#configuration-options and the examples below.

Should we add a compliance matrix entry for using the correct default port?

tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 10, 2020
@tigrannajaryan
Change default OTLP port number
c649b27 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 10, 2020
@tigrannajaryan
Change default OTLP port number
5a3bc77 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 10, 2020
@tigrannajaryan
Change default OTLP port number
d8b5a24 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 10, 2020
@tigrannajaryan
Change default OTLP port number
c72dd55 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 10, 2020
@tigrannajaryan
Change default OTLP port number
dbe5792 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
@tigrannajaryan tigrannajaryan mentioned this pull request Nov 10, 2020
Merged
@tigrannajaryan
Copy link
Member Author

Corresponding change in the Collector, allowing to receive on new and legacy ports simultaneously to ease the transition open-telemetry/opentelemetry-collector#2104

@tigrannajaryan
Copy link
Member Author

Please also update the default mentioned in specification/protocol/exporter.md#configuration-options and the examples below.

Done.

Should we add a compliance matrix entry for using the correct default port?

I added this to the CHANGELOG, hopefully it is sufficient.

@bogdandrutu
Copy link
Member

@tigrannajaryan is the number confirmed?

@tigrannajaryan
Copy link
Member Author

@tigrannajaryan is the number confirmed?

@mtwo submitted the request with IANA. It will take a while to get the confirmation. I think we should move forward with the assumption that it will be accepted. On the minor chance that it will be rejected and a different port number suggested we can update the spec again, we still have time.

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/otlp-portnum branch from ada6e24 to ca65854 Compare November 12, 2020 15:24
@carlosalberto
Copy link
Contributor

I think we should move forward with the assumption that it will be accepted. On the minor chance that it will be rejected and a different port number suggested we can update the spec again, we still have time.

+1

@carlosalberto
Copy link
Contributor

@open-telemetry/specs-approvers Anybody objects to going forward with this proposal, given that the request with IANA has been filled but it may take a while to get a confirmation? If not, I suggest merging early tomorrow or on Monday.

tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 12, 2020
@tigrannajaryan
Change default OTLP port number
a4a1a4a 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
@yurishkuro
Copy link
Member

sgtm

I actually care less about having an official IANA port than about not using ephemeral port, because the latter has a higher chance of causing instability at deployment.

@carlosalberto
Copy link
Contributor

@bogdandrutu Any opinion? ;)

tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 13, 2020
@tigrannajaryan
Change default OTLP port number
cfe828d 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
@bogdandrutu
Copy link
Member

SGTM, let's merge this.

bogdandrutu
bogdandrutu reviewed Nov 13, 2020
View reviewed changes
specification/protocol/otlp.md Outdated
@@ -445,7 +445,7 @@ connections SHOULD be configurable.

#### Default Port

The default network port for OTLP/HTTP is 55681. There is currently an [open
The default network port for OTLP/HTTP is 4318. There is currently an [open
Copy link
Member

Choose a reason for hiding this comment

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

Did we not say that we try to not have 2 ports?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping @tigrannajaryan

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The spec now lists the same port for both OTLP/gRPC and OTLP/HTTP.

@tigrannajaryan
Change default OTLP port number
e229c97 
Contributes to open-telemetry#1148

Note that a separate port is used for OTLP/HTTP for now. There is currently work
in progress to confirm that we can use the same port. Once we have the confirmation
I will update the spec again to use one port.
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/otlp-portnum branch from ca65854 to ba86535 Compare November 16, 2020 16:59
@carlosalberto
Copy link
Contributor

Since now we have the two-ports update, it looks we are ready to merge today, right? @tigrannajaryan @bogdandrutu

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/otlp-portnum branch from ba86535 to ac800f1 Compare November 16, 2020 19:05
@bogdandrutu
Copy link
Member

4 approves + author (TC member) - merging

@bogdandrutu bogdandrutu merged commit 4dd4ee0 into open-telemetry:master Nov 16, 2020
@jtescher jtescher mentioned this pull request Nov 16, 2020
Closed
@tigrannajaryan tigrannajaryan deleted the feature/tigran/otlp-portnum branch November 16, 2020 22:33
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 17, 2020
@tigrannajaryan
Change default OTLP port number
b5efd85 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-collector that referenced this pull request Nov 18, 2020
@tigrannajaryan
Change default OTLP/gRPC port number
2d8f1a8 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
@jkwatson jkwatson mentioned this pull request Nov 19, 2020
Closed
bogdandrutu pushed a commit to open-telemetry/opentelemetry-collector that referenced this pull request Nov 19, 2020
@tigrannajaryan
Change default OTLP/gRPC port number (#2104)
29859f2 
This implements specification change open-telemetry/opentelemetry-specification#1221

To make transition to new port numbers less painful OTLP receiver will
also accept data on the legacy port numbers when it is configured to
use the default endpoint. Users who use the default Collector config
can continue sending data to the legacy ports and have a graceful period
to update their senders to start sending to the new ports.

Note that OTLP/HTTP continues using a separate port number from OTLP/gRPC.
There is separate work in progress to use one port for both.
@jkwatson jkwatson mentioned this pull request Nov 20, 2020
Closed
@jufab jufab mentioned this pull request Dec 4, 2020
Merged
@srikanthccv srikanthccv mentioned this pull request Jan 9, 2021
Closed
TomRoSystems added a commit to TomRoSystems/opentelemetry-cpp that referenced this pull request Jan 19, 2021
@TomRoSystems
OTLP change default port number
0884e23 
From specification - open-telemetry/opentelemetry-specification#1221
This was referenced Jan 19, 2021
Merged
Merged
@cijothomas cijothomas mentioned this pull request Feb 2, 2021
Merged
3 tasks
@SeanHood SeanHood mentioned this pull request Mar 5, 2021
Merged
7 tasks
hughluo added a commit to hughluo/opentelemetry.io that referenced this pull request May 12, 2021
@hughluo
Change the default port for OLTP gRPC
e970042 
4317 is now the default port for OLTP gRPC, as open-telemetry/opentelemetry-specification#1221
@hughluo hughluo mentioned this pull request May 12, 2021
Closed
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
@tigrannajaryan
Change default OTLP port number (open-telemetry#1221)
19c839a 
* Change default OTLP port number

Contributes to open-telemetry#1148

Note that a separate port is used for OTLP/HTTP for now. There is currently work
in progress to confirm that we can use the same port. Once we have the confirmation
I will update the spec again to use one port.

* Address PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlosalberto carlosalberto carlosalberto approved these changes

@arminru arminru arminru approved these changes

@yurishkuro yurishkuro yurishkuro approved these changes

@bogdandrutu bogdandrutu bogdandrutu approved these changes

@codeboten codeboten codeboten approved these changes

Assignees

@carlosalberto carlosalberto

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tigrannajaryan @bogdandrutu @carlosalberto @yurishkuro @codeboten @arminru