Add a SidekiqClientLogger to write job related events that happen on the client-side of sidekiq and configure Sidekiq.logger to write to that logger on the client.
We could then ingest sidekiq_client.log in (#201 (closed)) into the sidekiq index, so we have a single index where we can aggregate job information.
This means that developers can use Sidekiq.logger to write job related information, if this is done within a a job running on sidekiq server, this will go to STDOUT of sidekiq itself. When this is done on the client, the message would go to the new sidekiq_client.log.
I'm confused: why would we want to log both structured and unstructured logs for Sidekiq? Is anyone actually asking for the unstructured form? Gitlab::Multilogger was really meant as a transitional step (e.g. for AppLogger).
I'm confused: why would we want to log both structured and unstructured logs for Sidekiq? Is anyone actually asking for the unstructured form? Gitlab::Multilogger was really meant as a transitional step (e.g. for AppLogger).
@stanhu I'm suggesting to use the Gitlab::Multilogger for the same reason: The Gitlab::SidekiqLogger already exists in unstructured form. I'm not sure how widely used it is though: I thought it would be the safest bet to go to the transition the same way, but perhaps we can just skip it?
if there are shared keys between the two files, they'll need to have the same types.
@igorwwwwwwwwwwwwwwwwwwww Yep, I'm planning on extracting the job parsing stuff that's currently used in the StructuredLogger for jobs to make sure that the log entries look the same.
@reprazent Ah, right. Gitlab::SidekiqLogger is only used in two places as far as I can tell (RootStatisticsWorker and ScheduleAggregationWorker), so I think we can just skip it. Just use Sidekiq.logger.
@reprazent Does this mean that sidekiq related logs that were previously only going to production.log will now be logged to sidekiq_json.log in a structured manner?
Recently I did some tests to see what would happen for a failed export as part of the K8 migration of the project_export queue and got some interesting results:
For this test I prevented outbound connections to object storage from the sidekiq pod:
Does this mean that sidekiq related logs that were previously only going to production.log will now be logged to sidekiq_json.log in a structured manner?
@jarv No, the export itself uses it's own logger, which apparently gets directed to Rails.logger which is what writes to production.log.
A few interesting observations:
If you are only looking at the logs from sidekiq, everything went fine, it's only production.log that reports the failure.
there is a misleading "successfully exported" message right before the export error execution expired.
Import/Export - Project ansible-3 with ID: 500 successfully exported
Gets printed when the export is completely written to disk
While what you're seeing in the sidekiq stdout is the structured log of only the job. The job, through the Projects::ImportExport::ExportService deals with exceptions differently (in this case notifying the end user). So from sidekiq's perspective, the job succeeded and should not be retried or anything like that.
Long story short:
I think the logging for project export itself warrants another issue .
Bob Van Landuytchanged title from Turn the Gitlab::SidekiqLogger into a Gitlab::Multilogger that logs both structured and unstructured to Turn the Gitlab::SidekiqLogger into a Gitlab::JsonLogger that logs emits structured logs
changed title from Turn the Gitlab::SidekiqLogger into a Gitlab::Multilogger that logs both structured and unstructured to Turn the Gitlab::SidekiqLogger into a Gitlab::JsonLogger that logs emits structured logs
It turns out sidekiq.log is a bit of a special case, omnibus symlinks that to /var/log/gitlab/sidekiq/current to be able to display sidekiq logs in the admin interface. This doesn't work for multi-node installations or sidekiq cluster, but we can't break that functionality for other users. See the discussion in https://gitlab.slack.com/archives/C1FCTU4BE/p1583417541260900
So instead, I'll have sidekiq-client emit messages to a new file. I'll try to keep this configuration specific to sidekiq-client, so we can use Sidekiq.logger transparently.
I think we should consider removing the log viewing from the admin interface. It doesn't make any sense for HA installations, and I'm not aware of that many people using it.
@stanhu@marin Good point, though I don't think that would help with this particular issue, gitlab-rails/log/sidekiq.log is currently unused as an actual logfile. Only as a way for the application to read it for that dashboard, since writing to it wouldn't be visible anywhere on a HA setup.
So I've created gitlab-org/gitlab#209250 (closed) to get rid of it. At which point we can also remove the Gitlab::SidekiqLogger since sidekiq has its own logging setup in Gitlab::SidekiqLogging, used to get nice structured logs that actually relate to jobs.
Bob Van Landuytchanged title from Turn the Gitlab::SidekiqLogger into a Gitlab::JsonLogger that logs emits structured logs to {+Add a structured sidekiq_client.log file for job related messages for sidekiq client+}
changed title from Turn the Gitlab::SidekiqLogger into a Gitlab::JsonLogger that logs emits structured logs to {+Add a structured sidekiq_client.log file for job related messages for sidekiq client+}