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

Author: DAcodedBEAT

client.go

@@ -346,6 +346,13 @@ func NewClient(options ClientOptions) (*Client, error) {
 		options.TraceIgnoreStatusCodes = [][]int{{404}}
 	}
 
+	// Check for Spotlight environment variable
+	if !options.Spotlight {
+		if spotlightEnv := os.Getenv("SENTRY_SPOTLIGHT"); spotlightEnv == "true" || spotlightEnv == "1" {
+			options.Spotlight = true
+		}
+	}
+
 	// SENTRYGODEBUG is a comma-separated list of key=value pairs (similar
 	// to GODEBUG). It is not a supported feature: recognized debug options
 	// may change any time.
@@ -402,12 +409,6 @@ func NewClient(options ClientOptions) (*Client, error) {
 }
 
 func (client *Client) setupTransport() {
-	if !client.options.Spotlight {
-		if spotlightEnv := os.Getenv("SENTRY_SPOTLIGHT"); spotlightEnv == "true" || spotlightEnv == "1" {
-			client.options.Spotlight = true
-		}
-	}
-
 	opts := client.options
 	transport := opts.Transport
 

spotlight_test.go

@@ -1,7 +1,6 @@
 package sentry
 
 import (
-	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -27,21 +26,27 @@ func TestSpotlightTransport(t *testing.T) {
 	}))
 	defer server.Close()
 
-	mock := &mockTransport{}
+	mock := &MockTransport{}
 	st := NewSpotlightTransport(mock)
 	st.Configure(ClientOptions{SpotlightURL: server.URL + "/stream"})
 
 	event := NewEvent()
+	event.Sdk.Name = "sentry-go"
+	event.Sdk.Version = SDKVersion
 	event.Message = "Test message"
 	st.SendEvent(event)
 
 	time.Sleep(100 * time.Millisecond)
 
-	if len(mock.events) != 1 {
-		t.Errorf("Expected 1 event, got %d", len(mock.events))
+	if len(mock.Events()) != 1 {
+		t.Errorf("Expected 1 event, got %d", len(mock.Events()))
 	}
-	if mock.events[0].Message != "Test message" {
-		t.Errorf("Expected 'Test message', got %s", mock.events[0].Message)
+	if mock.Events()[0].Message != "Test message" {
+		t.Errorf("Expected 'Test message', got %s", mock.Events()[0].Message)
+	}
+
+	if !st.Flush(time.Second) {
+		t.Errorf("Expected Flush to succeed")
 	}
 }
 
@@ -147,24 +152,3 @@ func TestSpotlightClientOptions(t *testing.T) {
 		})
 	}
 }
-
-// mockTransport is a simple transport for testing.
-type mockTransport struct {
-	events []*Event
-}
-
-func (m *mockTransport) Configure(ClientOptions) {}
-
-func (m *mockTransport) SendEvent(event *Event) {
-	m.events = append(m.events, event)
-}
-
-func (m *mockTransport) Flush(time.Duration) bool {
-	return true
-}
-
-func (m *mockTransport) FlushWithContext(_ context.Context) bool {
-	return true
-}
-
-func (m *mockTransport) Close() {}

transport.go

@@ -487,11 +487,7 @@ started:
 	}
 
 fail:
-	if itemCount := len(b.items); itemCount > 0 {
-		debuglog.Printf("Buffer flushing was canceled or timed out. %d items were not flushed.", itemCount)
-	} else {
-		debuglog.Println("Buffer flushing was canceled or timed out. No items were pending.")
-	}
+	debuglog.Println("Buffer flushing was canceled or timed out. Some events may not have been sent.")
 	return false
 }
 
@@ -776,16 +772,15 @@ func (st *SpotlightTransport) Configure(options ClientOptions) {
 }
 
 func (st *SpotlightTransport) SendEvent(event *Event) {
-	// Send to the underlying transport (Sentry) unless it's noopTransport
-	if _, isNoop := st.underlying.(noopTransport); !isNoop {
-		st.underlying.SendEvent(event)
-	}
+	// Send to the underlying transport (Sentry)
+	st.underlying.SendEvent(event)
 
 	// Always send to Spotlight
 	st.sendToSpotlight(event)
 }
 
 func (st *SpotlightTransport) sendToSpotlight(event *Event) {
+	ctx := context.Background()
 	dsn, _ := NewDsn("https://placeholder@localhost/1")
 
 	eventBody := getRequestBodyFromEvent(event)
@@ -800,14 +795,14 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
 		return
 	}
 
-	req, err := http.NewRequest("POST", st.spotlightURL, envelope)
+	req, err := http.NewRequestWithContext(ctx, "POST", st.spotlightURL, envelope)
 	if err != nil {
 		DebugLogger.Printf("Failed to create Spotlight request: %v", err)
 		return
 	}
 
 	req.Header.Set("Content-Type", "application/x-sentry-envelope")
-	req.Header.Set("User-Agent", "sentry-go/"+SDKVersion)
+	req.Header.Set("User-Agent", fmt.Sprintf("%s/%s", event.Sdk.Name, event.Sdk.Version))
 
 	DebugLogger.Printf("Sending event to Spotlight at %s", st.spotlightURL)
 
@@ -817,6 +812,9 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
 		return
 	}
 	defer func() {
+		// Drain body up to a limit and close it, allowing the
+		// transport to reuse TCP connections.
+		_, _ = io.CopyN(io.Discard, resp.Body, maxDrainResponseBytes)
 		if closeErr := resp.Body.Close(); closeErr != nil {
 			DebugLogger.Printf("Failed to close Spotlight response body: %v", closeErr)
 		}
@@ -826,9 +824,6 @@ func (st *SpotlightTransport) sendToSpotlight(event *Event) {
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		DebugLogger.Printf("Spotlight returned non-2xx status: %d", resp.StatusCode)
-		if body, err := io.ReadAll(resp.Body); err == nil && len(body) > 0 {
-			DebugLogger.Printf("Spotlight error response: %s", string(body))
-		}
 	} else {
 		DebugLogger.Printf("Successfully sent event to Spotlight")
 	}