From 53a7ec2a7986e1aaa5a73190764ff5ad9a789937 Mon Sep 17 00:00:00 2001
From: Bastian Krol <bastian.krol@dash0.com>
Date: Mon, 24 Jun 2024 07:47:36 +0200
Subject: [PATCH] test(preload): build once, then test against both libc
 flavors

The previous build & test setup was invalid: So far we have built
_different_ libdash0envhook.so binaries for the LD_PRELOAD hook per
libc flavor. With this, the binary works in the target system.

But actually this approach is not not possible in the real world. We
can set one binary as LD_PRELOAD when instrumenting a container and we
do not know which libc flavor it is ahead of time. Thus we ultimately
need to build a libdash0envhook.so binary that works independently of
which libc flavor the target system uses.

This commit only fixes the test setup. The binary does not yet support
both libc flavors. This will be handled in a follow up commit.

[skip ci]
---
 images/dash0-instrumentation/preload/Makefile | 28 ++++++----
 .../preload/docker/Dockerfile-build           | 12 +++++
 .../Dockerfile-test-glibc}                    |  2 +-
 .../Dockerfile-test-musl}                     |  2 +-
 .../preload/scripts/build-in-container.sh     | 54 +++++++++++++++++++
 .../preload/scripts/make-clean.sh             | 12 +++++
 .../run-tests-in-container.sh}                | 23 ++++----
 .../preload/scripts/test-all.sh               | 16 ++++++
 .../preload/src/libdash0envhook.c             |  1 -
 .../dash0-instrumentation/preload/src/map.c   |  3 +-
 .../dash0-instrumentation/preload/src/map.h   |  1 +
 .../dash0-instrumentation/preload/test-all.sh | 14 -----
 .../preload/test/appundertest.c               |  3 +-
 .../test/{smoke-test.sh => run-tests.sh}      | 18 +++++--
 .../preload/testbin/.gitignore                |  1 -
 .../preload/testbin/aarch64/.gitignore        |  1 +
 .../preload/testbin/x86_64/.gitignore         |  1 +
 17 files changed, 144 insertions(+), 48 deletions(-)
 create mode 100644 images/dash0-instrumentation/preload/docker/Dockerfile-build
 rename images/dash0-instrumentation/preload/{Dockerfile-glibc => docker/Dockerfile-test-glibc} (88%)
 rename images/dash0-instrumentation/preload/{Dockerfile-musl => docker/Dockerfile-test-musl} (86%)
 create mode 100755 images/dash0-instrumentation/preload/scripts/build-in-container.sh
 create mode 100755 images/dash0-instrumentation/preload/scripts/make-clean.sh
 rename images/dash0-instrumentation/preload/{build-and-test.sh => scripts/run-tests-in-container.sh} (73%)
 create mode 100755 images/dash0-instrumentation/preload/scripts/test-all.sh
 delete mode 100755 images/dash0-instrumentation/preload/test-all.sh
 rename images/dash0-instrumentation/preload/test/{smoke-test.sh => run-tests.sh} (87%)
 delete mode 100644 images/dash0-instrumentation/preload/testbin/.gitignore
 create mode 100644 images/dash0-instrumentation/preload/testbin/aarch64/.gitignore
 create mode 100644 images/dash0-instrumentation/preload/testbin/x86_64/.gitignore

diff --git a/images/dash0-instrumentation/preload/Makefile b/images/dash0-instrumentation/preload/Makefile
index 6b1316af..23915430 100644
--- a/images/dash0-instrumentation/preload/Makefile
+++ b/images/dash0-instrumentation/preload/Makefile
@@ -1,13 +1,15 @@
 # SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
 # SPDX-License-Identifier: Apache-2.0
 
+ARCH := $(shell uname -m)
 CFLAGS ?= -Wall -Werror -Wextra -O2
+LIB_LINKER_FLAGS ?= -nostdlib -rdynamic -shared -ldl
 
 SRC_DIR := src
-BIN_DIR := bin
 OBJ_DIR := obj
 LIB_DIR := lib
 TEST_DIR := test
+TESTBIN_DIR := testbin/$(ARCH)
 SRC_EXT := c
 
 OS := $(shell uname -s)
@@ -17,21 +19,25 @@ NAMES := $(notdir $(basename $(wildcard $(SRC_DIR)/*.$(SRC_EXT))))
 OBJECTS :=$(patsubst %,$(OBJ_DIR)/%.o,$(NAMES))
 
 TEST_NAMES := $(notdir $(basename $(wildcard $(TEST_DIR)/*.$(SRC_EXT))))
-TEST_OBJECTS :=$(patsubst %,$(OBJ_DIR)/%.o,$(TEST_NAMES))
+TEST_OBJECTS :=$(patsubst %,$(TESTBIN_DIR)/%.o,$(TEST_NAMES))
 
-all: $(OBJECTS) $(TEST_OBJECTS) testbin/appundertest.so
-	$(CC) $(CFLAGS) -rdynamic -shared -ldl $(OBJECTS) -o $(LIB_DIR)/libdash0envhook.so
+all: $(LIB_DIR)/libdash0envhook.so
+
+$(LIB_DIR)/libdash0envhook.so: $(OBJECTS)
+	$(CC) $(CFLAGS) $(LIB_LINKER_FLAGS) $(OBJECTS) -o $(LIB_DIR)/libdash0envhook.so
 
 $(OBJ_DIR)/%.o: $(SRC_DIR)/%.$(SRC_EXT)
 	$(CC) -c -fPIC $^ -o $@ $(DEBUG) $(CFLAGS) $(LIBS)
 
-$(OBJ_DIR)/%.o: $(TEST_DIR)/%.$(SRC_EXT)
-	$(CC) -c $^ -o $@ $(DEBUG) $(CFLAGS) $(LIBS)
+clean: clean-test
+	@rm -f $(OBJECTS) $(LIB_DIR)/libdash0envhook.so
+
+clean-test:
+	@rm -f $(TEST_OBJECTS)
 
-testbin/appundertest.so: test/appundertest.c
-	$(CC) $(CFLAGS) -o $@ test/appundertest.c
+build-test: $(TEST_OBJECTS)
 
-clean:
-	@rm -f $(OBJECTS) $(OBJ_DIR)/*.o $(TEST_OBJECTS) $(LIB_DIR)/*.so testbin/appundertest.so
+$(TESTBIN_DIR)/%.o: $(TEST_DIR)/%.$(SRC_EXT)
+	$(CC) $(CFLAGS) -o $@ $(DEBUG) $^
 
-.PHONY: all clean install
+.PHONY: all clean install clean-test build-test
diff --git a/images/dash0-instrumentation/preload/docker/Dockerfile-build b/images/dash0-instrumentation/preload/docker/Dockerfile-build
new file mode 100644
index 00000000..4221c59c
--- /dev/null
+++ b/images/dash0-instrumentation/preload/docker/Dockerfile-build
@@ -0,0 +1,12 @@
+# SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+FROM ubuntu:24.10
+
+RUN apt update && \
+  apt-get install -y \
+    build-essential
+
+WORKDIR /usr/src/dash0/preload/
+
+CMD ["scripts/make-clean.sh"]
diff --git a/images/dash0-instrumentation/preload/Dockerfile-glibc b/images/dash0-instrumentation/preload/docker/Dockerfile-test-glibc
similarity index 88%
rename from images/dash0-instrumentation/preload/Dockerfile-glibc
rename to images/dash0-instrumentation/preload/docker/Dockerfile-test-glibc
index 234317df..3d728af3 100644
--- a/images/dash0-instrumentation/preload/Dockerfile-glibc
+++ b/images/dash0-instrumentation/preload/docker/Dockerfile-test-glibc
@@ -9,4 +9,4 @@ RUN apt update && \
 
 WORKDIR /usr/src/dash0/preload/
 
-CMD ["test/smoke-test.sh"]
+CMD ["test/run-tests.sh"]
diff --git a/images/dash0-instrumentation/preload/Dockerfile-musl b/images/dash0-instrumentation/preload/docker/Dockerfile-test-musl
similarity index 86%
rename from images/dash0-instrumentation/preload/Dockerfile-musl
rename to images/dash0-instrumentation/preload/docker/Dockerfile-test-musl
index e5160ada..a5befcb1 100644
--- a/images/dash0-instrumentation/preload/Dockerfile-musl
+++ b/images/dash0-instrumentation/preload/docker/Dockerfile-test-musl
@@ -7,4 +7,4 @@ RUN apk add --no-cache build-base
 
 WORKDIR /usr/src/dash0/preload/
 
-CMD ["test/smoke-test.sh"]
+CMD ["test/run-tests.sh"]
diff --git a/images/dash0-instrumentation/preload/scripts/build-in-container.sh b/images/dash0-instrumentation/preload/scripts/build-in-container.sh
new file mode 100755
index 00000000..e03edfdd
--- /dev/null
+++ b/images/dash0-instrumentation/preload/scripts/build-in-container.sh
@@ -0,0 +1,54 @@
+#!/usr/bin/env sh
+
+# SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+set -eu
+
+cd "$(dirname "$0")"/..
+
+# TODO build multi platform image
+
+if [ -z "${ARCH:-}" ]; then
+  ARCH=arm64
+fi
+if [ "$ARCH" = arm64 ]; then
+  docker_platform=linux/arm64
+elif [ "$ARCH" = x86_64 ]; then
+  docker_platform=linux/amd64
+else
+  echo "The architecture $ARCH is not supported."
+  exit 1
+fi
+
+dockerfile_name=docker/Dockerfile-build
+image_name=dash0-env-hook-builder
+container_name=$image_name
+
+docker_run_extra_arguments=""
+if [ "${INTERACTIVE:-}" = "true" ]; then
+  docker_run_extra_arguments=/bin/bash
+fi
+
+echo
+echo
+echo ">>> Building the library on $ARCH <<<"
+
+docker rm -f "$container_name"
+docker build \
+  --platform "$docker_platform" \
+  . \
+  -f "$dockerfile_name" \
+  -t "$image_name"
+
+# note: building one image for both platforms is not supported by Docker desktop's standard "docker build" command.
+# docker build --platform linux/amd64,linux/arm64 . -f $dockerfile_name -t dash0-env-hook-builder-all-$LIBC
+
+docker run \
+  --platform "$docker_platform" \
+  --name "$container_name" \
+  -it \
+  --volume "$(pwd):/usr/src/dash0/preload/" \
+  "$image_name" \
+  $docker_run_extra_arguments
+
diff --git a/images/dash0-instrumentation/preload/scripts/make-clean.sh b/images/dash0-instrumentation/preload/scripts/make-clean.sh
new file mode 100755
index 00000000..5bd47a3d
--- /dev/null
+++ b/images/dash0-instrumentation/preload/scripts/make-clean.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env sh
+
+# SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+set -eu
+
+cd "$(dirname "$0")"/..
+
+make clean
+make
+
diff --git a/images/dash0-instrumentation/preload/build-and-test.sh b/images/dash0-instrumentation/preload/scripts/run-tests-in-container.sh
similarity index 73%
rename from images/dash0-instrumentation/preload/build-and-test.sh
rename to images/dash0-instrumentation/preload/scripts/run-tests-in-container.sh
index 754547b0..8f5419c2 100755
--- a/images/dash0-instrumentation/preload/build-and-test.sh
+++ b/images/dash0-instrumentation/preload/scripts/run-tests-in-container.sh
@@ -5,7 +5,7 @@
 
 set -eu
 
-cd "$(dirname "$0")"
+cd "$(dirname "$0")"/..
 
 if [ -z "${ARCH:-}" ]; then
   ARCH=arm64
@@ -25,17 +25,13 @@ if [ -z "${LIBC:-}" ]; then
   LIBC=glibc
 fi
 
-echo
-echo
-echo ">>> Building and testing for $ARCH and $LIBC <<<"
-
-dockerfile_name="Dockerfile-$LIBC"
+dockerfile_name="docker/Dockerfile-test-$LIBC"
 if [ ! -f "$dockerfile_name" ]; then
   echo "The file \"$dockerfile_name\" does not exist, the libc flavor $LIBC is not supported."
   exit 1
 fi
 
-image_name=dash0-env-hook-builder-$ARCH-$LIBC
+image_name=dash0-env-hook-test-$ARCH-$LIBC
 container_name=$image_name
 
 docker_run_extra_arguments=""
@@ -50,11 +46,16 @@ if [ "${INTERACTIVE:-}" = "true" ]; then
   fi
 fi
 
-docker rm -f "$container_name"
-docker build --platform "$docker_platform" . -f "$dockerfile_name" -t "$image_name"
+echo
+echo
+echo ">>> Testing the library on $ARCH and $LIBC <<<"
 
-# note: building one image for both platforms is not suppored on Docker desktop
-# docker build --platform linux/amd64,linux/arm64 . -f $dockerfile_name -t dash0-env-hook-builder-all-$LIBC
+docker rm -f "$container_name"
+docker build \
+  --platform "$docker_platform" \
+  . \
+  -f "$dockerfile_name" \
+  -t "$image_name"
 
 docker run \
   --platform "$docker_platform" \
diff --git a/images/dash0-instrumentation/preload/scripts/test-all.sh b/images/dash0-instrumentation/preload/scripts/test-all.sh
new file mode 100755
index 00000000..af878dcb
--- /dev/null
+++ b/images/dash0-instrumentation/preload/scripts/test-all.sh
@@ -0,0 +1,16 @@
+#!/usr/bin/env sh
+
+# SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+set -eu
+
+cd "$(dirname "$0")"/..
+
+scripts/build-in-container.sh
+
+ARCH=arm64 LIBC=glibc scripts/run-tests-in-container.sh
+ARCH=x86_64 LIBC=glibc scripts/run-tests-in-container.sh
+ARCH=arm64 LIBC=musl scripts/run-tests-in-container.sh
+ARCH=x86_64 LIBC=musl scripts/run-tests-in-container.sh
+
diff --git a/images/dash0-instrumentation/preload/src/libdash0envhook.c b/images/dash0-instrumentation/preload/src/libdash0envhook.c
index 4c8710a5..85ceab73 100644
--- a/images/dash0-instrumentation/preload/src/libdash0envhook.c
+++ b/images/dash0-instrumentation/preload/src/libdash0envhook.c
@@ -22,7 +22,6 @@ Entry map[1];
 
 char* default_node_options_value = "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js";
 
-
 __attribute__((constructor)) static void setup(void) {
   Entry node_options_entry = { .key = "NODE_OPTIONS", .value = NULL };
   map[0] = node_options_entry;
diff --git a/images/dash0-instrumentation/preload/src/map.c b/images/dash0-instrumentation/preload/src/map.c
index 13f93367..0fcb4e5a 100644
--- a/images/dash0-instrumentation/preload/src/map.c
+++ b/images/dash0-instrumentation/preload/src/map.c
@@ -1,13 +1,12 @@
 // SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-#include <stdlib.h>
 #include <string.h>
 
 #include "map.h"
 
 Entry* find(Entry map[], int len, const char* key) {
- for (int i = 0; i < len; i++) {
+  for (int i = 0; i < len; i++) {
     Entry* e = &map[i];
     if (strcmp(e->key, key) == 0) {
       return e;
diff --git a/images/dash0-instrumentation/preload/src/map.h b/images/dash0-instrumentation/preload/src/map.h
index 67659986..95f013f5 100755
--- a/images/dash0-instrumentation/preload/src/map.h
+++ b/images/dash0-instrumentation/preload/src/map.h
@@ -9,3 +9,4 @@ typedef struct Entry {
 char* get_map_entry(Entry map[], int len, const char* key);
 
 void put_map_entry(Entry* map, int len, const char* key, char* value);
+
diff --git a/images/dash0-instrumentation/preload/test-all.sh b/images/dash0-instrumentation/preload/test-all.sh
deleted file mode 100755
index 02ada664..00000000
--- a/images/dash0-instrumentation/preload/test-all.sh
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/usr/bin/env sh
-
-# SPDX-FileCopyrightText: Copyright 2024 Dash0 Inc.
-# SPDX-License-Identifier: Apache-2.0
-
-set -eu
-
-cd "$(dirname "$0")"
-
-ARCH=arm64 LIBC=glibc ./build-and-test.sh
-ARCH=x86_64 LIBC=glibc ./build-and-test.sh
-ARCH=arm64 LIBC=musl ./build-and-test.sh
-ARCH=x86_64 LIBC=musl ./build-and-test.sh
-
diff --git a/images/dash0-instrumentation/preload/test/appundertest.c b/images/dash0-instrumentation/preload/test/appundertest.c
index 2ea99d6b..576c1507 100644
--- a/images/dash0-instrumentation/preload/test/appundertest.c
+++ b/images/dash0-instrumentation/preload/test/appundertest.c
@@ -33,8 +33,7 @@ void echo_env_var_secure(const char* name) {
 
 int main(int argc, char* argv[]) {
   if (argc < 2) {
-    fputs("not enough arguments, the name of the test case needs to be specifed", stdout);
-    fputs("\n", stdout);
+    fputs("error: not enough arguments, the name of the test case needs to be specifed\n", stdout);
     exit(1);
   }
   char* test_case = argv[1];
diff --git a/images/dash0-instrumentation/preload/test/smoke-test.sh b/images/dash0-instrumentation/preload/test/run-tests.sh
similarity index 87%
rename from images/dash0-instrumentation/preload/test/smoke-test.sh
rename to images/dash0-instrumentation/preload/test/run-tests.sh
index 46ebdc9f..77200ff2 100755
--- a/images/dash0-instrumentation/preload/test/smoke-test.sh
+++ b/images/dash0-instrumentation/preload/test/run-tests.sh
@@ -34,6 +34,15 @@ else
   printf "${GREEN}verifying CPU architecture %s successful${NC}\n" "$EXPECTED_CPU_ARCHITECTURE"
 fi
 
+preload_lib=$directory/lib/libdash0envhook.so
+if [ ! -f $preload_lib ]; then
+  printf "${RED}error: $preload_lib does not exist, not running any tests.${NC}\n"
+  exit 1
+fi
+
+appundertest=testbin/"${arch_output}"/appundertest.o
+echo appundertest: $appundertest
+
 run_test_case() {
   test_case=$1
   command=$2
@@ -41,9 +50,9 @@ run_test_case() {
   existing_node_options_value=${4:-}
   set +e
   if [ "$existing_node_options_value" != "" ]; then
-    test_output=$(LD_PRELOAD="$directory/lib/libdash0envhook.so" NODE_OPTIONS="$existing_node_options_value" ./testbin/appundertest.so "$command")
+    test_output=$(LD_PRELOAD="$preload_lib" NODE_OPTIONS="$existing_node_options_value" "$appundertest" "$command")
   else
-    test_output=$(LD_PRELOAD="$directory/lib/libdash0envhook.so" ./testbin/appundertest.so "$command")
+    test_output=$(LD_PRELOAD="$preload_lib" "$appundertest" "$command")
   fi
   test_exit_code=$?
   set -e
@@ -62,8 +71,9 @@ run_test_case() {
   fi
 }
 
-make clean
-make
+# We always need to clean out the old appundertest.o, it might have been built for a different libc flavor.
+make clean-test
+make build-test
 
 exit_code=0
 
diff --git a/images/dash0-instrumentation/preload/testbin/.gitignore b/images/dash0-instrumentation/preload/testbin/.gitignore
deleted file mode 100644
index 140f8cf8..00000000
--- a/images/dash0-instrumentation/preload/testbin/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-*.so
diff --git a/images/dash0-instrumentation/preload/testbin/aarch64/.gitignore b/images/dash0-instrumentation/preload/testbin/aarch64/.gitignore
new file mode 100644
index 00000000..5761abcf
--- /dev/null
+++ b/images/dash0-instrumentation/preload/testbin/aarch64/.gitignore
@@ -0,0 +1 @@
+*.o
diff --git a/images/dash0-instrumentation/preload/testbin/x86_64/.gitignore b/images/dash0-instrumentation/preload/testbin/x86_64/.gitignore
new file mode 100644
index 00000000..5761abcf
--- /dev/null
+++ b/images/dash0-instrumentation/preload/testbin/x86_64/.gitignore
@@ -0,0 +1 @@
+*.o