Move test demo apps out of Bazel to ease Go version deprecation and support testing offsetgen instrumentation #2217
Conversation
… for offsetgen testing Signed-off-by: Dom Del Nano <[email protected]>
| "@com_github_sirupsen_logrus//:logrus", | ||
| "@com_github_spf13_pflag//:pflag", | ||
| "@com_github_spf13_viper//:viper", |
There was a problem hiding this comment.
These were removed to make the Dockerfile logic as simple as possible. By removing additional dependencies, the Dockerfile only needs to run a single go get (two in the case of go_grpc_tls_pl)
| COPY greetpb greetpb | ||
| RUN go mod init px.dev/pixie/src/stirling/testing/demo_apps/go_grpc_tls_pl/server && \ | ||
| go get google.golang.org/grpc@${GOOGLE_GOLANG_GRPC} && \ | ||
| go get github.com/gogo/protobuf/proto && \ |
There was a problem hiding this comment.
This could be removed if the greetpb package didn't use gogo. I tried to make this change, but it appears that Pixie's go_proto_library hardcodes the gogo compiler.
Since gogo is deprecated, my assumption is that this won't break for older go versions (go get always fetches the newest version and won't install an older, compatible version)
| "@com_github_spf13_pflag//:pflag", | ||
| "@com_github_spf13_viper//:viper", |
There was a problem hiding this comment.
As mentioned above, any non essential packages were removed to make the Dockerfile logic as simple as possible.
| declare -A GO_VERSIONS=( | ||
| ["1.18"]="v0.35.0" | ||
| ["1.19"]="v0.35.0" | ||
| ["1.20"]="v0.35.0" | ||
| ["1.21"]="v0.35.0" | ||
| ["1.22"]="v0.35.0" | ||
| ) |
There was a problem hiding this comment.
The bulk of this shell script logic could be refactored to a single file. That would allow us to consolidate the go version to dependency mapping for both golang.org/x/net and google.golang.org/grpc. I wanted to get some feedback on this direction before doing so, but thought that would be something to include in this PR.
ddelnano
force-pushed
the
ddelnano/add-go-offsetgen-compatible-images
branch
from
2117bfa to
6e4f8c0
Compare
…ns and for offsetgen testing Signed-off-by: Dom Del Nano <[email protected]>
ddelnano
force-pushed
the
ddelnano/add-go-offsetgen-compatible-images
branch
from
6e4f8c0 to
f7355b6
Compare
src/stirling/testing/demo_apps/go_grpc_tls_pl/server/Dockerfile
Outdated
Show resolved
Hide resolved
src/stirling/testing/demo_apps/go_grpc_tls_pl/server/Dockerfile
Outdated
Show resolved
Hide resolved
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Signed-off-by: Dom Del Nano <[email protected]>
Summary: Remove unused bazel go sdks in favor of docker build images #2217 introduced a mechanism for building stirling's test demo apps outside of bazel. This allows us to decouple our test build dependencies from Pixie's own Go dependencies and also aids in testing the upcoming low memory Go uprobes (see that PR for a more detailed description). Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Existing tests pass --------- Signed-off-by: Dom Del Nano <[email protected]>
3 participants
Summary: Move test demo apps out of Bazel to ease Go version deprecation and support testing offsetgen instrumentation
This change is intended to help in two areas. The first is to decouple stirling's go version test matrix from Pixie's Go build. Our test suite currently builds all its go binaries with bazel, which means Pixie's go dependencies must accommodate all go versions used in our tests. This slows down upgrading our dependencies and go version.
The second is that in order to facilitate more memory efficient Go uprobes. I've implemented an opentelemetry-go-instrumentation offsetgen based version in #2207, #2212, pixie-io/opentelemetry-go-instrumentation#1. This instrumentation works by computing Go struct and function argument offsets ahead of time (avoiding expensive DWARF parsing at runtime) and applying the correct offset based on a binary's buildinfo. Unfortunately
rules_godoes not yet support populating the buildinfo section like the standard Go toolchain. We can eventually switch to this once it's implemented (bazel-contrib/rules_go#3090), but until then these images can serve an additional purpose.Relevant Issues: N/A
Type of change: /kind feature
Test Plan: Verified the following:
http2_trace_bpf_testandgo_tls_trace_bpf_test. These work for the existing DWARF based uprobes and the offsetgen based ones