Add query logging/tracing

[bwplotka]

We should log queries and preserve those optionally in tracing. This is to clearly figure out which query caused unexpected behavior or hit limit!

Nice to have: middleware logging: grpc-ecosystem/go-grpc-middleware#223 cc @adrien-f (: Although we can host something temporarily.

Related Prometheus issue: prometheus/prometheus#6223

AC :

• use separate request logger derived from base logger (only tags)
• Use the same request-id and trace id for those.
• Add as meaningful query statistics (it is only put inside metric system so far)
• Be careful on sensitive data in labels 🤔

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

[bwplotka]

We need that!

…

On Thu, 27 Feb 2020 at 11:14, stale[bot] ***@***.***> wrote: This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1706?email_source=notifications&email_token=ABVA3O55SFMX6J4FJ6D4FSDRE6OCTA5CNFSM4JH2VRSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEND65SY#issuecomment-591916747>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O7MH2CTC5GVIGGAUYDRE6OCTANCNFSM4JH2VRSA> .

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

[bwplotka]

Still needed, no progress on this yet. Help wanted!

@mkabischev did something promising!

#2321 (comment)

[bwplotka]

Tracing is done, but we might want to log more on debug.

[bwplotka]

This was merged: grpc-ecosystem/go-grpc-middleware#223 so we might want to think about upgrading middleware and getting this in.

However the current version is not fully correct on middleware sides, the v2 works just started, we could do that first: grpc-ecosystem/go-grpc-middleware#275

[vaibhavgulati]

valgrind-out.txt
Ran valgrind over thano0.12.0 it gave few errors I don't know if its helpful for you to find out memory leaks.

[obiesmans]

Small feedback as a tracing user :

It's working great and it's a really useful tool to troubleshoot our Thanos architecture.
Filtering sensitive data could be a neat addition (our GCS service-account key is in ENV).
Also, having queryiers/stores/sidecar logging slow queries could be useful to those not willing to invest in a tracing infrastructure yet.

[yashrsharma44]

Is this issue grpc-ecosystem/go-grpc-middleware#275 a blocker for solving #1706(Query Logging)?

[bwplotka]

Well not blocker, just some dependency we need to work on. I am maintainer on grpc middleware, as well, so yea we might want to push something forward.

TBH the v2 is mainly done so we can just try upgrade Thanos to that (: (just some minor import tweaks I need to fix first, probably tomorrow)

[aymericDD]

Still an issue

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

Let's get back to this since @aymericDD want to help in this.

Let's figure out what exactly is missing. Sounds to me like tracing is done. But logging might need more metadata/better usability.

• Hard to discover logs that are interesting
• Ideally we have start call and end call with some request id

Feel free @aymericDD to give more details on what's your current problem is.

[bwplotka]

Let's make sure we reuse exactly the same statistics and metadata as for tracing.

[bwplotka]

FYI https://thanos.io/tip/proposals-done/202005-query-logging.md/

[bwplotka]

Some starting point:

thanos/pkg/server/grpc/grpc.go

Line 85 in 24602c4

grpc_logging.UnaryServerInterceptor(kit.InterceptorLogger(logger), logOpts...),

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[aymericDD]

Still an issue

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[GiedriusS]

#5521 done