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

Address missing Sidekiq 7 middleware inclusions #1427

Open
gpacuilla-st opened this issue Dec 5, 2023 · 2 comments
Open

Address missing Sidekiq 7 middleware inclusions #1427

gpacuilla-st opened this issue Dec 5, 2023 · 2 comments

Comments

@gpacuilla-st
Copy link

gpacuilla-st commented Dec 5, 2023

Is your feature request related to a problem? Please describe.

It appears that this is not working with sidekiq 7+ currently.

There's some notes around changes in Sidekiq 7 related to this I think:

https://github.com/sidekiq/sidekiq/blob/5b3e895/docs/middleware.md

Middleware Changes in Sidekiq 7.0
tl;dr - middleware should now include Sidekiq::ClientMiddleware or Sidekiq::ServerMiddleware.

Currently, that's not implemented here: https://github.com/elastic/apm-agent-ruby/blob/636319c/lib/elastic_apm/spies/sidekiq.rb

Describe the solution you'd like

Implement the necessary Middleware changes for Sidekiq 7

Additional context

Currently, we aren't seeing any instrumentation being reported for background jobs submitted from our application (sidekiq client side) nor from the bundle exec sidekiq (server side) process which is running separately. Sidekiq is using redis which also never shows up as a dependency in the Kibana Observability APM view.

Relevant gem versions:
rails 7.0.7
sidekiq 7.1.3
redis 5.0.6
elastic-apm 4.7.0

For reference others have also had issues with this, before sidekiq 7: #73

I believe we should at least be seeing the transactions from the server side (bundle exec sidekiq process) going by this:
https://github.com/elastic/apm-agent-ruby/blob/636319c/lib/elastic_apm/spies/sidekiq.rb#L60-L66

@gpacuilla-st gpacuilla-st changed the title Support for Sidekiq 7 Address missing Sidekiq 7 middleware inclusions Dec 6, 2023
@gpacuilla-st
Copy link
Author

This is embarrassing, this actually seemed to be working but was hidden in the UI:

image

I still wonder if the middleware inclusion should be addressed however.

@smedrick
Copy link

smedrick commented Nov 1, 2024

I don't know how you ended up seeing Sidekiq in the APM, but I've only had luck running Sidekiq < 7.0 and getting transaction traces in Elastic

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

No branches or pull requests

2 participants