Skip to content

Commit e59de9d

Browse files
committed
Simplify route harvest context APIs
Rename the route-harvest Java hooks so context is part of the primary Attach and ExtractRoutes calls. The route package no longer needs context-free wrappers now that every production caller supplies the harvest timeout context. Add narrow tests for deadline-result normalization and unsupported JVM attach results so the cancellation paths remain covered.
1 parent ff58376 commit e59de9d

5 files changed

Lines changed: 43 additions & 27 deletions

File tree

pkg/internal/transform/route/harvest/harvester.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func NewRouteHarvester(cfg *services.RouteHarvestingConfig, disabled []services.
7474
mux: &sync.Mutex{},
7575
}
7676

77-
h.javaExtractRoutes = h.java.ExtractRoutesContext
77+
h.javaExtractRoutes = h.java.ExtractRoutes
7878
h.nodeExtractRoutes = ExtractNodejsRoutes
7979

8080
return h

pkg/internal/transform/route/harvest/harvester_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,24 @@ func TestHarvestRoutes_Timeout(t *testing.T) {
133133
assert.Greater(t, elapsed, 90*time.Millisecond)
134134
}
135135

136+
func TestHarvestRoutes_ContextDeadlineResultReturnsTimeout(t *testing.T) {
137+
harvester := NewRouteHarvester(&services.RouteHarvestingConfig{}, []services.RouteHarvesterLanguage{}, 1*time.Second)
138+
harvester.javaExtractRoutes = func(context.Context, app.PID) (*RouteHarvesterResult, error) {
139+
return nil, context.DeadlineExceeded
140+
}
141+
142+
fileInfo := createTestFileInfo(svc.InstrumentableJava)
143+
144+
result, err := harvester.HarvestRoutes(fileInfo)
145+
146+
require.Error(t, err)
147+
assert.Nil(t, result)
148+
149+
var harvestErr *HarvestError
150+
require.ErrorAs(t, err, &harvestErr)
151+
assert.Equal(t, "route harvesting timed out", harvestErr.Message)
152+
}
153+
136154
func TestHarvestRoutes_Panic(t *testing.T) {
137155
harvester := NewRouteHarvester(&services.RouteHarvestingConfig{}, []services.RouteHarvesterLanguage{}, 1*time.Second)
138156
harvester.javaExtractRoutes = panicExtractRoutes

pkg/internal/transform/route/harvest/java.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type JavaRoutes struct {
3030
type JavaAttacher interface {
3131
Init()
3232
Cleanup()
33-
AttachContext(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error)
33+
Attach(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error)
3434
}
3535

3636
type RealJavaAttacher struct {
@@ -154,11 +154,7 @@ func (h *JavaRoutes) processSymbolLine(lineBytes []byte, routes []string) []stri
154154
return routes
155155
}
156156

157-
func (h *JavaRoutes) ExtractRoutes(pid app.PID) (*RouteHarvesterResult, error) {
158-
return h.ExtractRoutesContext(context.Background(), pid)
159-
}
160-
161-
func (h *JavaRoutes) ExtractRoutesContext(ctx context.Context, pid app.PID) (*RouteHarvesterResult, error) {
157+
func (h *JavaRoutes) ExtractRoutes(ctx context.Context, pid app.PID) (*RouteHarvesterResult, error) {
162158
var routes []string
163159

164160
runtime.LockOSThread()
@@ -167,7 +163,7 @@ func (h *JavaRoutes) ExtractRoutesContext(ctx context.Context, pid app.PID) (*Ro
167163
h.Attacher.Init()
168164
defer h.Attacher.Cleanup()
169165

170-
out, err := h.Attacher.AttachContext(ctx, pid, []string{"jcmd", "VM.symboltable -verbose"}, true)
166+
out, err := h.Attacher.Attach(ctx, pid, []string{"jcmd", "VM.symboltable -verbose"}, true)
171167
if err != nil {
172168
return nil, err
173169
}
@@ -220,10 +216,6 @@ func (j RealJavaAttacher) Cleanup() {
220216
}
221217
}
222218

223-
func (j RealJavaAttacher) Attach(pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
224-
return j.Attacher.Attach(int(pid), argv, ignoreOnJ9)
225-
}
226-
227-
func (j RealJavaAttacher) AttachContext(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
219+
func (j RealJavaAttacher) Attach(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
228220
return j.Attacher.AttachContext(ctx, int(pid), argv, ignoreOnJ9)
229221
}

pkg/internal/transform/route/harvest/java_notlinux.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,7 @@ func NewJavaRoutesHarvester() *JavaRoutes {
2323
return &JavaRoutes{Attacher: fakeAttacher{}}
2424
}
2525

26-
func (h *JavaRoutes) ExtractRoutes(_ app.PID) (*RouteHarvesterResult, error) {
27-
return nil, nil
28-
}
29-
30-
func (h *JavaRoutes) ExtractRoutesContext(_ context.Context, _ app.PID) (*RouteHarvesterResult, error) {
26+
func (h *JavaRoutes) ExtractRoutes(_ context.Context, _ app.PID) (*RouteHarvesterResult, error) {
3127
return nil, nil
3228
}
3329

pkg/internal/transform/route/harvest/java_test.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ func TestJavaRoutes_ExtractRoutes(t *testing.T) {
422422
return NewReaderCloser(strings.NewReader(tt.mockOutput)), nil
423423
}}
424424

425-
result, err := harvester.ExtractRoutes(tt.pid)
425+
result, err := harvester.ExtractRoutes(context.Background(), tt.pid)
426426

427427
if tt.expectedError {
428428
require.Error(t, err)
@@ -458,7 +458,7 @@ Header: ...
458458
return NewReaderCloser(strings.NewReader(symbolTableOutput)), nil
459459
}}
460460

461-
result, err := harvester.ExtractRoutes(1234)
461+
result, err := harvester.ExtractRoutes(context.Background(), 1234)
462462

463463
require.NoError(t, err)
464464
require.NotNil(t, result)
@@ -478,7 +478,7 @@ Header: ...
478478
assert.Equal(t, expectedRoutes, result.Routes)
479479
}
480480

481-
func TestJavaRoutes_ExtractRoutesContext_ClosesReaderOnCancellation(t *testing.T) {
481+
func TestJavaRoutes_ExtractRoutes_ClosesReaderOnCancellation(t *testing.T) {
482482
harvester := NewJavaRoutesHarvester()
483483
ctx, cancel := context.WithCancel(context.Background())
484484
defer cancel()
@@ -494,7 +494,7 @@ func TestJavaRoutes_ExtractRoutesContext_ClosesReaderOnCancellation(t *testing.T
494494

495495
errCh := make(chan error, 1)
496496
go func() {
497-
_, err := harvester.ExtractRoutesContext(ctx, 1234)
497+
_, err := harvester.ExtractRoutes(ctx, 1234)
498498
errCh <- err
499499
}()
500500

@@ -520,6 +520,20 @@ func TestJavaRoutes_ExtractRoutesContext_ClosesReaderOnCancellation(t *testing.T
520520
}
521521
}
522522

523+
func TestJavaRoutes_ExtractRoutes_ReturnsPartialResultWhenAttachIsUnsupported(t *testing.T) {
524+
harvester := NewJavaRoutesHarvester()
525+
harvester.Attacher = FakeJavaAttacher{attachFunc: func(context.Context, app.PID, []string, bool) (io.ReadCloser, error) {
526+
return nil, nil
527+
}}
528+
529+
result, err := harvester.ExtractRoutes(context.Background(), 1234)
530+
531+
require.NoError(t, err)
532+
require.NotNil(t, result)
533+
assert.Nil(t, result.Routes)
534+
assert.Equal(t, PartialRoutes, result.Kind)
535+
}
536+
523537
func TestRegexPatterns(t *testing.T) {
524538
t.Run("validURLPath regex", func(t *testing.T) {
525539
validPaths := []string{
@@ -663,11 +677,7 @@ func (j FakeJavaAttacher) Init() {
663677
func (j FakeJavaAttacher) Cleanup() {
664678
}
665679

666-
func (j FakeJavaAttacher) Attach(pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
667-
return j.AttachContext(context.Background(), pid, argv, ignoreOnJ9)
668-
}
669-
670-
func (j FakeJavaAttacher) AttachContext(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
680+
func (j FakeJavaAttacher) Attach(ctx context.Context, pid app.PID, argv []string, ignoreOnJ9 bool) (io.ReadCloser, error) {
671681
return j.attachFunc(ctx, pid, argv, ignoreOnJ9)
672682
}
673683

0 commit comments

Comments
 (0)