otel: remove deprecated usages of otelgrpc#47919
Conversation
laurazard
force-pushed
the
fix-deprecated-otel
branch
2 times, most recently
from
62e281f to
1c39157
Compare
thaJeztah
added
status/2-code-review
kind/refactor
| "github.com/moby/buildkit/util/stack" | ||
| "github.com/moby/buildkit/util/tracing" |
There was a problem hiding this comment.
We were already depending on these, correct? (just making sure we want to couple more or less on BuildKit here)
I don't think any of this would end up on the client-side, but just in case, we should also do a test "vendor" PR from this branch in docker/cli to make sure we don't reintroduce BuildKit as a dependency there.
There was a problem hiding this comment.
For sure. This is still drafted, it was just a first pass, so I'll make sure of that before I mark the PR as ready.
There was a problem hiding this comment.
Yeah, I should've waited until no longer draft; just peeking (happy to see you're working on this!)
There was a problem hiding this comment.
We already depended on these, and I don't think these changes should end up in the CLI due to where they happened, but can do a test vendor PR there to confirm.
| "github.com/docker/docker/pkg/pidfile" | ||
| "github.com/docker/docker/pkg/process" | ||
| "github.com/docker/docker/pkg/system" | ||
| "github.com/moby/buildkit/util/grpcerrors" |
There was a problem hiding this comment.
Also /cc @dmcgowan who's looking at unifying some error-handling code and types (just in case any of this relates to that)
laurazard
force-pushed
the
fix-deprecated-otel
branch
5 times, most recently
from
58bb0a5 to
aded766
Compare
|
@jsternberg I'd appreciate a look from you as well |
Signed-off-by: Laura Brehm <[email protected]>
Signed-off-by: Laura Brehm <[email protected]>
laurazard
force-pushed
the
fix-deprecated-otel
branch
from
aded766 to
854130e
Compare
|
I need to amend this, there's a missing call to |
CI looks happy though, or doesn't it reproduce in CI? 😅 |
|
Nevermind 😅 There was a failing test before, so I assumed from the changes that it was that, but if CI's green I'm happy. |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
let's bring this one in 👍 Thanks!!
3 participants
- What I did
- How I did it
Based a lot off of the work in Buildkit: moby/buildkit#4753
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)