Skip to content

Commit 7079ce9

Browse files
MovieStoreGuyMovieStoreGuy
committed
Making errors consistently formatted
1 parent e555170 commit 7079ce9

File tree

2 files changed

+43
-29
lines changed

2 files changed

+43
-29
lines changed

‎exporter/signalfxexporter/exporter_test.go

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"crypto/tls"
1010
"encoding/json"
11+
"errors"
1112
"fmt"
1213
"io"
1314
"net/http"
@@ -122,7 +123,7 @@ func TestConsumeMetrics(t *testing.T) {
122123
wantErr bool
123124
wantPermanentErr bool
124125
wantThrottleErr bool
125-
expectedErrorMsg string
126+
wantStatusCode int
126127
}{
127128
{
128129
name: "happy_path",
@@ -135,22 +136,23 @@ func TestConsumeMetrics(t *testing.T) {
135136
httpResponseCode: http.StatusForbidden,
136137
numDroppedTimeSeries: 1,
137138
wantErr: true,
138-
expectedErrorMsg: "HTTP 403 \"Forbidden\"",
139+
wantStatusCode: 403,
139140
},
140141
{
141142
name: "response_bad_request",
142143
md: smallBatch,
143144
httpResponseCode: http.StatusBadRequest,
144145
numDroppedTimeSeries: 1,
145146
wantPermanentErr: true,
146-
expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request",
147+
wantStatusCode: 400,
147148
},
148149
{
149150
name: "response_throttle",
150151
md: smallBatch,
151152
httpResponseCode: http.StatusTooManyRequests,
152153
numDroppedTimeSeries: 1,
153154
wantThrottleErr: true,
155+
wantStatusCode: 429,
154156
},
155157
{
156158
name: "response_throttle_with_header",
@@ -159,6 +161,7 @@ func TestConsumeMetrics(t *testing.T) {
159161
httpResponseCode: http.StatusServiceUnavailable,
160162
numDroppedTimeSeries: 1,
161163
wantThrottleErr: true,
164+
wantStatusCode: 503,
162165
},
163166
{
164167
name: "large_batch",
@@ -207,25 +210,30 @@ func TestConsumeMetrics(t *testing.T) {
207210
converter: c,
208211
}
209212

213+
errMsg := fmt.Sprintf("HTTP %q %d %q",
214+
serverURL.JoinPath("/v2/datapoint").String(),
215+
tt.wantStatusCode,
216+
http.StatusText(tt.wantStatusCode),
217+
)
218+
210219
numDroppedTimeSeries, err := dpClient.pushMetricsData(context.Background(), tt.md)
211220
assert.Equal(t, tt.numDroppedTimeSeries, numDroppedTimeSeries)
212221

213222
if tt.wantErr {
214223
assert.Error(t, err)
215-
assert.EqualError(t, err, tt.expectedErrorMsg)
224+
assert.EqualError(t, err, errMsg)
216225
return
217226
}
218227

219228
if tt.wantPermanentErr {
220229
assert.Error(t, err)
221230
assert.True(t, consumererror.IsPermanent(err))
222-
assert.True(t, strings.HasPrefix(err.Error(), tt.expectedErrorMsg))
223-
assert.ErrorContains(t, err, "response content")
231+
assert.ErrorContains(t, err, errMsg)
224232
return
225233
}
226234

227235
if tt.wantThrottleErr {
228-
expected := fmt.Errorf("HTTP %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode))
236+
expected := errors.New(errMsg)
229237
expected = exporterhelper.NewThrottleRetry(expected, time.Duration(tt.retryAfter)*time.Second)
230238
assert.EqualValues(t, expected, err)
231239
return
@@ -1892,8 +1900,8 @@ func TestConsumeMixedMetrics(t *testing.T) {
18921900
wantErr bool
18931901
wantPermanentErr bool
18941902
wantThrottleErr bool
1895-
expectedErrorMsg string
18961903
wantPartialMetricsErr bool
1904+
wantStatusCode int
18971905
}{
18981906
{
18991907
name: "happy_path",
@@ -1912,15 +1920,15 @@ func TestConsumeMixedMetrics(t *testing.T) {
19121920
sfxHTTPResponseCode: http.StatusForbidden,
19131921
numDroppedTimeSeries: 1,
19141922
wantErr: true,
1915-
expectedErrorMsg: "HTTP 403 \"Forbidden\"",
1923+
wantStatusCode: 403,
19161924
},
19171925
{
19181926
name: "response_forbidden_otlp",
19191927
md: smallBatchHistogramOnly,
19201928
otlpHTTPResponseCode: http.StatusForbidden,
19211929
numDroppedTimeSeries: 2,
19221930
wantErr: true,
1923-
expectedErrorMsg: "HTTP 403 \"Forbidden\"",
1931+
wantStatusCode: 403,
19241932
},
19251933
{
19261934
name: "response_forbidden_mixed",
@@ -1929,23 +1937,23 @@ func TestConsumeMixedMetrics(t *testing.T) {
19291937
otlpHTTPResponseCode: http.StatusForbidden,
19301938
numDroppedTimeSeries: 2,
19311939
wantErr: true,
1932-
expectedErrorMsg: "HTTP 403 \"Forbidden\"",
1940+
wantStatusCode: 403,
19331941
},
19341942
{
19351943
name: "response_bad_request_sfx",
19361944
md: smallBatch,
19371945
sfxHTTPResponseCode: http.StatusBadRequest,
19381946
numDroppedTimeSeries: 1,
19391947
wantPermanentErr: true,
1940-
expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request",
1948+
wantStatusCode: 400,
19411949
},
19421950
{
19431951
name: "response_bad_request_otlp",
19441952
md: smallBatchHistogramOnly,
19451953
otlpHTTPResponseCode: http.StatusBadRequest,
19461954
numDroppedTimeSeries: 2,
19471955
wantPermanentErr: true,
1948-
expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request",
1956+
wantStatusCode: 400,
19491957
},
19501958
{
19511959
name: "response_bad_request_mixed",
@@ -1954,14 +1962,15 @@ func TestConsumeMixedMetrics(t *testing.T) {
19541962
otlpHTTPResponseCode: http.StatusBadRequest,
19551963
numDroppedTimeSeries: 2,
19561964
wantPermanentErr: true,
1957-
expectedErrorMsg: "Permanent error: \"HTTP/1.1 400 Bad Request",
1965+
wantStatusCode: 400,
19581966
},
19591967
{
19601968
name: "response_throttle_sfx",
19611969
md: smallBatch,
19621970
sfxHTTPResponseCode: http.StatusTooManyRequests,
19631971
numDroppedTimeSeries: 1,
19641972
wantThrottleErr: true,
1973+
wantStatusCode: 429,
19651974
},
19661975
{
19671976
name: "response_throttle_mixed",
@@ -1971,6 +1980,7 @@ func TestConsumeMixedMetrics(t *testing.T) {
19711980
numDroppedTimeSeries: 2,
19721981
wantThrottleErr: true,
19731982
wantPartialMetricsErr: true,
1983+
wantStatusCode: 429,
19741984
},
19751985
{
19761986
name: "response_throttle_otlp",
@@ -1979,6 +1989,7 @@ func TestConsumeMixedMetrics(t *testing.T) {
19791989
numDroppedTimeSeries: 2,
19801990
wantThrottleErr: true,
19811991
wantPartialMetricsErr: true,
1992+
wantStatusCode: 429,
19821993
},
19831994
{
19841995
name: "response_throttle_with_header_sfx",
@@ -1987,6 +1998,7 @@ func TestConsumeMixedMetrics(t *testing.T) {
19871998
sfxHTTPResponseCode: http.StatusServiceUnavailable,
19881999
numDroppedTimeSeries: 1,
19892000
wantThrottleErr: true,
2001+
wantStatusCode: 503,
19902002
},
19912003
{
19922004
name: "response_throttle_with_header_otlp",
@@ -1996,6 +2008,7 @@ func TestConsumeMixedMetrics(t *testing.T) {
19962008
numDroppedTimeSeries: 2,
19972009
wantThrottleErr: true,
19982010
wantPartialMetricsErr: true,
2011+
wantStatusCode: 503,
19992012
},
20002013
{
20012014
name: "response_throttle_with_header_mixed",
@@ -2006,6 +2019,7 @@ func TestConsumeMixedMetrics(t *testing.T) {
20062019
numDroppedTimeSeries: 2,
20072020
wantThrottleErr: true,
20082021
wantPartialMetricsErr: true,
2022+
wantStatusCode: 503,
20092023
},
20102024
{
20112025
name: "large_batch",
@@ -2065,31 +2079,37 @@ func TestConsumeMixedMetrics(t *testing.T) {
20652079
numDroppedTimeSeries, err := sfxClient.pushMetricsData(context.Background(), tt.md)
20662080
assert.Equal(t, tt.numDroppedTimeSeries, numDroppedTimeSeries)
20672081

2082+
errMsg := fmt.Sprintf("HTTP %q %d %q",
2083+
serverURL.JoinPath("/v2/datapoint").String(),
2084+
tt.wantStatusCode,
2085+
http.StatusText(tt.wantStatusCode),
2086+
)
2087+
20682088
if tt.wantErr {
20692089
assert.Error(t, err)
2070-
assert.EqualError(t, err, tt.expectedErrorMsg)
2090+
assert.EqualError(t, err, errMsg)
20712091
return
20722092
}
20732093

20742094
if tt.wantPermanentErr {
2095+
errMsg = "Permanent error: " + errMsg
20752096
assert.Error(t, err)
20762097
assert.True(t, consumererror.IsPermanent(err))
2077-
assert.True(t, strings.HasPrefix(err.Error(), tt.expectedErrorMsg))
2078-
assert.ErrorContains(t, err, "response content")
2098+
assert.EqualError(t, err, errMsg)
20792099
return
20802100
}
20812101

20822102
if tt.wantThrottleErr {
20832103
if tt.wantPartialMetricsErr {
20842104
partialMetrics, _ := utils.GetHistograms(smallBatch)
2085-
throttleErr := fmt.Errorf("HTTP %d %q", tt.otlpHTTPResponseCode, http.StatusText(tt.otlpHTTPResponseCode))
2105+
throttleErr := errors.New(errMsg)
20862106
throttleErr = exporterhelper.NewThrottleRetry(throttleErr, time.Duration(tt.retryAfter)*time.Second)
20872107
testErr := consumererror.NewMetrics(throttleErr, partialMetrics)
20882108
assert.EqualValues(t, testErr, err)
20892109
return
20902110
}
20912111

2092-
expected := fmt.Errorf("HTTP %d %q", tt.sfxHTTPResponseCode, http.StatusText(tt.sfxHTTPResponseCode))
2112+
expected := errors.New(errMsg)
20932113
expected = exporterhelper.NewThrottleRetry(expected, time.Duration(tt.retryAfter)*time.Second)
20942114
assert.EqualValues(t, expected, err)
20952115
return

‎internal/splunk/httprequest.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,28 @@ package splunk // import "github.com/open-telemetry/opentelemetry-collector-cont
66
import (
77
"fmt"
88
"net/http"
9-
"net/http/httputil"
109
"strconv"
1110
"time"
1211

1312
"go.opentelemetry.io/collector/consumer/consumererror"
1413
"go.opentelemetry.io/collector/exporter/exporterhelper"
15-
"go.uber.org/multierr"
1614
)
1715

1816
const HeaderRetryAfter = "Retry-After"
1917

2018
// HandleHTTPCode handles an http response and returns the right type of error in case of a failure.
2119
func HandleHTTPCode(resp *http.Response) error {
2220
// Splunk accepts all 2XX codes.
23-
if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices {
21+
if resp.StatusCode/100 == 2 {
2422
return nil
2523
}
2624

2725
err := fmt.Errorf(
2826
"HTTP %q %d %q",
2927
resp.Request.URL.String(),
3028
resp.StatusCode,
31-
http.StatusText(resp.StatusCode))
29+
http.StatusText(resp.StatusCode),
30+
)
3231

3332
switch resp.StatusCode {
3433
// Check for responses that may include "Retry-After" header.
@@ -45,12 +44,7 @@ func HandleHTTPCode(resp *http.Response) error {
4544
err = exporterhelper.NewThrottleRetry(err, time.Duration(retryAfter)*time.Second)
4645
// Check for permanent errors.
4746
case http.StatusBadRequest, http.StatusUnauthorized:
48-
dump, err2 := httputil.DumpResponse(resp, true)
49-
if err2 == nil {
50-
err = consumererror.NewPermanent(fmt.Errorf("%w", fmt.Errorf("%q", dump)))
51-
} else {
52-
err = multierr.Append(err, err2)
53-
}
47+
err = consumererror.NewPermanent(err)
5448
}
5549

5650
return err

0 commit comments

Comments
 (0)