Remove go 1.16 and 1.17. Add Go 1.22 and 1.23 into dynamic tracer tes…

…t matrix (#2120)

Summary: Remove go 1.16 and 1.17. Add Go 1.22 and 1.23 into dynamic
tracer test matrix

Once Go 1.16 and 1.17 are removed from the repo, we can pursue go
dependency upgrades like #2115. I also added Go 1.22 and 1.23 (the
latest versions) to dynamic tracer tests that ideally should cover
multiple go versions.

Relevant Issues: N/A

Type of change: /kind cleanup

Test Plan: Existing tests

---------

Signed-off-by: Dom Del Nano <[email protected]>

WORKSPACE

@@ -211,16 +211,6 @@ load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
 gazelle_dependencies(go_sdk = "go_sdk")
 
 # Download alternative go toolchains after all other dependencies, so that they aren't used by external dependencies.
-go_download_sdk(
-    name = "go_sdk_1_16",
-    version = "1.16.15",
-)
-
-go_download_sdk(
-    name = "go_sdk_1_17",
-    version = "1.17.13",
-)
-
 go_download_sdk(
     name = "go_sdk_1_18",
     version = "1.18.10",

bazel/pl_build_system.bzl

@@ -24,7 +24,7 @@ load("@rules_python//python:defs.bzl", "py_test")
 load("//bazel:toolchain_transitions.bzl", "qemu_interactive_runner")
 
 pl_boringcrypto_go_sdk = ["1.20.13"]
-pl_supported_go_sdk_versions = ["1.16", "1.17", "1.18", "1.19", "1.20", "1.21", "1.22", "1.23"]
+pl_supported_go_sdk_versions = ["1.18", "1.19", "1.20", "1.21", "1.22", "1.23"]
 
 # The last version in this list corresponds to the boringcrypto go sdk version.
 pl_all_supported_go_sdk_versions = pl_supported_go_sdk_versions + pl_boringcrypto_go_sdk

src/stirling/e2e_tests/BUILD.bazel

@@ -88,14 +88,14 @@ pl_sh_bpf_test(
     srcs = ["stirling_wrapper_bpf_test.sh"],
     args = [
         "$(location //src/stirling/binaries:stirling_wrapper)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_19_grpc_client)",
         "-stirling_profiler_java_agent_libs %s" % agent_libs_arg,
     ],
     data = [
         "//src/stirling/binaries:stirling_wrapper",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_19_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs",
     ] + agent_libs,
     # TODO(PL-1462): Uprobe attaching sometimes fails.
     flaky = True,
@@ -132,13 +132,13 @@ pl_sh_bpf_test(
     srcs = ["stirling_wrapper_container_bpf_test.sh"],
     args = [
         "$(location //src/stirling/binaries:stirling_wrapper_image.tar)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_19_grpc_client)",
     ],
     data = [
         "//src/stirling/binaries:stirling_wrapper_image.tar",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_19_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs",
     ],
     flaky = True,
     tags = [

src/stirling/obj_tools/dwarf_reader_test.cc

@@ -34,6 +34,10 @@ constexpr std::string_view kTestGo1_20Binary =
     "src/stirling/obj_tools/testdata/go/test_go_1_20_binary";
 constexpr std::string_view kTestGo1_21Binary =
     "src/stirling/obj_tools/testdata/go/test_go_1_21_binary";
+constexpr std::string_view kTestGo1_22Binary =
+    "src/stirling/obj_tools/testdata/go/test_go_1_22_binary";
+constexpr std::string_view kTestGo1_23Binary =
+    "src/stirling/obj_tools/testdata/go/test_go_1_23_binary";
 constexpr std::string_view kGoGRPCServer =
     "src/stirling/testing/demo_apps/go_grpc_tls_pl/server/golang_1_19_grpc_tls_server_binary";
 constexpr std::string_view kCppBinary = "src/stirling/obj_tools/testdata/cc/test_exe_/test_exe";
@@ -46,6 +50,8 @@ const auto kGo1_18BinaryPath = px::testing::BazelRunfilePath(kTestGo1_18Binary);
 const auto kGo1_19BinaryPath = px::testing::BazelRunfilePath(kTestGo1_19Binary);
 const auto kGo1_20BinaryPath = px::testing::BazelRunfilePath(kTestGo1_20Binary);
 const auto kGo1_21BinaryPath = px::testing::BazelRunfilePath(kTestGo1_21Binary);
+const auto kGo1_22BinaryPath = px::testing::BazelRunfilePath(kTestGo1_22Binary);
+const auto kGo1_23BinaryPath = px::testing::BazelRunfilePath(kTestGo1_23Binary);
 const auto kGoServerBinaryPath = px::testing::BazelRunfilePath(kGoGRPCServer);
 const auto kGoBinaryUnconventionalPath = px::testing::BazelRunfilePath(kGoBinaryUnconventional);
 
@@ -572,7 +578,11 @@ INSTANTIATE_TEST_SUITE_P(GolangDwarfReaderParameterizedTest, GolangDwarfReaderTe
                                            DwarfReaderTestParam{kGo1_20BinaryPath, true},
                                            DwarfReaderTestParam{kGo1_20BinaryPath, false},
                                            DwarfReaderTestParam{kGo1_21BinaryPath, true},
-                                           DwarfReaderTestParam{kGo1_21BinaryPath, false}));
+                                           DwarfReaderTestParam{kGo1_21BinaryPath, false},
+                                           DwarfReaderTestParam{kGo1_22BinaryPath, true},
+                                           DwarfReaderTestParam{kGo1_22BinaryPath, false},
+                                           DwarfReaderTestParam{kGo1_23BinaryPath, true},
+                                           DwarfReaderTestParam{kGo1_23BinaryPath, false}));
 
 INSTANTIATE_TEST_SUITE_P(GolangDwarfReaderParameterizedIndexTest, GolangDwarfReaderIndexTest,
                          ::testing::Values(true, false));

src/stirling/obj_tools/testdata/go/BUILD.bazel

@@ -69,10 +69,17 @@ filegroup(
         # These older 32 bit binaries have been the source of bugs, so this test case verifies we don't
         # introduce a regression (https://github.com/pixie-io/pixie/issues/1300).
         "test_go1_13_i386_binary",
-        ":test_go_1_17_binary",
+        # This binary was built with go 1.17. This ensures that the 64 bit little endian case buildinfo logic is tested.
+        # (https://github.com/golang/go/blob/1dbbafc70fd3e2c284469ab3e0936c1bb56129f6/src/debug/buildinfo/buildinfo.go#L192-L208).
+        # Newer versions of go generate the endian agnostic buildinfo header
+        # (https://github.com/golang/go/blob/1dbbafc70fd3e2c284469ab3e0936c1bb56129f6/src/debug/buildinfo/buildinfo.go#L189-L190)
+        # and so it cannot be tested without compiling against an older Go version.
+        "test_go_1_17_binary",
         ":test_go_1_18_binary",
         ":test_go_1_19_binary",
         ":test_go_1_20_binary",
         ":test_go_1_21_binary",
+        ":test_go_1_22_binary",
+        ":test_go_1_23_binary",
     ],
 )

