Commit 6ed955e1 authored by Markus Koller's avatar Markus Koller

Set current project/group in WebHookWorker context

Currently this isn't always reliably detected, depending on how the
worker is triggered.

By setting this explicitly we always see the related project and
namespace in Kibana, and also the subscription plan in EE.

This also sets the `related_class` field to the webhook type, which
is useful for troubleshooting purposes.
parent 31a69eb7
...@@ -39,6 +39,11 @@ class ProjectHook < WebHook ...@@ -39,6 +39,11 @@ class ProjectHook < WebHook
def rate_limit def rate_limit
project.actual_limits.limit_for(:web_hook_calls) project.actual_limits.limit_for(:web_hook_calls)
end end
override :application_context
def application_context
super.merge(project: project)
end
end end
ProjectHook.prepend_mod_with('ProjectHook') ProjectHook.prepend_mod_with('ProjectHook')
...@@ -80,6 +80,11 @@ class WebHook < ApplicationRecord ...@@ -80,6 +80,11 @@ class WebHook < ApplicationRecord
nil nil
end end
# Custom attributes to be included in the worker context.
def application_context
{ related_class: type }
end
private private
def web_hooks_disable_failed? def web_hooks_disable_failed?
......
...@@ -94,9 +94,11 @@ class WebHookService ...@@ -94,9 +94,11 @@ class WebHookService
if rate_limited?(hook) if rate_limited?(hook)
log_rate_limit(hook) log_rate_limit(hook)
else else
Gitlab::ApplicationContext.with_context(hook.application_context) do
WebHookWorker.perform_async(hook.id, data, hook_name) WebHookWorker.perform_async(hook.id, data, hook_name)
end end
end end
end
private private
......
...@@ -42,4 +42,9 @@ class GroupHook < WebHook ...@@ -42,4 +42,9 @@ class GroupHook < WebHook
def rate_limit def rate_limit
group.actual_limits.limit_for(:web_hook_calls) group.actual_limits.limit_for(:web_hook_calls)
end end
override :application_context
def application_context
super.merge(namespace: group)
end
end end
...@@ -29,4 +29,15 @@ RSpec.describe GroupHook do ...@@ -29,4 +29,15 @@ RSpec.describe GroupHook do
expect(hook_ultimate.rate_limit).to be(500) expect(hook_ultimate.rate_limit).to be(500)
end end
end end
describe '#application_context' do
let_it_be(:hook) { build(:group_hook) }
it 'includes the type and group' do
expect(hook.application_context).to eq(
related_class: 'GroupHook',
namespace: hook.group
)
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe WebHookService do
let_it_be(:project) { create(:project) }
let_it_be_with_reload(:project_hook) { create(:project_hook, project: project) }
let(:service_instance) { described_class.new(project_hook, {}, :push_hooks) }
describe '#async_execute' do
context 'when hook has custom context attributes' do
it 'includes the subscription plan in the worker context' do
expect(WebHookWorker).to receive(:perform_async) do
expect(Gitlab::ApplicationContext.current).to include(
'meta.subscription_plan' => 'default'
)
end
service_instance.async_execute
end
context 'when the rate-limiting feature flag is disabled' do
before do
stub_feature_flags(web_hooks_rate_limit: false)
end
it 'does not include the subscription plan in the worker context' do
expect(WebHookWorker).to receive(:perform_async) do
expect(Gitlab::ApplicationContext.current).not_to include('meta.subscription_plan')
end
service_instance.async_execute
end
end
end
end
end
...@@ -39,4 +39,15 @@ RSpec.describe ProjectHook do ...@@ -39,4 +39,15 @@ RSpec.describe ProjectHook do
expect(hook.rate_limit).to be(100) expect(hook.rate_limit).to be(100)
end end
end end
describe '#application_context' do
let_it_be(:hook) { build(:project_hook) }
it 'includes the type and project' do
expect(hook.application_context).to include(
related_class: 'ProjectHook',
project: hook.project
)
end
end
end end
...@@ -30,4 +30,14 @@ RSpec.describe ServiceHook do ...@@ -30,4 +30,14 @@ RSpec.describe ServiceHook do
expect(hook.rate_limit).to be_nil expect(hook.rate_limit).to be_nil
end end
end end
describe '#application_context' do
let(:hook) { build(:service_hook) }
it 'includes the type' do
expect(hook.application_context).to eq(
related_class: 'ServiceHook'
)
end
end
end end
...@@ -177,4 +177,14 @@ RSpec.describe SystemHook do ...@@ -177,4 +177,14 @@ RSpec.describe SystemHook do
expect(hook.rate_limit).to be_nil expect(hook.rate_limit).to be_nil
end end
end end
describe '#application_context' do
let(:hook) { build(:system_hook) }
it 'includes the type' do
expect(hook.application_context).to eq(
related_class: 'SystemHook'
)
end
end
end end
...@@ -430,5 +430,19 @@ RSpec.describe WebHookService do ...@@ -430,5 +430,19 @@ RSpec.describe WebHookService do
end end
end end
end end
context 'when hook has custom context attributes' do
it 'includes the attributes in the worker context' do
expect(WebHookWorker).to receive(:perform_async) do
expect(Gitlab::ApplicationContext.current).to include(
'meta.project' => project_hook.project.full_path,
'meta.root_namespace' => project.root_ancestor.path,
'meta.related_class' => 'ProjectHook'
)
end
service_instance.async_execute
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment