Commit e4acd2e3 authored by Dylan Griffith (ex GitLab)'s avatar Dylan Griffith (ex GitLab) 💬
Browse files

Maintain correlation_id across Sidekiq retries 

Previously this code was overriding the correlation_id in the `job`
hash. That's because retries were generating a new context and this new
context had a new random `correlation_id`.

Now we take the previous `correlation_id` into account when generating a
new context.

This resolves gitlab-org/gitlab#440700
parent 6c957a24

lib/labkit/middleware/sidekiq/context/client.rb

+9 −1
Original line number Diff line number Diff line
@@ -9,7 +9,15 @@ module Labkit 
        # be reinstantiated by Sidekiq-server when running the job.

        class Client

          def call(_worker_class, job, _queue, _redis_pool)

            Labkit::Context.with_context do |context|

            attributes = {}



            # Don't overwrite the correlation_id from the job. A new context

            # will always generate a new correlation_id and we'd rather carry

            # through the correlation_id from the previous job if it is

            # present (eg. for retries).

            attributes[Labkit::Context::CORRELATION_ID_KEY] = job["correlation_id"] if job["correlation_id"]



            Labkit::Context.with_context(attributes) do |context|

              job.merge!(context.to_h)



              yield

spec/labkit/middleware/sidekiq/context/client_spec.rb

+17 −0
Original line number Diff line number Diff line 
# frozen_string_literal: true



require "sidekiq/testing"

require "active_support"

require "active_support/core_ext"

require_relative "../../../../support/sidekiq_middleware/shared_contexts"



describe Labkit::Middleware::Sidekiq::Context::Client do
@@ -27,6 +29,21 @@ describe Labkit::Middleware::Sidekiq::Context::Client do 
    expect(job.keys).to include(Labkit::Context::CORRELATION_ID_KEY)

  end



  it "does not override the correlation_id in the event of a retry" do

    job = { "jid" => "thejobid", "correlation_id" => "thecorrelationid" }



    yielded = false



    # Here we cannot easily simulate a sidekiq job retrying so we instead just

    # call the middleware directly to unit test how the `job` is manipulated

    described_class.new.call(test_worker.class, job, nil, nil) do

      expect(job["correlation_id"]).to eq("thecorrelationid")

      yielded = true

    end



    expect(yielded).to eq(true)

  end



  def find_job(jid)

    test_worker.jobs.find { |j| j["jid"] == jid }

  end