From fa2297b6166f37932e5baa46362585a4223fb7d9 Mon Sep 17 00:00:00 2001 From: sbylica-splunk Date: Wed, 9 Oct 2024 08:26:04 +0200 Subject: [PATCH] Error msg improvements --- .chloggen/hec_errors_msg_improvements.yaml | 27 +++++++++++++++ internal/splunk/httprequest.go | 20 ++++++++++++ internal/splunk/httprequest_test.go | 38 ++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100644 .chloggen/hec_errors_msg_improvements.yaml diff --git a/.chloggen/hec_errors_msg_improvements.yaml b/.chloggen/hec_errors_msg_improvements.yaml new file mode 100644 index 000000000000..eaf88b12585f --- /dev/null +++ b/.chloggen/hec_errors_msg_improvements.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: Added messages returned by Splunk to error displayed in logs when there is some issue with exporting events with HEC + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34339] + +# (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] \ No newline at end of file diff --git a/internal/splunk/httprequest.go b/internal/splunk/httprequest.go index cddc1598ccad..63473c797f9e 100644 --- a/internal/splunk/httprequest.go +++ b/internal/splunk/httprequest.go @@ -4,7 +4,10 @@ package splunk // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/splunk" import ( + "bytes" + "encoding/json" "fmt" + "io" "net/http" "net/http/httputil" "strconv" @@ -29,6 +32,23 @@ func HandleHTTPCode(resp *http.Response) error { resp.StatusCode, http.StatusText(resp.StatusCode)) + // Check if there is any error text returned by Splunk that we can append to the error message + if resp.Body != nil { + var jsonResponse map[string]any + bodyString, _ := io.ReadAll(resp.Body) + resp.Body = io.NopCloser(bytes.NewBuffer(bodyString)) + unmarshalError := json.Unmarshal(bodyString, &jsonResponse) + + if unmarshalError == nil { + responseErrorValue, ok := jsonResponse["text"] + if ok { + err = multierr.Append(err, fmt.Errorf("reason: %v", responseErrorValue)) + } else { + err = multierr.Append(err, fmt.Errorf("no error message from Splunk found")) + } + } + } + switch resp.StatusCode { // Check for responses that may include "Retry-After" header. case http.StatusTooManyRequests, http.StatusServiceUnavailable: diff --git a/internal/splunk/httprequest_test.go b/internal/splunk/httprequest_test.go index 99597839d01b..f253f2e3df2a 100644 --- a/internal/splunk/httprequest_test.go +++ b/internal/splunk/httprequest_test.go @@ -5,14 +5,17 @@ package splunk import ( "fmt" + "io" "net/http" "strconv" + "strings" "testing" "time" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.uber.org/multierr" ) func TestConsumeMetrics(t *testing.T) { @@ -20,9 +23,12 @@ func TestConsumeMetrics(t *testing.T) { name string httpResponseCode int retryAfter int + respBody string wantErr bool wantPermanentErr bool wantThrottleErr bool + wantErrMessage bool + noErrMessage bool }{ { name: "response_forbidden", @@ -49,6 +55,18 @@ func TestConsumeMetrics(t *testing.T) { name: "large_batch", httpResponseCode: http.StatusAccepted, }, + { + name: "response_disabled_token", + httpResponseCode: http.StatusForbidden, + respBody: "{\"text\":\"Token disabled\",\"code\":1}", + wantErrMessage: true, + }, + { + name: "response_no_text", + httpResponseCode: http.StatusForbidden, + respBody: "{\"code\":1}", + noErrMessage: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -61,6 +79,10 @@ func TestConsumeMetrics(t *testing.T) { } } + if tt.respBody != "" { + resp.Body = io.NopCloser(strings.NewReader(tt.respBody)) + } + err := HandleHTTPCode(resp) if tt.wantErr { assert.Error(t, err) @@ -80,6 +102,22 @@ func TestConsumeMetrics(t *testing.T) { return } + if tt.wantErrMessage { + assert.Error(t, err) + expected := fmt.Errorf("HTTP %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode)) + expected = multierr.Append(expected, fmt.Errorf("reason: %v", "Token disabled")) + assert.EqualValues(t, expected, err) + return + } + + if tt.noErrMessage { + assert.Error(t, err) + expected := fmt.Errorf("HTTP %d %q", tt.httpResponseCode, http.StatusText(tt.httpResponseCode)) + expected = multierr.Append(expected, fmt.Errorf("no error message from Splunk found")) + assert.EqualValues(t, expected, err) + return + } + assert.NoError(t, err) }) }