receive: Improve receive error logging #2773
Conversation
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
kakkoyun
force-pushed
the
receive_logging
branch
from
df235a5
to
e5f2ca0
Compare
| // Copyright (c) The Thanos Authors. | ||
| // Licensed under the Apache License 2.0. | ||
|
|
||
| package middleware |
There was a problem hiding this comment.
Looks great, however, do we want to correlate the trace-id with request-id? Maybe my idea would be suitable for another PR for correlating them.
Otherwise looks great 💯
There was a problem hiding this comment.
We can try correlate or use a single id for both. We need to check how actually different middleware interacts and what happens if tracing is not enabled. For the scope of this PR, I want to keep track of execution of async forward requests in the scope a single request.
There was a problem hiding this comment.
Aligning everything with the W3C trace context would probably work, but doesn't block this I would say.
There was a problem hiding this comment.
Awesome looks like first attempt for request logging @yashrsharma44 - do you mind helping us in review? (:
| // Copyright (c) The Thanos Authors. | ||
| // Licensed under the Apache License 2.0. | ||
|
|
||
| package middleware |
There was a problem hiding this comment.
Looks great, however, do we want to correlate the trace-id with request-id? Maybe my idea would be suitable for another PR for correlating them.
Otherwise looks great 💯
Changes
Verification
make test-localMINIO_ENABLED=1 REMOTE_WRITE_ENABLED=1 ./scripts/quickstart.sh