[e2e tests] Ping v2 binary to be ready before running tests (#5382)

This PR started as a dependabot upgrade of OTEL Collector dependencies,
but the [tests were
failing](https://github.com/jaegertracing/jaeger/actions/runs/8854720626/job/24318352528?pr=5382).

The main change here is adding a ping from `e2eInitialize` to the
jaeger-v2 binary to make sure that it started before proceeding with
tests (ideally OTEL Collector should have a healthcheck endpoint that
signals when all components have been started successfully).

Also added some better logging.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>

.github/workflows/ci-badger.yaml

@@ -37,11 +37,10 @@ jobs:
       run: |
         case ${{ matrix.version }} in
           v1)
-              make badger-storage-integration-test
+            make badger-storage-integration-test
             ;;
           v2)
-            STORAGE=badger \
-              make jaeger-v2-storage-integration-test
+            STORAGE=badger make jaeger-v2-storage-integration-test
             ;;
         esac
 
@@ -53,6 +52,6 @@ jobs:
       with:
         files: cover.out
         verbose: true
-        flags: badger
+        flags: badger_${{ matrix.version }}
         fail_ci_if_error: true
         token: ${{ env.CODECOV_TOKEN }}

.github/workflows/ci-grpc.yml

@@ -37,13 +37,10 @@ jobs:
       run: |
         case ${{ matrix.version }} in
           v1)
-            SPAN_STORAGE_TYPE=memory \
-              make grpc-storage-integration-test
+            SPAN_STORAGE_TYPE=memory make grpc-storage-integration-test
             ;;
           v2)
-            STORAGE=grpc \
-              SPAN_STORAGE_TYPE=memory \
-              make jaeger-v2-storage-integration-test
+            STORAGE=grpc SPAN_STORAGE_TYPE=memory make jaeger-v2-storage-integration-test
             ;;
         esac
 
@@ -55,6 +52,6 @@ jobs:
       with:
         files: cover.out
         verbose: true
-        flags: grpc
+        flags: grpc_${{ matrix.version }}
         fail_ci_if_error: true
         token: ${{ env.CODECOV_TOKEN }}

cmd/jaeger/internal/integration/grpc_test.go

@@ -6,7 +6,6 @@ package integration
 import (
 	"testing"
 
-	"github.com/jaegertracing/jaeger/pkg/testutils"
 	"github.com/jaegertracing/jaeger/plugin/storage/integration"
 )
 