src/stirling/scripts/benchmark_stirling_wrapper_http2_tracing.sh

@@ -24,11 +24,11 @@ BAZEL_BUILD_CMD="bazel build --compilation_mode=opt"
 
 TEST_EXE_BASE_DIR="bazel-bin/src/stirling/source_connectors/socket_tracer/protocols/http2/testing"
 
-GRPC_CLIENT_LABEL="src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_17_grpc_client"
-GRPC_CLIENT_EXE="${TEST_EXE_BASE_DIR}/go_grpc_client/golang_1_17_grpc_client"
+GRPC_CLIENT_LABEL="src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_23_grpc_client"
+GRPC_CLIENT_EXE="${TEST_EXE_BASE_DIR}/go_grpc_client/golang_1_23_grpc_client"
 
-GRPC_SERVER_LABEL="src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_17_grpc_server"
-GRPC_SERVER_EXE="${TEST_EXE_BASE_DIR}/go_grpc_server/golang_1_17_grpc_server"
+GRPC_SERVER_LABEL="src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_23_grpc_server"
+GRPC_SERVER_EXE="${TEST_EXE_BASE_DIR}/go_grpc_server/golang_1_23_grpc_server"
 
 STIRLING_WRAPPER_LABEL="src/stirling/binaries:stirling_wrapper"
 STIRLING_WRAPPER_EXE="bazel-bin/src/stirling/binaries/stirling_wrapper"

src/stirling/source_connectors/dynamic_bpftrace/BUILD.bazel

@@ -38,8 +38,8 @@ pl_cc_bpf_test(
     name = "dynamic_bpftrace_connector_bpf_test",
     srcs = ["dynamic_bpftrace_connector_bpf_test.cc"],
     data = [
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_17_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_18_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_19_grpc_server_with_certs",
     ],
     tags = [
         "cpu:16",

src/stirling/source_connectors/dynamic_bpftrace/dynamic_bpftrace_connector_bpf_test.cc

@@ -462,23 +462,23 @@ TEST(DynamicBPFTraceConnectorTest, BPFTraceCheckPrintfsError) {
                HasSubstr("All printf statements must have exactly the same format string")));
 }
 
-constexpr std::string_view kServerPath_1_16 =
+constexpr std::string_view kServerPath_1_18 =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-    "golang_1_16_grpc_server";
-constexpr std::string_view kServerPath_1_17 =
+    "golang_1_18_grpc_server";
+constexpr std::string_view kServerPath_1_19 =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-    "golang_1_17_grpc_server";
+    "golang_1_19_grpc_server";
 
 TEST(DynamicBPFTraceConnectorTest, InsertUProbeTargetObjPaths) {
-  std::string go1_16_binary_path = px::testing::BazelRunfilePath(kServerPath_1_16).string();
-  std::string go1_17_binary_path = px::testing::BazelRunfilePath(kServerPath_1_17).string();
+  std::string go1_18_binary_path = px::testing::BazelRunfilePath(kServerPath_1_18).string();
+  std::string go1_19_binary_path = px::testing::BazelRunfilePath(kServerPath_1_19).string();
 
-  ASSERT_TRUE(fs::Exists(go1_16_binary_path));
-  ASSERT_TRUE(fs::Exists(go1_17_binary_path));
+  ASSERT_TRUE(fs::Exists(go1_18_binary_path));
+  ASSERT_TRUE(fs::Exists(go1_19_binary_path));
 
   DeploymentSpec spec;
-  spec.mutable_path_list()->add_paths(go1_16_binary_path);
-  spec.mutable_path_list()->add_paths(go1_17_binary_path);
+  spec.mutable_path_list()->add_paths(go1_18_binary_path);
+  spec.mutable_path_list()->add_paths(go1_19_binary_path);
 
   std::string uprobe_script =
       "// Deploys uprobes to trace http2 traffic.\n"
@@ -489,16 +489,16 @@ TEST(DynamicBPFTraceConnectorTest, InsertUProbeTargetObjPaths) {
   InsertUprobeTargetObjPaths(spec, &uprobe_script);
   EXPECT_EQ(
       uprobe_script,
-      absl::StrCat("// Deploys uprobes to trace http2 traffic.\n", "uprobe:", go1_16_binary_path,
+      absl::StrCat("// Deploys uprobes to trace http2 traffic.\n", "uprobe:", go1_18_binary_path,
                    ":\"golang.org/x/net/http2.(*Framer).WriteDataPadded\",\n"
                    "uprobe:",
-                   go1_17_binary_path,
+                   go1_19_binary_path,
                    ":\"golang.org/x/net/http2.(*Framer).WriteDataPadded\""
                    "{ printf(\"stream_id: %d, end_stream: %d\", arg0, arg1); }\n",
-                   "uretprobe:", go1_16_binary_path,
+                   "uretprobe:", go1_18_binary_path,
                    ":\"golang.org/x/net/http2.(*Framer).WriteDataPadded\",\n"
                    "uretprobe:",
-                   go1_17_binary_path,
+                   go1_19_binary_path,
                    ":\"golang.org/x/net/http2.(*Framer).WriteDataPadded\""
                    "{ printf(\"retval: %d\", retval); }"));
 }

src/stirling/source_connectors/dynamic_tracer/BUILD.bazel

@@ -48,10 +48,12 @@ pl_cc_bpf_test(
     timeout = "moderate",
     srcs = ["dynamic_trace_bpf_test.cc"],
     data = [
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
         "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_21_grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_22_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_23_grpc_client",
         "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_21_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_22_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_23_grpc_server_with_certs",
     ],
     tags = [
         "cpu:16",
@@ -72,6 +74,8 @@ pl_cc_bpf_test(
     data = [
         "//src/stirling/obj_tools/testdata/cc:test_exe",
         "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_22_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_23_binary",
         "//src/stirling/testing/dns:dns_hammer",
         "//src/stirling/testing/dns:dns_hammer_image.tar",
     ],

src/stirling/source_connectors/dynamic_tracer/dynamic_trace_bpf_test.cc

@@ -30,13 +30,27 @@
 
 #include "src/stirling/proto/stirling.pb.h"
 
-constexpr std::string_view kClientPath =
+constexpr std::string_view kGo1_21_ClientPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
     "golang_1_21_grpc_client";
-constexpr std::string_view kServerPath =
+constexpr std::string_view kGo1_21_ServerPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
     "golang_1_21_grpc_server";
 
+constexpr std::string_view kGo1_22_ClientPath =
+    "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
+    "golang_1_22_grpc_client";
+constexpr std::string_view kGo1_22_ServerPath =
+    "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
+    "golang_1_22_grpc_server";
+
+constexpr std::string_view kGo1_23_ClientPath =
+    "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
+    "golang_1_23_grpc_client";
+constexpr std::string_view kGo1_23_ServerPath =
+    "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
+    "golang_1_23_grpc_server";
+
 DECLARE_bool(debug_dt_pipeline);
 namespace px {
 namespace stirling {
@@ -54,11 +68,12 @@ using ::testing::StrEq;
 using LogicalProgram = ::px::stirling::dynamic_tracing::ir::logical::TracepointDeployment;
 
 // TODO(yzhao): Create test fixture that wraps the test binaries.
-class GoHTTPDynamicTraceTest : public ::testing::Test {
+class GoHTTPDynamicTraceTest
+    : public ::testing::TestWithParam<std::pair<std::string, std::string>> {
  protected:
   void SetUp() override {
-    client_path_ = px::testing::BazelRunfilePath(kClientPath).string();
-    server_path_ = px::testing::BazelRunfilePath(kServerPath).string();
+    client_path_ = px::testing::BazelRunfilePath(GetParam().first).string();
+    server_path_ = px::testing::BazelRunfilePath(GetParam().second).string();
 
     ASSERT_TRUE(fs::Exists(server_path_));
     ASSERT_TRUE(fs::Exists(client_path_));
@@ -113,6 +128,11 @@ class GoHTTPDynamicTraceTest : public ::testing::Test {
   std::unique_ptr<SourceConnector> connector_;
 };
 
+INSTANTIATE_TEST_SUITE_P(GoHTTPDynamicTraceTestInstances, GoHTTPDynamicTraceTest,
+                         ::testing::Values(std::make_pair(kGo1_21_ClientPath, kGo1_21_ServerPath),
+                                           std::make_pair(kGo1_22_ClientPath, kGo1_22_ServerPath),
+                                           std::make_pair(kGo1_23_ClientPath, kGo1_23_ServerPath)));
+
 constexpr char kGRPCTraceProgram[] = R"(
 tracepoints {
   program {
@@ -176,7 +196,7 @@ tracepoints {
 }
 )";
 
-TEST_F(GoHTTPDynamicTraceTest, TraceGolangHTTPClientAndServer) {
+TEST_P(GoHTTPDynamicTraceTest, TraceGolangHTTPClientAndServer) {
   ASSERT_NO_FATAL_FAILURE(InitTestFixturesAndRunTestProgram(kGRPCTraceProgram));
   std::vector<TaggedRecordBatch> tablets = GetRecords();
 
@@ -200,7 +220,7 @@ TEST_F(GoHTTPDynamicTraceTest, TraceGolangHTTPClientAndServer) {
   }
 }
 
-TEST_F(GoHTTPDynamicTraceTest, TraceReturnValue) {
+TEST_P(GoHTTPDynamicTraceTest, TraceReturnValue) {
   ASSERT_NO_FATAL_FAILURE(InitTestFixturesAndRunTestProgram(kReturnValueTraceProgram));
   std::vector<TaggedRecordBatch> tablets = GetRecords();