Skip to content

Commit

Permalink
[v2] Use health check in grpc e2e test (jaegertracing#6113)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Resolves jaegertracing#5859

## Description of the changes
- Use default health check extension
- Currently it causes the test to go into infinite loop because of
recursive tracing due to jaegertracing#5971

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Oct 29, 2024
1 parent e327edb commit 3c1e85d
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 20 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/ci-lint-checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ jobs:

- uses: ./.github/actions/block-pr-from-main-branch

- run: make lint-nocommit
- run: |
git fetch origin main
make lint-nocommit
dco-check:
runs-on: ubuntu-latest
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ lint-license:

.PHONY: lint-nocommit
lint-nocommit:
@if git diff main | grep '@no''commit' ; then \
@if git diff origin/main | grep '@no''commit' ; then \
echo "❌ Cannot merge PR that contains @no""commit string" ; \
GIT_PAGER=cat git diff -G '@no''commit' main ; \
GIT_PAGER=cat git diff -G '@no''commit' origin/main ; \
false ; \
else \
echo "✅ Changes do not contain @no""commit string" ; \
Expand Down
23 changes: 9 additions & 14 deletions cmd/jaeger/internal/integration/e2e_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -42,10 +41,10 @@ const otlpPort = 4317
type E2EStorageIntegration struct {
integration.StorageIntegration

SkipStorageCleaner bool
ConfigFile string
BinaryName string
HealthCheckEndpoint string
SkipStorageCleaner bool
ConfigFile string
BinaryName string
HealthCheckPort int // overridable for Kafka tests which run two binaries and need different ports

// EnvVarOverrides contains a map of environment variables to set.
// The key in the map is the environment variable to override and the value
Expand Down Expand Up @@ -160,10 +159,11 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) {
}

func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
healthCheckEndpoint := s.HealthCheckEndpoint
if healthCheckEndpoint == "" {
healthCheckEndpoint = "http://localhost:13133/status"
healthCheckPort := s.HealthCheckPort
if healthCheckPort == 0 {
healthCheckPort = ports.CollectorV2HealthChecks
}
healthCheckEndpoint := fmt.Sprintf("http://localhost:%d/status", healthCheckPort)
t.Logf("Checking if %s is available on %s", s.BinaryName, healthCheckEndpoint)
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
Expand All @@ -187,11 +187,6 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
t.Logf("HTTP response not OK: %v", string(body))
return false
}
// for backwards compatibility with other healthchecks
if !strings.HasSuffix(healthCheckEndpoint, "/status") {
t.Logf("OK HTTP from endpoint that is not healthcheckv2")
return true
}

var healthResponse struct {
Status string `json:"status"`
Expand All @@ -203,7 +198,7 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {

// Check if the status field in the JSON is "StatusOK"
if healthResponse.Status != "StatusOK" {
t.Logf("Received non-K status %s: %s", healthResponse.Status, string(body))
t.Logf("Received non-OK status %s: %s", healthResponse.Status, string(body))
return false
}
return true
Expand Down
6 changes: 3 additions & 3 deletions cmd/jaeger/internal/integration/kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ func TestKafkaStorage(t *testing.T) {
t.Log("Collector initialized")

ingester := &E2EStorageIntegration{
BinaryName: "jaeger-v2-ingester",
ConfigFile: "../../config-kafka-ingester.yaml",
HealthCheckEndpoint: "http://localhost:14133/status",
BinaryName: "jaeger-v2-ingester",
ConfigFile: "../../config-kafka-ingester.yaml",
HealthCheckPort: 14133,
StorageIntegration: integration.StorageIntegration{
CleanUp: purge,
GetDependenciesReturnsSource: true,
Expand Down

0 comments on commit 3c1e85d

Please sign in to comment.