Fix mimirtool error logging

[dimitarvdimitrov]

Signed-off-by: Dimitar Dimitrov dimitar.dimitrov@grafana.com

What this PR does

When provided with --log.level=debug and a request fails mimirtool should print out some part of the response for easier debugging. Instead it uses a scanner which only reads the first token from the response.

The first token is by default the first line. Which in some cases can be fairly useless.

This PR

• changes the logged reported content to be the first 1024 bytes regardless of new lines.
• removes duplication of the error content (logging both errMsg and msg)
• increases the number of bytes to read from the body from 512 to 1024 for good measure.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

• NA Tests updated
• NA Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[aknuds1]

LGTM (except for a CHANGELOG typo).

[bboreham]

Maybe this should be %q, so any funny characters are visible?
I can see arguments on both sides whether newline or \n is easier for the reader, but a \t would definitely be clearer than some whitespace.

[dimitarvdimitrov]

pushed that change in da755e9

[bboreham]

Why did this change? I don't see it mentioned in the commit description.

[dimitarvdimitrov]

without this removal, the error message is repeated. errMsg already contains msg

mimir/pkg/mimirtool/client/client.go

Lines 180 to 194 in 465cbff

	msg := string(bodyHead)
	var errMsg string
	if msg == "" {
	errMsg = fmt.Sprintf("server returned HTTP status: %s", r.Status)
	} else {
	errMsg = fmt.Sprintf("server returned HTTP status: %s, body: %s", r.Status, msg)
	}
	if r.StatusCode == http.StatusNotFound {
	log.WithFields(log.Fields{
	"status": r.Status,
	}).Debugln(errMsg)
	return ErrResourceNotFound
	}

[aknuds1]

I noticed the same as @dimitarvdimitrov is saying, figured that's why msg gets removed.

[dimitarvdimitrov]

actually I went ahead and did some more changes. even with the removed msg (and with %q), there was still duplication

time="2022-10-11T07:28:28Z" level=error msg="server returned HTTP status: 400 Bad Request, body: \"block upload is disabled\\n\"" status="400 Bad Request"

in da755e9 I changed this to

time="2022-10-11T07:45:02Z" level=error msg=response body="block upload is disabled\n" status="400 Bad Request"

and an example flow is for a failed blocks upload

time="2022-10-11T07:45:02Z" level=info msg=Backfilling blocks=/shared/01GF301Z9T3CX0YK0VZ5B18MWJ user=anonymous
time="2022-10-11T07:45:02Z" level=info msg="making request to start block upload" block=01GF301Z9T3CX0YK0VZ5B18MWJ file=meta.json path=/shared/01GF301Z9T3CX0YK0VZ5B18MWJ
time="2022-10-11T07:45:02Z" level=error msg=response body="block upload is disabled\n" status="400 Bad Request"
time="2022-10-11T07:45:02Z" level=error msg="failed uploading block" error="request to start block upload failed: POST request to http://compactor:8080/api/v1/upload/block/01GF301Z9T3CX0YK0VZ5B18MWJ/start failed: server returned HTTP status: 400 Bad Request, body: \"block upload is disabled\\n\"" path=/shared/01GF301Z9T3CX0YK0VZ5B18MWJ
time="2022-10-11T07:45:02Z" level=info msg="finished uploading blocks" already_exists=0 failed=1 succeeded=0
mimirtool: error: blocks failed to upload 1 block(s), try --help

[dimitarvdimitrov]

can you please take another look @bboreham?

[pracucci]

This looks very useful to me. LGTM!