diff --git a/.chloggen/msg_signalfx-exporter-include-endpoint-in-error.yaml b/.chloggen/msg_signalfx-exporter-include-endpoint-in-error.yaml new file mode 100644 index 0000000000000..b70f607055cf2 --- /dev/null +++ b/.chloggen/msg_signalfx-exporter-include-endpoint-in-error.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: signalfxexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Errors will now include the URL that it was trying to access + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [ 39026 ] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [ user ] diff --git a/.chloggen/msg_splunk-hec-include-endpoint-in-error.yaml b/.chloggen/msg_splunk-hec-include-endpoint-in-error.yaml new file mode 100644 index 0000000000000..32cff41392aa7 --- /dev/null +++ b/.chloggen/msg_splunk-hec-include-endpoint-in-error.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: splunkhecexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Errors will now include the URL that it was trying to access + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [ 39026 ] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/exporter/signalfxexporter/exporter_test.go b/exporter/signalfxexporter/exporter_test.go index 582b96f677f23..9cee6f75a54cf 100644 --- a/exporter/signalfxexporter/exporter_test.go +++ b/exporter/signalfxexporter/exporter_test.go @@ -8,6 +8,7 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -122,7 +123,7 @@ func TestConsumeMetrics(t *testing.T) { wantErr bool wantPermanentErr bool wantThrottleErr bool - expectedErrorMsg string + wantStatusCode int }{ { name: "happy_path", @@ -135,7 +136,7 @@ func TestConsumeMetrics(t *testing.T) { httpResponseCode: http.StatusForbidden, numDroppedTimeSeries: 1, wantErr: true, - expectedErrorMsg: "HTTP 403 \"Forbidden\"", + wantStatusCode: 403, }, { name: "response_bad_request", @@ -143,7 +144,7 @@ func TestConsumeMetrics(t *testing.T) { httpResponseCode: http.StatusBadRequest, numDroppedTimeSeries: 1, wantPermanentErr: true, - expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request", + wantStatusCode: 400, }, { name: "response_throttle", @@ -151,6 +152,7 @@ func TestConsumeMetrics(t *testing.T) { httpResponseCode: http.StatusTooManyRequests, numDroppedTimeSeries: 1, wantThrottleErr: true, + wantStatusCode: 429, }, { name: "response_throttle_with_header", @@ -159,6 +161,7 @@ func TestConsumeMetrics(t *testing.T) { httpResponseCode: http.StatusServiceUnavailable, numDroppedTimeSeries: 1, wantThrottleErr: true, + wantStatusCode: 503, }, { name: "large_batch", @@ -207,25 +210,29 @@ func TestConsumeMetrics(t *testing.T) { converter: c, } + errMsg := fmt.Sprintf("HTTP \"/v2/datapoint\" %d %q", + tt.wantStatusCode, + http.StatusText(tt.wantStatusCode), + ) + numDroppedTimeSeries, err := dpClient.pushMetricsData(context.Background(), tt.md) assert.Equal(t, tt.numDroppedTimeSeries, numDroppedTimeSeries) if tt.wantErr { assert.Error(t, err) - assert.EqualError(t, err, tt.expectedErrorMsg) + assert.EqualError(t, err, errMsg) return } if tt.wantPermanentErr { assert.Error(t, err) assert.True(t, consumererror.IsPermanent(err)) - assert.True(t, strings.HasPrefix(err.Error(), tt.expectedErrorMsg)) - assert.ErrorContains(t, err, "response content") + assert.ErrorContains(t, err, errMsg) return } if tt.wantThrottleErr { - expected := fmt.Errorf("HTTP %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode)) + expected := errors.New(errMsg) expected = exporterhelper.NewThrottleRetry(expected, time.Duration(tt.retryAfter)*time.Second) assert.EqualValues(t, expected, err) return @@ -1892,8 +1899,8 @@ func TestConsumeMixedMetrics(t *testing.T) { wantErr bool wantPermanentErr bool wantThrottleErr bool - expectedErrorMsg string wantPartialMetricsErr bool + wantStatusCode int }{ { name: "happy_path", @@ -1912,7 +1919,7 @@ func TestConsumeMixedMetrics(t *testing.T) { sfxHTTPResponseCode: http.StatusForbidden, numDroppedTimeSeries: 1, wantErr: true, - expectedErrorMsg: "HTTP 403 \"Forbidden\"", + wantStatusCode: 403, }, { name: "response_forbidden_otlp", @@ -1920,7 +1927,7 @@ func TestConsumeMixedMetrics(t *testing.T) { otlpHTTPResponseCode: http.StatusForbidden, numDroppedTimeSeries: 2, wantErr: true, - expectedErrorMsg: "HTTP 403 \"Forbidden\"", + wantStatusCode: 403, }, { name: "response_forbidden_mixed", @@ -1929,7 +1936,7 @@ func TestConsumeMixedMetrics(t *testing.T) { otlpHTTPResponseCode: http.StatusForbidden, numDroppedTimeSeries: 2, wantErr: true, - expectedErrorMsg: "HTTP 403 \"Forbidden\"", + wantStatusCode: 403, }, { name: "response_bad_request_sfx", @@ -1937,7 +1944,7 @@ func TestConsumeMixedMetrics(t *testing.T) { sfxHTTPResponseCode: http.StatusBadRequest, numDroppedTimeSeries: 1, wantPermanentErr: true, - expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request", + wantStatusCode: 400, }, { name: "response_bad_request_otlp", @@ -1945,7 +1952,7 @@ func TestConsumeMixedMetrics(t *testing.T) { otlpHTTPResponseCode: http.StatusBadRequest, numDroppedTimeSeries: 2, wantPermanentErr: true, - expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request", + wantStatusCode: 400, }, { name: "response_bad_request_mixed", @@ -1954,7 +1961,7 @@ func TestConsumeMixedMetrics(t *testing.T) { otlpHTTPResponseCode: http.StatusBadRequest, numDroppedTimeSeries: 2, wantPermanentErr: true, - expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request", + wantStatusCode: 400, }, { name: "response_throttle_sfx", @@ -1962,6 +1969,7 @@ func TestConsumeMixedMetrics(t *testing.T) { sfxHTTPResponseCode: http.StatusTooManyRequests, numDroppedTimeSeries: 1, wantThrottleErr: true, + wantStatusCode: 429, }, { name: "response_throttle_mixed", @@ -1971,6 +1979,7 @@ func TestConsumeMixedMetrics(t *testing.T) { numDroppedTimeSeries: 2, wantThrottleErr: true, wantPartialMetricsErr: true, + wantStatusCode: 429, }, { name: "response_throttle_otlp", @@ -1979,6 +1988,7 @@ func TestConsumeMixedMetrics(t *testing.T) { numDroppedTimeSeries: 2, wantThrottleErr: true, wantPartialMetricsErr: true, + wantStatusCode: 429, }, { name: "response_throttle_with_header_sfx", @@ -1987,6 +1997,7 @@ func TestConsumeMixedMetrics(t *testing.T) { sfxHTTPResponseCode: http.StatusServiceUnavailable, numDroppedTimeSeries: 1, wantThrottleErr: true, + wantStatusCode: 503, }, { name: "response_throttle_with_header_otlp", @@ -1996,6 +2007,7 @@ func TestConsumeMixedMetrics(t *testing.T) { numDroppedTimeSeries: 2, wantThrottleErr: true, wantPartialMetricsErr: true, + wantStatusCode: 503, }, { name: "response_throttle_with_header_mixed", @@ -2006,6 +2018,7 @@ func TestConsumeMixedMetrics(t *testing.T) { numDroppedTimeSeries: 2, wantThrottleErr: true, wantPartialMetricsErr: true, + wantStatusCode: 503, }, { name: "large_batch", @@ -2065,31 +2078,36 @@ func TestConsumeMixedMetrics(t *testing.T) { numDroppedTimeSeries, err := sfxClient.pushMetricsData(context.Background(), tt.md) assert.Equal(t, tt.numDroppedTimeSeries, numDroppedTimeSeries) + errMsg := fmt.Sprintf("HTTP \"/v2/datapoint\" %d %q", + tt.wantStatusCode, + http.StatusText(tt.wantStatusCode), + ) + if tt.wantErr { assert.Error(t, err) - assert.EqualError(t, err, tt.expectedErrorMsg) + assert.EqualError(t, err, errMsg) return } if tt.wantPermanentErr { + errMsg = "Permanent error: " + errMsg assert.Error(t, err) assert.True(t, consumererror.IsPermanent(err)) - assert.True(t, strings.HasPrefix(err.Error(), tt.expectedErrorMsg)) - assert.ErrorContains(t, err, "response content") + assert.EqualError(t, err, errMsg) return } if tt.wantThrottleErr { if tt.wantPartialMetricsErr { partialMetrics, _ := utils.GetHistograms(smallBatch) - throttleErr := fmt.Errorf("HTTP %d %q", tt.otlpHTTPResponseCode, http.StatusText(tt.otlpHTTPResponseCode)) + throttleErr := errors.New(errMsg) throttleErr = exporterhelper.NewThrottleRetry(throttleErr, time.Duration(tt.retryAfter)*time.Second) testErr := consumererror.NewMetrics(throttleErr, partialMetrics) assert.EqualValues(t, testErr, err) return } - expected := fmt.Errorf("HTTP %d %q", tt.sfxHTTPResponseCode, http.StatusText(tt.sfxHTTPResponseCode)) + expected := errors.New(errMsg) expected = exporterhelper.NewThrottleRetry(expected, time.Duration(tt.retryAfter)*time.Second) assert.EqualValues(t, expected, err) return diff --git a/exporter/splunkhecexporter/client_test.go b/exporter/splunkhecexporter/client_test.go index 8a6ab38c6e89a..8b69e76df41db 100644 --- a/exporter/splunkhecexporter/client_test.go +++ b/exporter/splunkhecexporter/client_test.go @@ -66,6 +66,7 @@ func newTestClientWithPresetResponses(codes []int, bodies []string) (*http.Clien return &http.Response{ StatusCode: code, + Request: req, Body: io.NopCloser(bytes.NewBufferString(body)), Header: make(http.Header), } @@ -1316,7 +1317,11 @@ func TestErrorReceived(t *testing.T) { case <-time.After(5 * time.Second): t.Fatal("Should have received request") } - assert.EqualError(t, err, "HTTP 500 \"Internal Server Error\"") + errMsg := fmt.Sprintf("HTTP \"/services/collector\" %d %q", + http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError), + ) + assert.EqualError(t, err, errMsg) } func TestInvalidLogs(t *testing.T) { @@ -1394,7 +1399,9 @@ func TestHeartbeatStartupFailed(t *testing.T) { assert.NoError(t, err) assert.EqualError(t, exporter.Start(context.Background(), componenttest.NewNopHost()), - fmt.Sprintf("%s: heartbeat on startup failed: HTTP 403 \"Forbidden\"", params.ID.String()), + fmt.Sprintf("%s: heartbeat on startup failed: HTTP \"/services/collector\" 403 \"Forbidden\"", + params.ID.String(), + ), ) assert.NoError(t, exporter.Shutdown(context.Background())) } @@ -1593,7 +1600,7 @@ func Test_pushLogData_PostError(t *testing.T) { func Test_pushLogData_ShouldAddResponseTo400Error(t *testing.T) { config := NewFactory().CreateDefaultConfig().(*Config) - url := &url.URL{Scheme: "http", Host: "splunk"} + url := &url.URL{Scheme: "http", Host: "splunk", Path: "/v1/endpoint"} splunkClient := newLogsClient(exportertest.NewNopSettings(metadata.Type), NewFactory().CreateDefaultConfig().(*Config)) logs := createLogData(1, 1, 1) @@ -1605,9 +1612,8 @@ func Test_pushLogData_ShouldAddResponseTo400Error(t *testing.T) { // Sending logs using the client. err := splunkClient.pushLogData(context.Background(), logs) require.True(t, consumererror.IsPermanent(err), "Expecting permanent error") - require.ErrorContains(t, err, "HTTP/0.0 400") + require.EqualError(t, err, "Permanent error: HTTP \"/v1/endpoint\" 400 \"Bad Request\"") // The returned error should contain the response body responseBody. - assert.ErrorContains(t, err, responseBody) // An HTTP client that returns some other status code other than 400 and response body responseBody. httpClient, _ = newTestClient(500, responseBody) @@ -1615,7 +1621,7 @@ func Test_pushLogData_ShouldAddResponseTo400Error(t *testing.T) { // Sending logs using the client. err = splunkClient.pushLogData(context.Background(), logs) require.False(t, consumererror.IsPermanent(err), "Expecting non-permanent error") - require.ErrorContains(t, err, "HTTP 500") + require.EqualError(t, err, "HTTP \"/v1/endpoint\" 500 \"Internal Server Error\"") // The returned error should not contain the response body responseBody. assert.NotContains(t, err.Error(), responseBody) } diff --git a/internal/splunk/go.mod b/internal/splunk/go.mod index 2c7adfdd9a820..3d292db9fba8e 100644 --- a/internal/splunk/go.mod +++ b/internal/splunk/go.mod @@ -9,7 +9,6 @@ require ( go.opentelemetry.io/collector/pdata v1.29.0 go.opentelemetry.io/collector/semconv v0.123.0 go.uber.org/goleak v1.3.0 - go.uber.org/multierr v1.11.0 ) require ( @@ -47,6 +46,7 @@ require ( go.opentelemetry.io/otel/metric v1.35.0 // indirect go.opentelemetry.io/otel/sdk v1.35.0 // indirect go.opentelemetry.io/otel/trace v1.35.0 // indirect + go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect golang.org/x/net v0.37.0 // indirect golang.org/x/sys v0.31.0 // indirect diff --git a/internal/splunk/httprequest.go b/internal/splunk/httprequest.go index cddc1598ccad0..eda7d8fba5159 100644 --- a/internal/splunk/httprequest.go +++ b/internal/splunk/httprequest.go @@ -6,13 +6,11 @@ package splunk // import "github.com/open-telemetry/opentelemetry-collector-cont import ( "fmt" "net/http" - "net/http/httputil" "strconv" "time" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/exporter/exporterhelper" - "go.uber.org/multierr" ) const HeaderRetryAfter = "Retry-After" @@ -25,9 +23,11 @@ func HandleHTTPCode(resp *http.Response) error { } err := fmt.Errorf( - "HTTP %d %q", + "HTTP %q %d %q", + resp.Request.URL.Path, resp.StatusCode, - http.StatusText(resp.StatusCode)) + http.StatusText(resp.StatusCode), + ) switch resp.StatusCode { // Check for responses that may include "Retry-After" header. @@ -44,12 +44,7 @@ func HandleHTTPCode(resp *http.Response) error { err = exporterhelper.NewThrottleRetry(err, time.Duration(retryAfter)*time.Second) // Check for permanent errors. case http.StatusBadRequest, http.StatusUnauthorized: - dump, err2 := httputil.DumpResponse(resp, true) - if err2 == nil { - err = consumererror.NewPermanent(fmt.Errorf("%w", fmt.Errorf("%q", dump))) - } else { - err = multierr.Append(err, err2) - } + err = consumererror.NewPermanent(err) } return err diff --git a/internal/splunk/httprequest_test.go b/internal/splunk/httprequest_test.go index 99597839d01b5..fb39bdc59c7f9 100644 --- a/internal/splunk/httprequest_test.go +++ b/internal/splunk/httprequest_test.go @@ -6,6 +6,7 @@ package splunk import ( "fmt" "net/http" + "net/url" "strconv" "testing" "time" @@ -53,6 +54,9 @@ func TestConsumeMetrics(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { resp := &http.Response{ + Request: &http.Request{ + URL: &url.URL{Scheme: "http", Host: "splunk.com", Path: "/endpoint"}, + }, StatusCode: tt.httpResponseCode, } if tt.retryAfter != 0 { @@ -74,7 +78,7 @@ func TestConsumeMetrics(t *testing.T) { } if tt.wantThrottleErr { - expected := fmt.Errorf("HTTP %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode)) + expected := fmt.Errorf("HTTP \"/endpoint\" %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode)) expected = exporterhelper.NewThrottleRetry(expected, time.Duration(tt.retryAfter)*time.Second) assert.EqualValues(t, expected, err) return