Skip to content

feat: Sidekiq metrics / metrics integration draft #1314

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
Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat: Sidekiq metrics / metrics integration draft #1314

wants to merge 3 commits into from

Conversation

zvkemp
Copy link
Contributor

@zvkemp zvkemp commented Dec 20, 2024 •
edited
Loading

Still todo:

  • test that metrics aren't included if the option is disabled
  • move a bunch of logic into Instrumentation::Base or an appropriate mixin
  • standardize the logic gating on presence/absence of the metrics-api gem
  • should metrics-api be unconditionally required? I.e does the presence of metrics-api in the bundle mean it should be considered active?
  • test the client middleware
  • any metrics on the other patch modules?
  • Once the logic has been moved to its appropriate home, this PR should be split into at least two parts; 1) for generic metrics support on Instrumentation::Base and 2) concrete metrics on sidekiq

blockers

  • opentelemetry-metrics-api and opentelemetry-metrics-sdk are not in sync (metrics-sdk is newer, and includes methods that aren't implemented by the rubygems version of metrics-api). In tests, I am bundling from github for both of them.
  • Semantic conventions for metrics are not present in the ruby lib
  • messaging.queue.latency should be proposed to semconv
  • resolve question of how to time metrics that correspond to a span — it seems like in most cases a collector should be able to synthesize histograms or counters for anything with a span; unclear why semconv demands some required metrics that also have spans.

@zvkemp zvkemp force-pushed the sidekiq-metrics branch 2 times, most recently from 28a6af8 to 04c4243 Compare January 7, 2025 20:07
@zvkemp zvkemp force-pushed the sidekiq-metrics branch 4 times, most recently from 8e4e07f to 8c18433 Compare January 24, 2025 21:58
@zvkemp zvkemp mentioned this pull request Jan 31, 2025
Open
4 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 7, 2025

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@github-actions github-actions bot added the stale Marks an issue/PR stale label Mar 7, 2025
@kaylareopelle kaylareopelle removed the stale Marks an issue/PR stale label Mar 11, 2025
@github-actions GitHub Actions
Copy link
Contributor

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@github-actions github-actions bot added the stale Marks an issue/PR stale label Apr 11, 2025
@kaylareopelle kaylareopelle removed the stale Marks an issue/PR stale label Apr 21, 2025
@github-actions GitHub Actions
Copy link
Contributor

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@github-actions github-actions bot added the stale Marks an issue/PR stale label May 22, 2025
@kaylareopelle kaylareopelle added keep Ensures stale-bot keeps this issue/PR open and removed stale Marks an issue/PR stale labels May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbogsany fbogsany Awaiting requested review from fbogsany fbogsany will be requested when the pull request is marked ready for review fbogsany is a code owner

@mwear mwear Awaiting requested review from mwear mwear will be requested when the pull request is marked ready for review mwear is a code owner

@robertlaurin robertlaurin Awaiting requested review from robertlaurin robertlaurin will be requested when the pull request is marked ready for review robertlaurin is a code owner

@dazuma dazuma Awaiting requested review from dazuma dazuma will be requested when the pull request is marked ready for review dazuma is a code owner

@ericmustin ericmustin Awaiting requested review from ericmustin ericmustin will be requested when the pull request is marked ready for review ericmustin is a code owner

@arielvalentin arielvalentin Awaiting requested review from arielvalentin arielvalentin will be requested when the pull request is marked ready for review arielvalentin is a code owner

@ahayworth ahayworth Awaiting requested review from ahayworth ahayworth will be requested when the pull request is marked ready for review ahayworth is a code owner

@plantfansam plantfansam Awaiting requested review from plantfansam plantfansam will be requested when the pull request is marked ready for review plantfansam is a code owner

@robbkidd robbkidd Awaiting requested review from robbkidd robbkidd will be requested when the pull request is marked ready for review robbkidd is a code owner

@simi simi Awaiting requested review from simi simi will be requested when the pull request is marked ready for review simi is a code owner

@kaylareopelle kaylareopelle Awaiting requested review from kaylareopelle kaylareopelle will be requested when the pull request is marked ready for review kaylareopelle is a code owner

@xuan-cao-swi xuan-cao-swi Awaiting requested review from xuan-cao-swi xuan-cao-swi will be requested when the pull request is marked ready for review xuan-cao-swi is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
keep Ensures stale-bot keeps this issue/PR open
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zvkemp @kaylareopelle