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

Only log the disablement of send_status once #448

Merged
merged 2 commits into from
Oct 4, 2022
Merged

Only log the disablement of send_status once #448

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
697b00d
Only log the disablement of send_status once
mpfz0r Sep 29, 2022
a04be42
Add missing trailing space to log message.
thll Oct 4, 2022
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
7 changes: 5 additions & 2 deletions api/graylog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@ func UpdateRegistration(httpClient *http.Client, checksum string, ctx *context.C
respBody := new(graylog.ResponseCollectorRegistration)
resp, err := c.Do(r, &respBody)
if resp != nil && resp.StatusCode == 400 && strings.Contains(err.Error(), "Unable to map property") {
Copy link
Member

Choose a reason for hiding this comment

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

This comment is not about your change but the behavior of the sidecar here. Do you know why we disable the status if we detect the substring in the error message?

If the substring would indicate "send_status" not working, then the log wouldn't show up more than once because we disabled the status submission. Or do I miss anything?

I also don't understand the "Disabling send_status as fallback." message. What does fallback mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing this was put in place to deal with older servers that didn't support status requests.
It catches the generic jackson error about unknown properties.

The idea of the "fallback" was to register at least a minimal request from the sidecar.
otherwise it would not be registered at all.

I've also made a change to the server that relaxes the NodeDetails with @JsonIgnoreProperties(ignoreUnknown = true)

log.Error("[UpdateRegistration] Sending collector status failed. Disabling `send_status` as fallback! ", err)
ctx.UserConfig.SendStatus = false
log.Error("[UpdateRegistration] Sending collector status failed.", err)
if ctx.UserConfig.SendStatus {
log.Error("[UpdateRegistration] Disabling `send_status` as fallback.")
ctx.UserConfig.SendStatus = false
}
} else if resp != nil && resp.StatusCode == 304 {
log.Debug("[UpdateRegistration] No update available.")
respBody.NotModified = true
Expand Down