Skip to content

Commit 9ca4c44

Browse files
refactor(pyroscope): extract DebugInfoClient to leaf package
Moves the struct + Upload method out of package write into a new write/debuginfoclient package, breaking the cycle that prevented reporter tests from reusing the production type. Reporter tests now use debuginfoclient.Client instead of a duplicated test stub. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1fea1ed commit 9ca4c44

File tree

4 files changed

+50
-63
lines changed

4 files changed

+50
-63
lines changed

internal/component/pyroscope/ebpf/reporter/parca/reporter/pyroscope_uploader_test.go

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,19 @@ package reporter
55
import (
66
"bytes"
77
"context"
8-
"fmt"
98
"io"
109
"math/rand"
1110
"net/http"
1211
"net/http/httptest"
1312
"os"
14-
"strings"
1513
"sync"
1614
"testing"
1715
"time"
1816

1917
"connectrpc.com/connect"
2018
"github.com/go-kit/log"
2119
"github.com/gorilla/mux"
20+
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfoclient"
2221
debuginfov1alpha1 "github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1"
2322
"github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1/debuginfov1alpha1connect"
2423
"github.com/prometheus/client_golang/prometheus"
@@ -85,33 +84,6 @@ type uploadResult struct {
8584
data []byte
8685
}
8786

88-
// TODO remove this, do not mERGE
89-
// testDebugInfoClient implements DebugInfoClient by embedding a connect client
90-
// and doing the HTTP POST upload via an httpClient + baseURL.
91-
type testDebugInfoClient struct {
92-
debuginfov1alpha1connect.DebuginfoServiceClient
93-
httpClient *http.Client
94-
baseURL string
95-
}
96-
97-
func (c *testDebugInfoClient) Upload(ctx context.Context, buildID string, body io.Reader) error {
98-
uploadURL := strings.TrimRight(c.baseURL, "/") + "/debuginfo.v1alpha1.DebuginfoService/Upload/" + buildID
99-
req, err := http.NewRequestWithContext(ctx, "POST", uploadURL, body)
100-
if err != nil {
101-
return err
102-
}
103-
resp, err := c.httpClient.Do(req)
104-
if err != nil {
105-
return err
106-
}
107-
io.Copy(io.Discard, resp.Body)
108-
resp.Body.Close()
109-
if resp.StatusCode != http.StatusOK {
110-
return fmt.Errorf("upload: HTTP %d", resp.StatusCode)
111-
}
112-
return nil
113-
}
114-
11587
type mockDebuginfoHandler struct {
11688
shouldInitiateFunc func(ctx context.Context, req *connect.Request[debuginfov1alpha1.ShouldInitiateUploadRequest]) (*connect.Response[debuginfov1alpha1.ShouldInitiateUploadResponse], error)
11789
uploadFinishedFunc func(ctx context.Context, req *connect.Request[debuginfov1alpha1.UploadFinishedRequest]) (*connect.Response[debuginfov1alpha1.UploadFinishedResponse], error)
@@ -132,10 +104,11 @@ func startMockServer(t *testing.T, handler *mockDebuginfoHandler, uploadHandler
132104
router.Handle("/debuginfo.v1alpha1.DebuginfoService/Upload/{gnu_build_id}", uploadHandler).Methods("POST")
133105
server := httptest.NewServer(router)
134106
t.Cleanup(server.Close)
135-
return &testDebugInfoClient{
107+
return &debuginfoclient.Client{
136108
DebuginfoServiceClient: debuginfov1alpha1connect.NewDebuginfoServiceClient(server.Client(), server.URL),
137-
httpClient: server.Client(),
138-
baseURL: server.URL,
109+
HTTPClient: server.Client(),
110+
BaseURL: server.URL,
111+
UploadTimeout: time.Minute,
139112
}
140113
}
141114

internal/component/pyroscope/receive_http/debuginfo_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import (
1818
"github.com/stretchr/testify/require"
1919

2020
"github.com/grafana/alloy/internal/component/pyroscope"
21-
"github.com/grafana/alloy/internal/component/pyroscope/write"
2221
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfo"
22+
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfoclient"
2323
)
2424

2525
type downstreamResult struct {
@@ -68,7 +68,7 @@ func startMockDownstream(t *testing.T, shouldUpload bool, resultCh chan<- downst
6868
t.Cleanup(server.Close)
6969

7070
connectClient := debuginfov1alpha1connect.NewDebuginfoServiceClient(server.Client(), server.URL)
71-
return &write.DebugInfoClient{
71+
return &debuginfoclient.Client{
7272
DebuginfoServiceClient: connectClient,
7373
HTTPClient: server.Client(),
7474
BaseURL: server.URL,
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package debuginfoclient
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"io"
7+
"net/http"
8+
"net/url"
9+
"strings"
10+
"time"
11+
12+
"github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1/debuginfov1alpha1connect"
13+
)
14+
15+
type Client struct {
16+
debuginfov1alpha1connect.DebuginfoServiceClient
17+
HTTPClient *http.Client
18+
BaseURL string
19+
UploadTimeout time.Duration
20+
}
21+
22+
func (c *Client) Upload(ctx context.Context, buildID string, body io.Reader) error {
23+
ctx, cancel := context.WithTimeout(ctx, c.UploadTimeout)
24+
defer cancel()
25+
t1 := time.Now()
26+
uploadURL := strings.TrimRight(c.BaseURL, "/") + "/debuginfo.v1alpha1.DebuginfoService/Upload/" + url.PathEscape(buildID)
27+
req, err := http.NewRequestWithContext(ctx, "POST", uploadURL, body)
28+
if err != nil {
29+
return err
30+
}
31+
resp, err := c.HTTPClient.Do(req)
32+
if err != nil {
33+
return err
34+
}
35+
_, _ = io.Copy(io.Discard, resp.Body)
36+
_ = resp.Body.Close()
37+
if resp.StatusCode != http.StatusOK {
38+
return fmt.Errorf("upload: HTTP %d (duration %s)", resp.StatusCode, time.Since(t1))
39+
}
40+
return nil
41+
}

internal/component/pyroscope/write/write.go

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/grafana/alloy/internal/component/pyroscope"
2222
"github.com/grafana/alloy/internal/component/pyroscope/util"
2323
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfo"
24+
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfoclient"
2425
"github.com/grafana/alloy/internal/component/pyroscope/write/promhttp2"
2526
"github.com/grafana/dskit/backoff"
2627
"github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1/debuginfov1alpha1connect"
@@ -277,7 +278,7 @@ func newFanOut(logger log.Logger, tracer trace.Tracer, config Arguments, metrics
277278
endpointDataPath := filepath.Join(dataPath, fmt.Sprintf("endpoint-%d", i))
278279

279280
debugInfoConnect := debuginfov1alpha1connect.NewDebuginfoServiceClient(httpClient, endpoint.URL, WithUserAgent(userAgent))
280-
dic := &DebugInfoClient{
281+
dic := &debuginfoclient.Client{
281282
DebuginfoServiceClient: debugInfoConnect,
282283
HTTPClient: httpClient,
283284
BaseURL: endpoint.URL,
@@ -752,34 +753,6 @@ func validateLabels(lbls labels.Labels) error {
752753
return err
753754
}
754755

755-
type DebugInfoClient struct {
756-
debuginfov1alpha1connect.DebuginfoServiceClient
757-
HTTPClient *http.Client
758-
BaseURL string
759-
UploadTimeout time.Duration
760-
}
761-
762-
func (c *DebugInfoClient) Upload(ctx context.Context, buildID string, body io.Reader) error {
763-
ctx, cancel := context.WithTimeout(ctx, c.UploadTimeout)
764-
defer cancel()
765-
t1 := time.Now()
766-
uploadURL := strings.TrimRight(c.BaseURL, "/") + "/debuginfo.v1alpha1.DebuginfoService/Upload/" + url.PathEscape(buildID)
767-
req, err := http.NewRequestWithContext(ctx, "POST", uploadURL, body)
768-
if err != nil {
769-
return err
770-
}
771-
resp, err := c.HTTPClient.Do(req)
772-
if err != nil {
773-
return err
774-
}
775-
_, _ = io.Copy(io.Discard, resp.Body)
776-
_ = resp.Body.Close()
777-
if resp.StatusCode != http.StatusOK {
778-
return fmt.Errorf("upload: HTTP %d (duration %s)", resp.StatusCode, time.Since(t1))
779-
}
780-
return nil
781-
}
782-
783756
func configureTracing(config Arguments, httpClient *http.Client) {
784757
if config.Tracing.JaegerPropagator || config.Tracing.TraceContextPropagator {
785758
var propagators []propagation.TextMapPropagator

0 commit comments

Comments
 (0)