Sidekiq middleware

[Envek]

Resolves #36

For discussion: flamegraph { } method by default opens captured flamegraph in browser and prints some messages to stdout, should we revise it? (maybe in separate pull request)

[dduugg]

PR Review: "Sidekiq middleware"

Goal

Resolves #36. Adds a Singed::Sidekiq::ServerMiddleware so flamegraphs can be captured for Sidekiq jobs, with support for ActiveJob-wrapped jobs and three opt-in mechanisms: the x-singed payload key, a class-level capture_flamegraph? method, and the SINGED_MIDDLEWARE_ALWAYS_CAPTURE env var.

---

Implementation

• lib/singed/sidekiq.rb (new): The middleware itself. call checks capture_flamegraph? and either delegates to the block or wraps it in flamegraph(label). Supports ActiveJob wrapping via job_payload["wrapped"], and resolves job classes via constantize or Object.const_get.
• spec/singed/sidekiq_spec.rb (new): Good coverage of plain Sidekiq jobs, class-level capture_flamegraph?, SINGED_MIDDLEWARE_ALWAYS_CAPTURE, and ActiveJob jobs.
• spec/support/sidekiq.rb (new): Test harness with a SidekiqTestingInlineWithMiddlewares module that patches Sidekiq::Client#push to invoke server middlewares (Sidekiq's inline test mode normally skips them). Defines fixture job classes.
• README.md: Documents the new middleware and its three opt-in mechanisms.
• Gemfile / Gemfile.lock: Adds sidekiq and activejob as dev dependencies.

---

Risk Assessment

Low–Medium risk. This is additive (no changes to existing code paths), well-tested, and opt-in. A few things worth noting:

1. README example has a syntax error

The README shows:

MyJob.set(x-singed: true).perform_async

x-singed is not a valid Ruby symbol literal — it will be parsed as x - singed (subtraction). It should be:

MyJob.set("x-singed" => true).perform_async

2. job_class resolution could raise on unknown constants

Object.const_get(job_class.to_s) will raise NameError if the class can't be found (e.g. a stale job referencing a renamed/deleted class). This is an edge case but worth wrapping in a rescue if you want the middleware to be defensive:

Object.const_get(job_class.to_s)
rescue NameError
  nil

(Then capture_flamegraph? and flamegraph_label would need to handle nil gracefully.)

3. capture_flamegraph? reads job_payload["x-singed"] as a raw value

The value is returned as-is without coercion, so job_payload["x-singed"] = false would correctly skip capture, but job_payload["x-singed"] = "false" (a string) would be truthy and incorrectly trigger capture. Since the Rack middleware uses TRUTHY_STRINGS for env var coercion, consider applying the same treatment here, or at least documenting that the value must be a boolean.

4. SidekiqTestingInlineWithMiddlewares patches Sidekiq::Client#push globally

The config.before(:suite) block in spec/support/sidekiq.rb prepends this module for the entire test suite. If other specs rely on standard inline behavior, this could affect them. A narrower scope (e.g. before in the specific spec file) would be safer.

5. Singed.output_directory is set globally in before(:suite)

Singed.output_directory = Dir.mktmpdir("singed-sidekiq-spec")

This is a global side effect that affects all other specs run in the same suite. Consider resetting it after the suite or scoping it to the sidekiq specs only.

6. Minor: self.job_class shadowing

In capture_flamegraph?, the method calls self.job_class(...) but job_class is also a private instance method defined below. This works, but using self. on a private method is unusual in Ruby and could be confusing. Drop self. since it's called within the same object.

---

Summary

Solid, additive feature with good test coverage. Main items before merging:

• Fix the README example: x-singed: → "x-singed" =>
• Consider coercing job_payload["x-singed"] to boolean (consistent with how other flags are handled)
• Consider guarding Object.const_get against NameError
• Scope the SidekiqTestingInlineWithMiddlewares prepend and Singed.output_directory assignment more narrowly to avoid global test suite side effects

[Envek]

1. README example has a syntax error

Fixed

2. job_class resolution could raise on unknown constants

Fixed

3. capture_flamegraph? reads job_payload["x-singed"] as a raw value

Added check against truthy values too

4. SidekiqTestingInlineWithMiddlewares patches Sidekiq::Client#push globally

There is nothing else Sidekiq-related planned, so I believe it is fine

5. Singed.output_directory is set globally in before(:suite)

Fixed

6. Minor: self.job_class shadowing

Fixed

[dduugg]

Thanks @Envek for the thoughtful replies and follow-up fixes! The TRUTHY_STRINGS coercion for x-singed, the NameError rescue, the README fix, and the per-test output_directory scoping are all clean. The pragmatic call on keeping the global prepend is fine given there are no other Sidekiq specs to conflict with. Merging.