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.

Sign up for GitHub

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

Jump to bottom

Job latency #9

Merged
merged 6 commits into from
Feb 20, 2020
Merged

Job latency #9

merged 6 commits into from
Feb 20, 2020

Conversation

asusikov
Copy link
Contributor

What do this PR do?

  • added new metric for latency of job - time difference between enqueuing and running of job
  • renamed metric for latency of queue

Purposes

Why metric renamed?

We set metric's value from property of Sidekiq::Queue instance, so I think the new name is accurately determines its value

Envek
Envek approved these changes Feb 20, 2020
View reviewed changes
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.

Oh, metric renaming is a breaking change, but I can't not to agree with it.

@Envek Envek merged commit 9f37995 into yabeda-rb:master Feb 20, 2020
@Envek
Copy link
Member

Envek commented Feb 20, 2020

Released as 0.5.0. Thank you very much!

@asusikov
Copy link
Contributor Author

Oh, metric renaming is a breaking change, but I can't not to agree with it.

Yes, I agree this changes is not good but it's better then having two metrics with similar names - jobs_latency and job_latency, but means different

@asusikov
Copy link
Contributor Author

Thank you for quick release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Envek Envek Envek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asusikov @Envek