Skip to content

Add support for local and remote addresses on the server for child channels when UDS#13323

Merged
normanmaurer merged 1 commit intonetty:4.1from
violetagg:uds-addresses
Apr 19, 2023
Merged

Add support for local and remote addresses on the server for child channels when UDS#13323
normanmaurer merged 1 commit intonetty:4.1from
violetagg:uds-addresses

Conversation

@violetagg
Copy link
Copy Markdown
Member

Motivation:
Local and remote addresses are not available on the server for child channels when Unix Domain Sockets is used.
There are use cases where one may need this information in order to collect metrics per local or remote addresses.

Modifications:

  • Add JNI code for obtaining local/remote address
  • When creating a child channel initialise the local/remote addresses.
  • Add junit tests

Result:
Local and remote addresses are now available on the server for child channels when Unix Domain Sockets is used.

…annels when UDS

Motivation:
Local and remote addresses are not available on the server for child channels when
Unix Domain Sockets is used. There are use cases where one may need this information
in order to collect metrics per local or remote addresses.

Modifications:
- Add JNI code for obtaining local/remote address
- When creating a child channel initialise the local/remote addresses.
- Add junit tests

Result:
Local and remote addresses are now available on the server for child channels when
Unix Domain Sockets is used.
}

static jbyteArray netty_unix_socket_remoteDomainSocketAddress(JNIEnv* env, jclass clazz, jint fd) {
struct sockaddr_storage addr;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something I just noticed (we can fix this in another PR; affects other functions as well) is that we should probably initialize addr to zero, since we don't pass along the len output, so strlen will end up reading uninitialized memory.

@normanmaurer normanmaurer merged commit 79af2f2 into netty:4.1 Apr 19, 2023
@normanmaurer normanmaurer added this to the 4.1.92.Final milestone Apr 19, 2023
normanmaurer added a commit that referenced this pull request Apr 19, 2023
…annels when UDS (#13323)

Motivation:
Local and remote addresses are not available on the server for child channels when
Unix Domain Sockets is used. There are use cases where one may need this information
in order to collect metrics per local or remote addresses.

Modifications:
- Add JNI code for obtaining local/remote address
- When creating a child channel initialise the local/remote addresses.
- Add junit tests

Result:
Local and remote addresses are now available on the server for child channels when
Unix Domain Sockets is used.

Co-Authored-by: Norman Maurer <norman_maurer@apple.com>
normanmaurer added a commit that referenced this pull request Apr 21, 2023
#13343)

* Add support for local and remote addresses on the server for child channels when UDS (#13323)

Motivation:
Local and remote addresses are not available on the server for child channels when
Unix Domain Sockets is used. There are use cases where one may need this information
in order to collect metrics per local or remote addresses.

Modifications:
- Add JNI code for obtaining local/remote address
- When creating a child channel initialise the local/remote addresses.
- Add junit tests

Result:
Local and remote addresses are now available on the server for child channels when
Unix Domain Sockets is used.

Co-Authored-by: Norman Maurer <norman_maurer@apple.com>
Co-authored-by: Violeta Georgieva <violetag@vmware.com>
trustin added a commit to trustin/armeria that referenced this pull request Apr 24, 2023
Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to work around the issue where Netty's domain socket
  channel returns `null`
  - See: netty/netty#13323
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.
violetagg added a commit to reactor/reactor-netty that referenced this pull request Apr 26, 2023
- Exclude tests in Http2ConnectionInforTests with HOST header empty
netty/netty#13238
- Fix unix domain socket tests in HttpServerTests as local and remote addresses are now available
netty/netty#13323
@violetagg
Copy link
Copy Markdown
Member Author

@chrisvest @normanmaurer thanks

@violetagg violetagg deleted the uds-addresses branch April 26, 2023 12:40
violetagg added a commit to reactor/reactor-netty that referenced this pull request Apr 26, 2023
- Exclude tests in Http2ConnectionInforTests with HOST header empty
netty/netty#13238
- Fix unix domain socket tests in HttpServerTests as local and remote addresses are now available
netty/netty#13323
violetagg added a commit to reactor/reactor-netty that referenced this pull request Apr 26, 2023
- Exclude tests in Http2ConnectionInforTests with HOST header empty
netty/netty#13238
- Fix unix domain socket tests in HttpServerTests as local and remote addresses are now available
netty/netty#13323
trustin added a commit to trustin/armeria that referenced this pull request May 18, 2023
Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to handle the case whene Netty's domain socket
  channe returns `null`.
  - Also updated Netty to 4.1.92
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.

Add server-side Unix domain socket support

Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to work around the issue where Netty's domain socket
  channel returns `null`
  - See: netty/netty#13323
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.
trustin added a commit to trustin/armeria that referenced this pull request May 19, 2023
# This is the 1st commit message:

Add server-side Unix domain socket support

Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to work around the issue where Netty's domain socket
  channel returns `null`
  - See: netty/netty#13323
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.

# This is the commit message #2:

Fix leaks

# This is the commit message #3:

Update Netty to 4.1.92
trustin added a commit to trustin/armeria that referenced this pull request May 19, 2023
Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to work around the issue where Netty's domain socket
  channel returns `null`
  - See: netty/netty#13323
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.

Fix leaks

Update Netty to 4.1.92

Cache remote/localAddress / SocketAddress -> InetSocketAddress

Add client-side domain socket support

Lint

Windows

Formatting

Windows / Lint

Review comments
trustin added a commit to trustin/armeria that referenced this pull request May 20, 2023
Motivation:

In service mesh environment, it's quite common for a sidecar to
communicate with an app using a Unix domain socket. Armeria could be
used in such a scenarios if it is capable of serving requests on a Unix
domain socket.

Modifications:

- Added `DomainSocketAddress`
- Added `ServerPort.isDomainSocket()`
- Added various getters to `TransportType` and `TransportTypeProvider`
  so it provides the transport-specific information about domain socket
  classes and whether the transport supports domain sockets
- Added `ChannelUtil.localAddress()` and `remoteAddress()` and replace
  the direct invocation of `Channel.localAddress()` and
  `remoteAddress()` to work around the issue where Netty's domain socket
  channel returns `null`
  - See: netty/netty#13323
- Added `ChannelUtil.isTcpOption()` to determine whether the given
  `ChannelOption` is for TCP or not, so that a user doesn't get a WARN
  log when they use domain sockets
- Made `ServerRule` and `ServerExtension`not instantiate their default
  `WebClient`s lazily, so that a user can start a serer that listens
  only on a domain socket

Result:

- Armeria is now capable of serving requests from a Unix domain socket,
  making it more service-mesh-friendly.

Fix leaks

Update Netty to 4.1.92

Cache remote/localAddress / SocketAddress -> InetSocketAddress

Add client-side domain socket support

Lint

Windows

Formatting

Windows / Lint

Review comments

Remove an unused field
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