Commit ede1dcb3 authored by Matthias Käppler's avatar Matthias Käppler 3️⃣
Browse files

Merge branch 'bvl-remove-known-context-keys' into 'master'

Don't enforce logged keys to be known by Labkit

See merge request gitlab-org/labkit-ruby!88
parents 1d201ff6 9c62fa8f
Loading
Loading
Loading
Loading
Loading
+1 −5
Original line number Diff line number Diff line
@@ -9,7 +9,6 @@ require "active_support/core_ext/string/inflections"
module Labkit
  # A context can be used to provide structured information on what resources
  # GitLab is working on within a service.
  # The currently supported keys are defined in the `KNOWN_KEYS` constant.
  #
  # Values can be provided by passing a hash. If one of the values is a Proc
  # the proc will only be called when the value is actually needed.
@@ -28,8 +27,6 @@ module Labkit
    CORRELATION_ID_KEY = "correlation_id"
    RAW_KEYS = [CORRELATION_ID_KEY].freeze
    HEADER_PREFIX = "X-Gitlab-"
    KNOWN_KEYS = %w[user project root_namespace subscription_plan caller_id
                    remote_ip related_class feature_category client_id].freeze

    class << self
      def with_context(attributes = {})
@@ -122,7 +119,6 @@ module Labkit

    def assign_attributes(attributes)
      attributes = attributes.transform_keys(&method(:log_key))
      attributes = attributes.slice(*known_log_keys)

      data.merge!(attributes)

@@ -138,7 +134,7 @@ module Labkit

    private

    delegate :log_key, :known_log_keys, to: :class
    delegate :log_key, to: :class

    attr_reader :data

+1 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ module Labkit
          def call(_worker_class, job, _queue)
            worker_name = (job["wrapped"].presence || job["class"]).to_s
            data = job.merge(Labkit::Context.log_key(:caller_id) => worker_name)
                      .select { |key, _| key.start_with?("#{Labkit::Context::LOG_KEY}.") || Labkit::Context::RAW_KEYS.include?(key.to_s) }

            Labkit::Context.with_context(data) do |_context|
              yield
+11 −8
Original line number Diff line number Diff line
@@ -130,13 +130,15 @@ describe Labkit::Context do
    let(:expected_hash) do
      log_hash(user: "user",
               root_namespace: "namespace",
               project: "project")
               project: "project",
               "random.key": "included")
    end

    it "returns a hash containing the expected values" do
      context = described_class.new(user: "user",
                                    project: "project",
                                    root_namespace: "namespace")
                                    root_namespace: "namespace",
                                    "random.key": "included")

      expect(context.to_h).to include(expected_hash)
    end
@@ -152,6 +154,7 @@ describe Labkit::Context do
        user: -> { "user" },
        root_namespace: -> { "namespace" },
        project: -> { "project" },
        "random.key": -> { "included" },
      )

      expect(context.to_h).to include(expected_hash)
@@ -185,12 +188,14 @@ describe Labkit::Context do
        root_namespace: -> { "namespace" },
        project: -> { "project" },
        caller_id: "ProjectsController#show",
        random_thing: "Random",
      )

      expect(context.to_headers).to include("X-Gitlab-Meta-User" => "user",
                                            "X-Gitlab-Meta-Root-Namespace" => "namespace",
                                            "X-Gitlab-Meta-Project" => "project",
                                            "X-Gitlab-Meta-Caller-Id" => "ProjectsController#show")
                                            "X-Gitlab-Meta-Caller-Id" => "ProjectsController#show",
                                            "X-Gitlab-Meta-Random-Thing" => "Random")
    end

    it "does not include empty values" do
@@ -205,7 +210,7 @@ describe Labkit::Context do
  end

  describe "#initialize" do
    it "assigns only known keys as strings" do
    it "assigns all keys as strings" do
      context = described_class.new(
        # rubocop: disable Style/HashSyntax
        # deliberately testing asigning symbols and strings as keys
@@ -215,8 +220,7 @@ describe Labkit::Context do
        # rubocop: enable Style/HashSyntax
      )

      expect(data_from(context)).to include(log_hash("user" => "u", "project" => "p"))
      expect(data_from(context).keys).not_to include(described_class.log_key("something_else"))
      expect(data_from(context)).to include(log_hash("user" => "u", "project" => "p", "something_else" => "nothing"))
    end

    it "assigns known keys starting with the log key" do
@@ -224,8 +228,7 @@ describe Labkit::Context do
        log_hash(project: "p", root_namespace: "n", user: "u", something_else: "nothing")
      )

      expect(data_from(context)).to include(log_hash("project" => "p", "root_namespace" => "n", "user" => "u"))
      expect(data_from(context).keys).not_to include(described_class.log_key("something_else"))
      expect(data_from(context)).to include(log_hash("project" => "p", "root_namespace" => "n", "user" => "u", "something_else" => "nothing"))
    end

    it "always assigns a correlation id" do
+15 −3
Original line number Diff line number Diff line
@@ -25,11 +25,23 @@ describe Labkit::Middleware::Sidekiq::Context::Server do
    end

    it "sets the application context from job params and clears after running" do
      expected_metadata = { "meta.project" => "jane.doe/bookstore", "meta.user" => "jane.doe" }
      job = expected_metadata.merge("class" => "TestWorker", "queue" => "default", "args" => ["do it"])
      expected_metadata = {
        "correlation_id" => "123",
        "meta.project" => "jane.doe/bookstore",
        "meta.user" => "jane.doe",
        "meta.random" => "key",
        :"meta.symbol" => "included",
      }
      job = expected_metadata.merge(
        "class" => "TestWorker",
        "queue" => "default",
        "args" => ["do it"],
        "something_else" => "not prefixed with meta",
        symbol: "not included", # rubocop: disable Style/HashSyntax explicitly testing mixed hash
      )

      expect(Labkit::Context).to receive(:with_context)
                                   .with(hash_including(job)).ordered.and_call_original
                                   .with(hash_including(expected_metadata.stringify_keys)).ordered.and_call_original
      expect(fake_job).to receive(:perform).with("do it").ordered

      Sidekiq::Client.push(job)