Conversation
milas
requested review from
StefanScherer,
glours,
laurazard,
ndeloof,
nicksieger and
ulyssessouza
and removed request for
a team
|
|
||
| const maxUnixSocketPathSize = len(syscall.RawSockaddrUnix{}.Path) | ||
|
|
||
| func DialInMemory(ctx context.Context, addr string) (net.Conn, error) { |
There was a problem hiding this comment.
@thaJeztah I was thinking of PRing this to docker/go-connections later. Thoughts? It's inspired by the InmemSocket there
There was a problem hiding this comment.
@milas hmm. need to think about this; honest answer;
- there's an issue with the latest (main) version of
go-connectionsthat we still need to sort out (moby started failing when we tried to update it to latest) - there's been some discussion about that repository (mainly around the TLS config specified in that repository, but also because it became a bit of a grab-bag of utilities that are not very related), and we need to re-think if we want to preserve it.
I wonder if we should have something like https://github.com/docker/go-metrics (either to provide abstractions, or to have a shared implementation for some of this).
(all just quick blurbs from my side! haven't dug in deep into all of this, but perhaps something we should discuss with more people)
There was a problem hiding this comment.
Ahhh sorry, I should have clarified -- I was only referring to the DialInMemory function - the way that library is structured right now, you can:
- create a listener type that works across OSes (impls behind build tags)
- configure an HTTP transport across OSes
...but it does NOT expose a function to create a platform-agnostic in-memory net.Conn directly
(That said, yes -- the rest of the observability pieces should be shared! I purposefully put this in internal/tracing so it cannot become a dependency elsewhere & it doesn't depend on anything in the rest of the Compose code base. Will spin it out into its own repo/module later)
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## v2 #10526 +/- ##
==========================================
- Coverage 59.65% 59.01% -0.64%
==========================================
Files 107 111 +4
Lines 9515 9709 +194
==========================================
+ Hits 5676 5730 +54
- Misses 3257 3395 +138
- Partials 582 584 +2
☔ View full report in Codecov by Sentry. |
glours
left a comment
glours
left a comment
There was a problem hiding this comment.
Most of the new files need the Copyright header.
e2e tests checking the console output seem to not be happy, maybe there is a trailing character added by the otel configuration? 🤔
milas
force-pushed
the
otel-init
branch
2 times, most recently
from
39eae89 to
368e242
Compare
This is a bunch of OTEL initialization code. It's all in `internal/` because there are re-usable parts here, but Compose isn't the right spot. Once we've stabilized the interfaces a bit and the need arises, we can move it to a separate module. Currently, a single span is produced to wrap the root Compose command. Compose will respect the standard OTEL environment variables as well as OTEL metadata from the Docker context. Both can be used simultaneously. The latter is intended for local system integration and is restricted to Unix sockets / named pipes. None of this is enabled by default. It requires setting the `COMPOSE_EXPERIMENTAL_OTEL=1` environment variable to gate it during development. Signed-off-by: Milas Bowman <[email protected]>
5 participants
What I did
This is a bunch of OTEL initialization code. It's all in
internal/because there are re-usable parts here, but Compose isn't the right spot. Once we've stabilized the interfaces a bit and the need arises, we can move it to a separate module.Currently, a single span is produced to wrap the root Compose command.
Compose will respect the standard OTEL environment variables as well as OTEL metadata from the Docker context. Both can be used simultaneously. The latter is intended for local system integration and is restricted to Unix sockets / named pipes.
None of this is enabled by default. It requires setting the
COMPOSE_EXPERIMENTAL_OTEL=1environment variable to gate it during development.(not mandatory) A picture of a cute animal, if possible in relation to what you did
