-
Notifications
You must be signed in to change notification settings - Fork 722
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
Introduce contextual logging for correlation with tracing #2491
Comments
I had a brief look at this when I had some down time. The work I did can be found at https://github.com/charith-elastic/cloud-on-k8s/tree/refactor/logging. The biggest problem that I ran into was that the way the control flow is structured makes it harder to instrument in a useful way. (We can, of course, get some superficial spans. The issue is getting something substantial that actually helps track down problems easily.)
In my opinion, to get useful and actionable tracing information for the whole operator in one go will require a lot of disruptive changes to the codebase. Perhaps a better approach would be to make any new code instrumentable from the start, which will require some refactoring/deprecation of old utility code as well. Eventually, these incremental refactorings would make the codebase more friendly for full instrumentation. |
After using some of Charith's approach in #3775 to add contextual logging to a new controller (which really did most of the job already), I think my mind has changed on how we want to approach starting this. I think a separate PR to lay the groundwork for contextual logging on its own makes sense. New controller PRs are already very large and a pain to review, and the refactoring commit(s) to add contextual logging adds a lot of noise since you are touching even more files. Additionally, other people are also might be touching those controllers as well so keeping the branch up to date is annoying. Once the framework is added we can incrementally work on plumbing it through the rest of the code base. |
Follow up task from #1189
We are currently using package level logger that have no context about the current APM transaction or span. If we want to enable log correlation which would allow us to jump from APM tracing events to the corresponding logs we would need to switch to contextual loggers that are enriched with the trace.id and transaction.id from the current span and transaction respectively.
The text was updated successfully, but these errors were encountered: