Skip to content

Commit

Permalink
fix(grpc): Allow gRPC connections via Unix socket (#1833)
Browse files Browse the repository at this point in the history
* fix(grpc): Allow gRPC connections via Unix socket

This commit addresses issue #1832.

The way `NET_PEER_IP` and `NET_PEER_PORT` are retrieved raises a `ValueError`
when gRPC connections are handled via Unix sockets.

```py
ip, port = (
    context.peer().split(",")[0].split(":", 1)[1].rsplit(":", 1)
)
```

When using an address like `unix:///tmp/grpc.sock` the value of `context.peer()` is `"unix:"`.
Substituting that in the function above...

```py
ip, port = "unix:".split(",")[0].split(":", 1)[1].rsplit(":", 1)
ip, port = ["unix:"][0].split(":", 1)[1].rsplit(":", 1)
ip, port = "unix:".split(":", 1)[1].rsplit(":", 1)
ip, port = ["unix", ""][1].rsplit(":", 1)
ip, port = "".rsplit(":", 1)
ip, port = [""]  # ValueError
```

I "addressed" the issue by guarding the retrieval of `net.peer.*` values under
an `if` statement that checks if we are using a Unix socket.

I extended the `server_interceptor` tests to run against TCP and Unix socket configurations.

---

**Open Questions**

- [ ] The socket tests will fail on Windows. Is there a way to annotate that?
- [ ] Are there other span values we should be setting for the unix socket?

* Update CHANGELOG

* Add placeholder attributes for linter

* fix lint

---------

Co-authored-by: Matt Oberle <[email protected]>
Co-authored-by: Shalev Roda <[email protected]>
  • Loading branch information
3 people authored Jun 21, 2023
1 parent ffc9334 commit 32ae65e
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 121 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1679))
- `opentelemetry-instrumentation-asgi` Add `http.server.response.size` metric
([#1789](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1789))
- `opentelemetry-instrumentation-grpc` Allow gRPC connections via Unix socket
([#1833](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1833))

## Version 1.18.0/0.39b0 (2023-05-10)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,24 +250,30 @@ def _start_span(
# * ipv4:127.0.0.1:57284
# * ipv4:10.2.1.1:57284,127.0.0.1:57284
#
try:
ip, port = (
context.peer().split(",")[0].split(":", 1)[1].rsplit(":", 1)
)
ip = unquote(ip)
attributes.update(
{
SpanAttributes.NET_PEER_IP: ip,
SpanAttributes.NET_PEER_PORT: port,
}
)
if context.peer() != "unix:":
try:
ip, port = (
context.peer()
.split(",")[0]
.split(":", 1)[1]
.rsplit(":", 1)
)
ip = unquote(ip)
attributes.update(
{
SpanAttributes.NET_PEER_IP: ip,
SpanAttributes.NET_PEER_PORT: port,
}
)

# other telemetry sources add this, so we will too
if ip in ("[::1]", "127.0.0.1"):
attributes[SpanAttributes.NET_PEER_NAME] = "localhost"
# other telemetry sources add this, so we will too
if ip in ("[::1]", "127.0.0.1"):
attributes[SpanAttributes.NET_PEER_NAME] = "localhost"

except IndexError:
logger.warning("Failed to parse peer address '%s'", context.peer())
except IndexError:
logger.warning(
"Failed to parse peer address '%s'", context.peer()
)

return self._tracer.start_as_current_span(
name=handler_call_details.method,
Expand Down
Loading

0 comments on commit 32ae65e

Please sign in to comment.