Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x-pack/filebeat/input/httpjson: improve error logging #36529

Merged
merged 2 commits into from
Sep 11, 2023
Merged

x-pack/filebeat/input/httpjson: improve error logging #36529

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ecfe2d
x-pack/filebeat/input/httpjson: improve error logging
efd6 Sep 6, 2023
3d859c3
address pr comment
efd6 Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Add support for expanding `journald.process.capabilities` into the human-readable effective capabilities in the ECS `process.thread.capabilities.effective` field. {issue}36454[36454] {pull}36470[36470]
- Allow fine-grained control of entity analytics API requests for AzureAD provider. {issue}36440[36440] {pull}36441[36441]
- Added support for Okta OAuth2 provider in the CEL input. {issue}36336[36336] {pull}36521[36521]
- Improve error logging in HTTPJSON input. {pull}36529[36529]

*Auditbeat*

Expand Down
39 changes: 21 additions & 18 deletions x-pack/filebeat/input/httpjson/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (r *requester) doRequest(ctx context.Context, trCtx *transformContext, publ
// perform and store regular call responses
httpResp, err = rf.collectResponse(ctx, trCtx, r)
if err != nil {
return fmt.Errorf("failed to execute rf.collectResponse: %w", err)
return fmt.Errorf("failed to collect first response: %w", err)
}

if rf.saveFirstResponse {
Expand All @@ -70,7 +70,7 @@ func (r *requester) doRequest(ctx context.Context, trCtx *transformContext, publ
httpResp.Body = io.NopCloser(bytes.NewReader(body))
err = json.Unmarshal(body, &bodyMap)
if err != nil {
r.log.Errorf("unable to unmarshal first_response.body: %v", err)
r.log.Errorf("unable to unmarshal first_response.body: %v", textContextError{error: err, body: body})
}
firstResponse := response{
url: *httpResp.Request.URL,
Expand Down Expand Up @@ -165,7 +165,7 @@ func (r *requester) doRequest(ctx context.Context, trCtx *transformContext, publ
// collect data from new urls
httpResp, err = rf.collectResponse(ctx, chainTrCtx, r)
if err != nil {
return fmt.Errorf("failed to execute rf.collectResponse: %w", err)
return fmt.Errorf("failed to collect tail response %d: %w", i, err)
}
// store data according to response type
if i == len(r.requestFactories)-1 && len(ids) != 0 {
Expand Down Expand Up @@ -222,12 +222,12 @@ func (rf *requestFactory) collectResponse(ctx context.Context, trCtx *transformC
if rf.isChain && rf.chainClient != nil {
httpResp, err = rf.chainClient.do(ctx, req)
if err != nil {
return nil, fmt.Errorf("failed to execute chain http client.Do: %w", err)
return nil, fmt.Errorf("failed to execute chain http %s: %w", req.Method, err)
}
} else {
httpResp, err = r.client.do(ctx, req)
if err != nil {
return nil, fmt.Errorf("failed to execute http client.Do: %w", err)
return nil, fmt.Errorf("failed to execute http %s: %w", req.Method, err)
}
}

Expand All @@ -239,7 +239,7 @@ func (c *httpClient) do(ctx context.Context, req *http.Request) (*http.Response,
return c.client.Do(req)
})
if err != nil {
return nil, fmt.Errorf("failed to execute http client.Do: %w", err)
return nil, err
}
defer resp.Body.Close()

Expand All @@ -250,9 +250,12 @@ func (c *httpClient) do(ctx context.Context, req *http.Request) (*http.Response,
return nil, fmt.Errorf("failed to read response body: %w", err)
}

if resp.StatusCode > 399 {
if resp.StatusCode >= http.StatusBadRequest {
body, _ := io.ReadAll(resp.Body)
return nil, fmt.Errorf("server responded with status code %d: %s", resp.StatusCode, string(body))
if len(body) == 0 {
return nil, fmt.Errorf("server responded with status code %d", resp.StatusCode)
}
return nil, fmt.Errorf("server responded with status code %d: %s", resp.StatusCode, body)
}
return resp, nil
}
Expand Down Expand Up @@ -311,7 +314,7 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
ch.Step.Auth = tryAssignAuth(config.Auth, ch.Step.Auth)
client, err := newChainHTTPClient(ctx, ch.Step.Auth, ch.Step.Request, log, reg)
if err != nil {
return nil, fmt.Errorf("failed in creating chain http client with error : %w", err)
return nil, fmt.Errorf("failed in creating chain http client with error: %w", err)
}
if ch.Step.Auth != nil && ch.Step.Auth.Basic.isEnabled() {
rf.user = ch.Step.Auth.Basic.User
Expand Down Expand Up @@ -339,7 +342,7 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
ch.While.Auth = tryAssignAuth(config.Auth, ch.While.Auth)
client, err := newChainHTTPClient(ctx, ch.While.Auth, ch.While.Request, log, reg, policy)
if err != nil {
return nil, fmt.Errorf("failed in creating chain http client with error : %w", err)
return nil, fmt.Errorf("failed in creating chain http client with error: %w", err)
}
if ch.While.Auth != nil && ch.While.Auth.Basic.isEnabled() {
rf.user = ch.While.Auth.Basic.User
Expand Down Expand Up @@ -372,7 +375,7 @@ func evaluateResponse(expression *valueTpl, data []byte, log *logp.Logger) (bool

err := json.Unmarshal(data, &dataMap)
if err != nil {
return false, fmt.Errorf("error while unmarshalling data : %w", err)
return false, fmt.Errorf("error while unmarshalling data: %w", textContextError{error: err, body: data})
}
tr := transformable{}
paramCtx := &transformContext{
Expand All @@ -384,11 +387,11 @@ func evaluateResponse(expression *valueTpl, data []byte, log *logp.Logger) (bool

val, err := expression.Execute(paramCtx, tr, "", nil, log)
if err != nil {
return false, fmt.Errorf("error while evaluating expression : %w", err)
return false, fmt.Errorf("error while evaluating expression: %w", err)
}
result, err := strconv.ParseBool(val)
if err != nil {
return false, fmt.Errorf("error while parsing boolean value of string : %w", err)
return false, fmt.Errorf("error while parsing boolean value of string: %w", err)
}

return result, nil
Expand Down Expand Up @@ -508,7 +511,7 @@ func (r *requester) getIdsFromResponses(intermediateResps []*http.Response, repl
// get replace values from collected json
var v interface{}
if err := json.Unmarshal(b, &v); err != nil {
return nil, fmt.Errorf("cannot unmarshal data: %w", err)
return nil, fmt.Errorf("cannot unmarshal data: %w", textContextError{error: err, body: b})
}
values, err := jsonpath.Get(replace, v)
if err != nil {
Expand All @@ -529,7 +532,7 @@ func (r *requester) getIdsFromResponses(intermediateResps []*http.Response, repl
case float64, string:
ids = append(ids, fmt.Sprintf("%v", tresp))
default:
r.log.Errorf("cannot collect IDs from type '%T' : '%v'", values, values)
r.log.Errorf("cannot collect IDs from type %T: %v", values, values)
}
}
return ids, nil
Expand Down Expand Up @@ -660,21 +663,21 @@ func (r *requester) processChainPaginationEvents(ctx context.Context, trCtx *tra
// reformat urls of requestFactory using ids
rf.url, err = generateNewUrl(rf.replace, urlString, id)
if err != nil {
return -1, fmt.Errorf("failed to generate new URL: %w", err)
return -1, fmt.Errorf("failed to generate new url: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message seems to be same as https://github.com/elastic/beats/pull/36529/files#diff-ffe24395246b343e8006e4eaef52c2d3bf68c20497ac96ef1e626d4351258047R155. Since this is specific for chain requests, should we change it to failed to generate new url for chain pagination request? Ofcourse, we also have the line number inside the error trace. Just checking..

Copy link
Contributor Author

@efd6 efd6 Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll differentiate this. Thanks, I was trying to make them all searchably unique. Line numbers are OK, but error message strings are far more robust, and in this case the error is generated at a different site to where the log call will be made.

}

// reformat url accordingly if replaceWith clause exists
if doReplaceWith {
rf.url, err = generateNewUrl(strings.TrimSpace(replaceArr[0]), rf.url.String(), val)
if err != nil {
return n, fmt.Errorf("failed to generate new URL: %w", err)
return n, fmt.Errorf("failed to generate new url: %w", err)
}
}

// collect data from new urls
httpResp, err = rf.collectResponse(ctx, chainTrCtx, r)
if err != nil {
return -1, fmt.Errorf("failed to execute rf.collectResponse: %w", err)
return -1, fmt.Errorf("failed to collect response: %w", err)
}
// store data according to response type
if i == len(r.requestFactories)-1 && len(ids) != 0 {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/filebeat/input/httpjson/request_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func Test_evaluateResponse(t *testing.T) {
log: log,
},
want: false,
expectedError: "error while parsing boolean value of string : strconv.ParseBool: parsing \"eq .last_response.body.status \\\"completed\\\" ]]\": invalid syntax",
expectedError: "error while parsing boolean value of string: strconv.ParseBool: parsing \"eq .last_response.body.status \\\"completed\\\" ]]\": invalid syntax",
},
{
name: "newEvaluateResponse_emptyExpressionError",
Expand All @@ -226,7 +226,7 @@ func Test_evaluateResponse(t *testing.T) {
log: log,
},
want: false,
expectedError: "error while evaluating expression : the template result is empty",
expectedError: "error while evaluating expression: the template result is empty",
},
{
name: "newEvaluateResponse_incompleteExpressionError",
Expand All @@ -236,7 +236,7 @@ func Test_evaluateResponse(t *testing.T) {
log: log,
},
want: false,
expectedError: "error while parsing boolean value of string : strconv.ParseBool: parsing \"initiated\": invalid syntax",
expectedError: "error while parsing boolean value of string: strconv.ParseBool: parsing \"initiated\": invalid syntax",
},
}
for _, tt := range tests {
Expand Down
Loading