@@ -17,11 +16,7 @@ type GRPCStorageIntegration struct {
 }
 
 func (s *GRPCStorageIntegration) initialize(t *testing.T) {
-	logger, _ := testutils.NewLogger()
-
-	s.remoteStorage = integration.StartNewRemoteMemoryStorage(t, logger)
-
-	s.CleanUp = s.cleanUp
+	s.remoteStorage = integration.StartNewRemoteMemoryStorage(t)
 }
 
 func (s *GRPCStorageIntegration) cleanUp(t *testing.T) {
@@ -32,10 +27,15 @@ func (s *GRPCStorageIntegration) cleanUp(t *testing.T) {
 func TestGRPCStorage(t *testing.T) {
 	integration.SkipUnlessEnv(t, "grpc")
 
-	s := &GRPCStorageIntegration{}
-	s.ConfigFile = "../../grpc_config.yaml"
-	s.SkipBinaryAttrs = true
-
+	s := &GRPCStorageIntegration{
+		E2EStorageIntegration: E2EStorageIntegration{
+			ConfigFile: "../../grpc_config.yaml",
+			StorageIntegration: integration.StorageIntegration{
+				SkipBinaryAttrs: true,
+			},
+		},
+	}
+	s.CleanUp = s.cleanUp
 	s.initialize(t)
 	s.e2eInitialize(t)
 	t.Cleanup(func() {

cmd/jaeger/internal/integration/integration.go

@@ -4,11 +4,15 @@
 package integration
 
 import (
+	"context"
+	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v3"
@@ -41,7 +45,7 @@ type E2EStorageIntegration struct {
 func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) {
 	logger, _ := testutils.NewLogger()
 	configFile := createStorageCleanerConfig(t, s.ConfigFile)
-
+	t.Logf("Starting Jaeger-v2 in the background with config file %s", configFile)
 	cmd := exec.Cmd{
 		Path: "./cmd/jaeger/jaeger",
 		Args: []string{"jaeger", "--config", configFile},
@@ -53,6 +57,22 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) {
 		Stderr: os.Stderr,
 	}
 	require.NoError(t, cmd.Start())
+	require.Eventually(t, func() bool {
+		url := fmt.Sprintf("http://localhost:%d/", ports.QueryHTTP)
+		t.Logf("Checking if Jaeger-v2 is available on %s", url)
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+		defer cancel()
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+		require.NoError(t, err)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Log(err)
+			return false
+		}
+		defer resp.Body.Close()
+		return resp.StatusCode == http.StatusOK
+	}, 30*time.Second, 500*time.Millisecond, "Jaeger-v2 did not start")
+	t.Log("Jaeger-v2 is ready")
 	t.Cleanup(func() {
 		require.NoError(t, cmd.Process.Kill())
 	})

cmd/query/app/grpc_handler.go

@@ -102,7 +102,7 @@ func (g *GRPCHandler) GetTrace(r *api_v2.GetTraceRequest, stream api_v2.QuerySer
 	}
 	trace, err := g.queryService.GetTrace(stream.Context(), r.TraceID)
 	if errors.Is(err, spanstore.ErrTraceNotFound) {
-		g.logger.Error(msgTraceNotFound, zap.Error(err))
+		g.logger.Warn(msgTraceNotFound, zap.Stringer("id", r.TraceID), zap.Error(err))
 		return status.Errorf(codes.NotFound, "%s: %v", msgTraceNotFound, err)
 	}
 	if err != nil {
@@ -122,7 +122,7 @@ func (g *GRPCHandler) ArchiveTrace(ctx context.Context, r *api_v2.ArchiveTraceRe
 	}
 	err := g.queryService.ArchiveTrace(ctx, r.TraceID)
 	if errors.Is(err, spanstore.ErrTraceNotFound) {
-		g.logger.Error("trace not found", zap.Error(err))
+		g.logger.Warn(msgTraceNotFound, zap.Stringer("id", r.TraceID), zap.Error(err))
 		return nil, status.Errorf(codes.NotFound, "%s: %v", msgTraceNotFound, err)
 	}
 	if err != nil {

plugin/storage/integration/grpc_test.go

@@ -22,10 +22,10 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
-	"github.com/jaegertracing/jaeger/pkg/testutils"
 	"github.com/jaegertracing/jaeger/plugin/storage/grpc"
 )
 
@@ -44,19 +44,18 @@ type GRPCStorageIntegrationTestSuite struct {
 }
 
 func (s *GRPCStorageIntegrationTestSuite) initialize(t *testing.T) {
-	s.logger, _ = testutils.NewLogger()
+	s.logger = zaptest.NewLogger(t, zaptest.Level(zap.DebugLevel))
 
 	if s.useRemoteStorage {
-		s.remoteStorage = StartNewRemoteMemoryStorage(t, s.logger)
+		s.remoteStorage = StartNewRemoteMemoryStorage(t)
 	}
 
 	f := grpc.NewFactory()
 	v, command := config.Viperize(f.AddFlags)
 	err := command.ParseFlags(s.flags)
 	require.NoError(t, err)
 	f.InitFromViper(v, zap.NewNop())
-	err = f.Initialize(metrics.NullFactory, s.logger)
-	require.NoError(t, err)
+	require.NoError(t, f.Initialize(metrics.NullFactory, s.logger))
 	s.factory = f
 
 	s.SpanWriter, err = f.CreateSpanWriter()

plugin/storage/integration/integration.go

@@ -40,10 +40,6 @@ import (
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 )
 
-const (
-	iterations = 100
-)
-
 //go:embed fixtures
 var fixtures embed.FS
 
@@ -130,12 +126,13 @@ func (s *StorageIntegration) skipIfNeeded(t *testing.T) {
 }
 
 func (s *StorageIntegration) waitForCondition(t *testing.T, predicate func(t *testing.T) bool) bool {
+	const iterations = 100 // Will wait at most 100 seconds.
 	for i := 0; i < iterations; i++ {
-		t.Logf("Waiting for storage backend to update documents, iteration %d out of %d", i+1, iterations)
 		if predicate(t) {
 			return true
 		}
-		time.Sleep(time.Second) // Will wait at most 100 seconds.
+		t.Logf("Waiting for storage backend to update documents, iteration %d out of %d", i+1, iterations)
+		time.Sleep(time.Second)
 	}
 	return predicate(t)
 }

plugin/storage/integration/remote_memory_storage.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
 
 	"github.com/jaegertracing/jaeger/cmd/remote-storage/app"
 	"github.com/jaegertracing/jaeger/pkg/config"
@@ -24,7 +25,8 @@ type RemoteMemoryStorage struct {
 	storageFactory *storage.Factory
 }
 
-func StartNewRemoteMemoryStorage(t *testing.T, logger *zap.Logger) *RemoteMemoryStorage {
+func StartNewRemoteMemoryStorage(t *testing.T) *RemoteMemoryStorage {
+	logger := zaptest.NewLogger(t, zaptest.Level(zap.DebugLevel))
 	opts := &app.Options{
 		GRPCHostPort: ports.PortToHostPort(ports.RemoteStorageGRPC),
 		Tenancy: tenancy.Options{
@@ -39,6 +41,7 @@ func StartNewRemoteMemoryStorage(t *testing.T, logger *zap.Logger) *RemoteMemory
 	storageFactory.InitFromViper(v, logger)
 	require.NoError(t, storageFactory.Initialize(metrics.NullFactory, logger))
 
+	t.Logf("Starting in-process remote storage server on %s", opts.GRPCHostPort)
 	server, err := app.NewServer(opts, storageFactory, tm, logger, healthcheck.New())
 	require.NoError(t, err)
 	require.NoError(t, server.Start())