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

Queue latency gauge (by queue name) #4

Merged
merged 2 commits into from
Apr 29, 2019
Merged

Conversation

dsalahutdinov
Copy link
Member

Uses sidekiq internal queue latency

@@ -46,6 +48,10 @@ module Sidekiq
sidekiq_active_processes.set({}, stats.processes_size)
sidekiq_jobs_retry_count.set({}, stats.retry_size)

::Sidekiq::Queue.all.each do |queue|
sidekiq_jobs_latency.set({ queue: queue.name }, queue.latency)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have to round the seconds value for better visibility 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we actually need any rounding

Copy link
Member

@Envek Envek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned that we scan for queues twice (for sizes and latencies), but I don't see any method in Sidekiq API that will allow us avoid it (when you initializing Sidekiq::Stats queue sizes will be requested immediately anyway). And I don't want reinvent half of sidekiq's api.rb for now.

Anyway it should be fast enough to be okay.

@@ -31,6 +31,8 @@ module Sidekiq
gauge :jobs_retry_count, comment: "The number of failed jobs waiting to be retried"
gauge :jobs_dead_count, comment: "The number of jobs exceeded their retry count."
gauge :active_processes, comment: "The number of active Sidekiq worker processes."
gauge :jobs_latency, comment: "The job latency: the delay of the job start time in seconds"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that original explanation of what is latency is a bit easier to understand:
https://github.com/mperham/sidekiq/blob/0352ca8fb541dec2caca886aa374e4cbb3c638d9/lib/sidekiq/api.rb#L254-L255

Also you can specify seconds in Yabeda API:

Suggested change
gauge :jobs_latency, comment: "The job latency: the delay of the job start time in seconds"
gauge :jobs_latency, unit: :seconds, comment: "Time in seconds since the oldest job in the queue was enqueued."

@@ -46,6 +48,10 @@ module Sidekiq
sidekiq_active_processes.set({}, stats.processes_size)
sidekiq_jobs_retry_count.set({}, stats.retry_size)

::Sidekiq::Queue.all.each do |queue|
sidekiq_jobs_latency.set({ queue: queue.name }, queue.latency)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we actually need any rounding

@dsalahutdinov
Copy link
Member Author

Metric comment and description are fixed

@Envek Envek merged commit 215b122 into yabeda-rb:master Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants