-
Notifications
You must be signed in to change notification settings - Fork 812
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.
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
Add tracing to the tenant federation mergeQueryable #4147
Comments
Your first linked example is what I would suggest to use… getting spanlogger from existing context, closing it at the end of method, and passing new context to the called methods is a good start. Additionally logging supplied tenantIDs via new spanlogger (to make them visible when viewing the trace) would be useful. We can go even further and create one child span per processed tenant – that can be useful for people troubleshooting slow queries to understand if there is specific tenant that was slow for some reason. |
1. added logic to add tag `tenant_id` to tracing spans if tenant is defined in context. 2. added additional tracing spans to merge_querier's methods Signed-off-by: Vladyslav Diachenko <[email protected]>
I have implemented a logic to add
|
Signed-off-by: Vladyslav Diachenko <[email protected]>
1.changed value of tenant_id tag in tracing span from string value to array 2. removed import alias for spanlogger Signed-off-by: Vladyslav Diachenko <[email protected]>
1. fixed tests 2. fixed name of the spans in mergeQuerier Signed-off-by: Vladyslav Diachenko <[email protected]>
Signed-off-by: Vladyslav Diachenko <[email protected]>
…gSlice function to create inner spans under this context Signed-off-by: Vladyslav Diachenko <[email protected]>
Signed-off-by: Vladyslav Diachenko <[email protected]>
…am that is not used in callback function Signed-off-by: Vladyslav Diachenko <[email protected]>
Signed-off-by: Vladyslav Diachenko <[email protected]>
* #4147 1. added logic to add tag `tenant_id` to tracing spans if tenant is defined in context. 2. added additional tracing spans to merge_querier's methods Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed errors reported by linters Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 1.changed value of tenant_id tag in tracing span from string value to array 2. removed import alias for spanlogger Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 1. fixed tests 2. fixed name of the spans in mergeQuerier Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 updated CHANGELOG.md Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 passed method's span context to mergeDistinctStringSlice function to create inner spans under this context Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 added mocktracer package for testing purpose Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed code style and removed redundant context param that is not used in callback function Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed code style Signed-off-by: Vladyslav Diachenko <[email protected]>
This is a work item that came out the review of #4117
The mergeQueryable aggregates data from underlying queryables concurrently. Understanding the call sequence is only possible providing spans.
/assign @simonswine
Some examples that creates spans:
cortex/pkg/querier/blocks_store_queryable.go
Line 318 in d382e1d
cortex/pkg/querier/worker/scheduler_processor.go
Line 134 in 5009276
The text was updated successfully, but these errors were encountered: