Skip to content

Commit 3c1e85d

Browse files
authored
[v2] Use health check in grpc e2e test (jaegertracing#6113)
## 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]>
1 parent e327edb commit 3c1e85d

File tree

4 files changed

+17
-20
lines changed

4 files changed

+17
-20
lines changed

.github/workflows/ci-lint-checks.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ jobs:
4747

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

50-
- run: make lint-nocommit
50+
- run: |
51+
git fetch origin main
52+
make lint-nocommit
5153
5254
dco-check:
5355
runs-on: ubuntu-latest

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ lint-license:
153153

154154
.PHONY: lint-nocommit
155155
lint-nocommit:
156-
@if git diff main | grep '@no''commit' ; then \
156+
@if git diff origin/main | grep '@no''commit' ; then \
157157
echo "❌ Cannot merge PR that contains @no""commit string" ; \
158-
GIT_PAGER=cat git diff -G '@no''commit' main ; \
158+
GIT_PAGER=cat git diff -G '@no''commit' origin/main ; \
159159
false ; \
160160
else \
161161
echo "✅ Changes do not contain @no""commit string" ; \

cmd/jaeger/internal/integration/e2e_integration.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"os"
1414
"os/exec"
1515
"path/filepath"
16-
"strings"
1716
"testing"
1817
"time"
1918

@@ -42,10 +41,10 @@ const otlpPort = 4317
4241
type E2EStorageIntegration struct {
4342
integration.StorageIntegration
4443

45-
SkipStorageCleaner bool
46-
ConfigFile string
47-
BinaryName string
48-
HealthCheckEndpoint string
44+
SkipStorageCleaner bool
45+
ConfigFile string
46+
BinaryName string
47+
HealthCheckPort int // overridable for Kafka tests which run two binaries and need different ports
4948

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

162161
func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
163-
healthCheckEndpoint := s.HealthCheckEndpoint
164-
if healthCheckEndpoint == "" {
165-
healthCheckEndpoint = "http://localhost:13133/status"
162+
healthCheckPort := s.HealthCheckPort
163+
if healthCheckPort == 0 {
164+
healthCheckPort = ports.CollectorV2HealthChecks
166165
}
166+
healthCheckEndpoint := fmt.Sprintf("http://localhost:%d/status", healthCheckPort)
167167
t.Logf("Checking if %s is available on %s", s.BinaryName, healthCheckEndpoint)
168168
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
169169
defer cancel()
@@ -187,11 +187,6 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
187187
t.Logf("HTTP response not OK: %v", string(body))
188188
return false
189189
}
190-
// for backwards compatibility with other healthchecks
191-
if !strings.HasSuffix(healthCheckEndpoint, "/status") {
192-
t.Logf("OK HTTP from endpoint that is not healthcheckv2")
193-
return true
194-
}
195190

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

204199
// Check if the status field in the JSON is "StatusOK"
205200
if healthResponse.Status != "StatusOK" {
206-
t.Logf("Received non-K status %s: %s", healthResponse.Status, string(body))
201+
t.Logf("Received non-OK status %s: %s", healthResponse.Status, string(body))
207202
return false
208203
}
209204
return true

cmd/jaeger/internal/integration/kafka_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,9 @@ func TestKafkaStorage(t *testing.T) {
5252
t.Log("Collector initialized")
5353

5454
ingester := &E2EStorageIntegration{
55-
BinaryName: "jaeger-v2-ingester",
56-
ConfigFile: "../../config-kafka-ingester.yaml",
57-
HealthCheckEndpoint: "http://localhost:14133/status",
55+
BinaryName: "jaeger-v2-ingester",
56+
ConfigFile: "../../config-kafka-ingester.yaml",
57+
HealthCheckPort: 14133,
5858
StorageIntegration: integration.StorageIntegration{
5959
CleanUp: purge,
6060
GetDependenciesReturnsSource: true,

0 commit comments

Comments
 (0)