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

feat: Sidekiq metrics / metrics integration draft #1314

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

zvkemp
Copy link

@zvkemp zvkemp commented Dec 20, 2024

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.

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.

1 participant