Sidekiq:Add trace correlation for internal logs #3823
Conversation
marcotc
added
the
feature-request
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
| # The main method used by Sidekiq to process jobs. | ||
| # The Sidekiq logger runs inside this method. | ||
| # @see Sidekiq::Processor#dispatch | ||
| def dispatch(*args, **kwargs, &block) |
There was a problem hiding this comment.
⚪ Code Quality Violation
Optional arguments should appear at the end (...read more)
The rule "Optional arguments should appear at the end" is an important programming practice in Ruby. It is used to ensure that the optional parameters in a method definition are placed after the required parameters. This rule is important because when a method is called, Ruby fills in the arguments from left to right. If an optional argument is placed before a required argument and the method is called with fewer arguments, Ruby will assign the provided arguments to the optional parameters, leaving the required parameters without values and causing an error.
Non-compliance with this rule often leads to unexpected behavior or bugs in the code, which can be quite challenging to debug. This is particularly true when the method is called with fewer arguments than defined. The errors caused by this can be hard to track down, as they may not occur at the place where the method is defined, but rather in some distant place where the method is called.
To avoid this, always place optional parameters at the end of the list of parameters in your method definitions. This way, Ruby will fill in the required parameters first, and only then use any remaining arguments for the optional ones. If there are no remaining arguments, the optional parameters will be set to their default values. This keeps your code clear, predictable, and free of unnecessary bugs.
|
To get all test scenarios to pass, I'll try a different approach of focusing the code changes on the JobLogger. |
|
Any more luck on this one @marcotc ? Very eager to get correlated logs for sidekiq 🙌🏼 |
|
It's way trickier that we though, sorry @alexevanczuk, we are still trying to figure out the best approach. |
|
Okay @marcotc , thanks for your perseverance!! |
|
Will this be continued @marcotc? 👀 If I understand correctly, after this PR, we just need to setup the necessary config for Datadog when initialize Sidekiq, and then the logs can be automatically attached with fields for tracing correlation like trace_id, etc.? |
Fixes #1068
This PR ensures that a trace is active so that Sidekiq's logger instance can emit correlated logs before and after a job is executed.
Motivation:
By default, Sidekiq logs a few messages before and after a job is executed in the Sidekiq server process.
These messages happen outside the context of Sidekiq server's middlewares. And because APM traces are created from a servers middleware, these log entries are not correlated with an APM trace.
These logs are important because they are emitted by default, and not having them correlated with traces creates a bad experience out-of-the-box when inspecting the logs from a new Sidekiq server.
How to test the change?
There are new integration tests to run a realistic Sidekiq server.