Skip to content

Commit a0b6588

Browse files
committed
feat: refactor Spotlight integration per code review
Address review comments from PR: - Move SENTRY_SPOTLIGHT env var check to NewClient - Remove noopTransport check in SendEvent - Use event.Sdk.Name/Version for User-Agent header - Use context for HTTP requests - Always drain HTTP response bodies - Use shared MockTransport from mocks.go
1 parent dce4632 commit a0b6588

File tree

3 files changed

+22
-42
lines changed

3 files changed

+22
-42
lines changed

client.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,13 @@ func NewClient(options ClientOptions) (*Client, error) {
346346
options.TraceIgnoreStatusCodes = [][]int{{404}}
347347
}
348348

349+
// Check for Spotlight environment variable
350+
if !options.Spotlight {
351+
if spotlightEnv := os.Getenv("SENTRY_SPOTLIGHT"); spotlightEnv == "true" || spotlightEnv == "1" {
352+
options.Spotlight = true
353+
}
354+
}
355+
349356
// SENTRYGODEBUG is a comma-separated list of key=value pairs (similar
350357
// to GODEBUG). It is not a supported feature: recognized debug options
351358
// may change any time.
@@ -402,12 +409,6 @@ func NewClient(options ClientOptions) (*Client, error) {
402409
}
403410

404411
func (client *Client) setupTransport() {
405-
if !client.options.Spotlight {
406-
if spotlightEnv := os.Getenv("SENTRY_SPOTLIGHT"); spotlightEnv == "true" || spotlightEnv == "1" {
407-
client.options.Spotlight = true
408-
}
409-
}
410-
411412
opts := client.options
412413
transport := opts.Transport
413414

spotlight_test.go

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package sentry
22

33
import (
4-
"context"
54
"net/http"
65
"net/http/httptest"
76
"testing"
@@ -27,21 +26,23 @@ func TestSpotlightTransport(t *testing.T) {
2726
}))
2827
defer server.Close()
2928

30-
mock := &mockTransport{}
29+
mock := &MockTransport{}
3130
st := NewSpotlightTransport(mock)
3231
st.Configure(ClientOptions{SpotlightURL: server.URL + "/stream"})
3332

3433
event := NewEvent()
34+
event.Sdk.Name = "sentry-go"
35+
event.Sdk.Version = SDKVersion
3536
event.Message = "Test message"
3637
st.SendEvent(event)
3738

3839
time.Sleep(100 * time.Millisecond)
3940

40-
if len(mock.events) != 1 {
41-
t.Errorf("Expected 1 event, got %d", len(mock.events))
41+
if len(mock.Events()) != 1 {
42+
t.Errorf("Expected 1 event, got %d", len(mock.Events()))
4243
}
43-
if mock.events[0].Message != "Test message" {
44-
t.Errorf("Expected 'Test message', got %s", mock.events[0].Message)
44+
if mock.Events()[0].Message != "Test message" {
45+
t.Errorf("Expected 'Test message', got %s", mock.Events()[0].Message)
4546
}
4647
}
4748

@@ -147,24 +148,3 @@ func TestSpotlightClientOptions(t *testing.T) {
147148
})
148149
}
149150
}
150-
151-
// mockTransport is a simple transport for testing.
152-
type mockTransport struct {
153-
events []*Event
154-
}
155-
156-
func (m *mockTransport) Configure(ClientOptions) {}
157-
158-
func (m *mockTransport) SendEvent(event *Event) {
159-
m.events = append(m.events, event)
160-
}
161-
162-
func (m *mockTransport) Flush(time.Duration) bool {
163-
return true
164-
}
165-
166-
func (m *mockTransport) FlushWithContext(_ context.Context) bool {
167-
return true
168-
}
169-
170-
func (m *mockTransport) Close() {}

transport.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -776,16 +776,15 @@ func (st *SpotlightTransport) Configure(options ClientOptions) {
776776
}
777777

778778
func (st *SpotlightTransport) SendEvent(event *Event) {
779-
// Send to the underlying transport (Sentry) unless it's noopTransport
780-
if _, isNoop := st.underlying.(noopTransport); !isNoop {
781-
st.underlying.SendEvent(event)
782-
}
779+
// Send to the underlying transport (Sentry)
780+
st.underlying.SendEvent(event)
783781

784782
// Always send to Spotlight
785783
st.sendToSpotlight(event)
786784
}
787785

788786
func (st *SpotlightTransport) sendToSpotlight(event *Event) {
787+
ctx := context.Background()
789788
dsn, _ := NewDsn("https://placeholder@localhost/1")
790789

791790
eventBody := getRequestBodyFromEvent(event)
@@ -800,14 +799,14 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
800799
return
801800
}
802801

803-
req, err := http.NewRequest("POST", st.spotlightURL, envelope)
802+
req, err := http.NewRequestWithContext(ctx, "POST", st.spotlightURL, envelope)
804803
if err != nil {
805804
DebugLogger.Printf("Failed to create Spotlight request: %v", err)
806805
return
807806
}
808807

809808
req.Header.Set("Content-Type", "application/x-sentry-envelope")
810-
req.Header.Set("User-Agent", "sentry-go/"+SDKVersion)
809+
req.Header.Set("User-Agent", fmt.Sprintf("%s/%s", event.Sdk.Name, event.Sdk.Version))
811810

812811
DebugLogger.Printf("Sending event to Spotlight at %s", st.spotlightURL)
813812

@@ -817,6 +816,9 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
817816
return
818817
}
819818
defer func() {
819+
// Drain body up to a limit and close it, allowing the
820+
// transport to reuse TCP connections.
821+
_, _ = io.CopyN(io.Discard, resp.Body, maxDrainResponseBytes)
820822
if closeErr := resp.Body.Close(); closeErr != nil {
821823
DebugLogger.Printf("Failed to close Spotlight response body: %v", closeErr)
822824
}
@@ -826,9 +828,6 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
826828

827829
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
828830
DebugLogger.Printf("Spotlight returned non-2xx status: %d", resp.StatusCode)
829-
if body, err := io.ReadAll(resp.Body); err == nil && len(body) > 0 {
830-
DebugLogger.Printf("Spotlight error response: %s", string(body))
831-
}
832831
} else {
833832
DebugLogger.Printf("Successfully sent event to Spotlight")
834833
}

0 commit comments

Comments
 (0)