Skip to content
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 logging when job starts #4

Closed
stanhu opened this issue Apr 1, 2018 · 6 comments · Fixed by #13
Closed

Add logging when job starts #4

stanhu opened this issue Apr 1, 2018 · 6 comments · Fixed by #13

Comments

@stanhu
Copy link

stanhu commented Apr 1, 2018

I noticed sidekiq-logstash doesn't log an event when a job starts:

Sidekiq.logger.info log_job(job, started_at)

That seems like valuable information. Is there any reason this isn't being done?

@iMacTia
Copy link
Owner

iMacTia commented Apr 2, 2018

Hi @stanhu,
that's a good question. Main reason is that I wanted to achieve a 1-line-per-job log, similarly to what lograge does for rails logs. This makes the integration and organisation in Logstash/Kibana (ELK stack) easier, which is the one I was using.
I'd like to keep this as the default behaviour, however if you still think it would be better to have an "event started" log entry as well I can review a PR.
The only requirements for that would be:

  1. Compatibility with Logstash (JSON format + @timestamp and @version fields)
  2. To be enabled through configuration (and turned off by default).

@stanhu
Copy link
Author

stanhu commented Apr 2, 2018

@iMacTia Thanks, makes sense. For your reference, I evaluated this gem and another one to determine what we wanted for GitLab: https://gitlab.com/gitlab-org/gitlab-ce/issues/20060#note_66026811

I used your code as a basis for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18121/diffs, so thanks for starting this project.

@stanhu
Copy link
Author

stanhu commented Apr 2, 2018

We've seen issues where Sidekiq jobs get stalled, and having the "start" message is incredibly helpful to know what was running before things got clogged.

@iMacTia
Copy link
Owner

iMacTia commented Apr 3, 2018

@stanhu I'm honoured you're considering one of my gems to be used in GitLab.

This gem was born as a small helper to be used in some of my production systems by me and my team.
We didn't have any fancy requirements so we ended up with a very simple gem and I never expected someone to use it. It was very useful for us in production though, especially combined with Kibana's powerful tools.

I'm happy for you to use the code as you wish, but please consider to publish your improvements back into the gem so that everyone will benefit, if that's possible of course.

I can also consider a merging/rebase of the gem into your organisation if that works better for you.

@mdibaiee
Copy link
Contributor

mdibaiee commented Apr 6, 2019

Hi @iMacTia !

Thank you for this gem, we also are looking for this feature as well since we have started using this gem. I'm happy to send a pull-request to support this (maybe through a configuration flag). I would like to confirm that you are eager to have this in the repository, and if you are, I will send the pull-request.

@iMacTia
Copy link
Owner

iMacTia commented Apr 6, 2019

Hi @mdibaiee, of course!

Improvements and contributions are always welcome! As far as you follow the guidelines I set out in my comment above I’d be more than happy to review and merge a PR 👍

Thanks for considering to contribute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants