Commit 85a6dbdb authored by Sean McGivern's avatar Sean McGivern

Remove controller, actions, and job class fields from Marginalia

These are all covered by the endpoint ID field.
parent c2688ebe
...@@ -312,7 +312,7 @@ gem 'pg_query', '~> 1.3.0' ...@@ -312,7 +312,7 @@ gem 'pg_query', '~> 1.3.0'
gem 'premailer-rails', '~> 1.10.3' gem 'premailer-rails', '~> 1.10.3'
# LabKit: Tracing and Correlation # LabKit: Tracing and Correlation
gem 'gitlab-labkit', '~> 0.16.1' gem 'gitlab-labkit', '~> 0.16.2'
# Thrift is a dependency of gitlab-labkit, we want a version higher than 0.14.0 # Thrift is a dependency of gitlab-labkit, we want a version higher than 0.14.0
# because of https://gitlab.com/gitlab-org/gitlab/-/issues/321900 # because of https://gitlab.com/gitlab-org/gitlab/-/issues/321900
gem 'thrift', '>= 0.14.0' gem 'thrift', '>= 0.14.0'
......
...@@ -455,7 +455,7 @@ GEM ...@@ -455,7 +455,7 @@ GEM
fog-xml (~> 0.1.0) fog-xml (~> 0.1.0)
google-api-client (>= 0.44.2, < 0.51) google-api-client (>= 0.44.2, < 0.51)
google-cloud-env (~> 1.2) google-cloud-env (~> 1.2)
gitlab-labkit (0.16.1) gitlab-labkit (0.16.2)
actionpack (>= 5.0.0, < 7.0.0) actionpack (>= 5.0.0, < 7.0.0)
activesupport (>= 5.0.0, < 7.0.0) activesupport (>= 5.0.0, < 7.0.0)
grpc (~> 1.19) grpc (~> 1.19)
...@@ -1425,7 +1425,7 @@ DEPENDENCIES ...@@ -1425,7 +1425,7 @@ DEPENDENCIES
gitlab-experiment (~> 0.5.0) gitlab-experiment (~> 0.5.0)
gitlab-fog-azure-rm (~> 1.0.1) gitlab-fog-azure-rm (~> 1.0.1)
gitlab-fog-google (~> 1.13) gitlab-fog-google (~> 1.13)
gitlab-labkit (~> 0.16.1) gitlab-labkit (~> 0.16.2)
gitlab-license (~> 1.3) gitlab-license (~> 1.3)
gitlab-mail_room (~> 0.0.9) gitlab-mail_room (~> 0.0.9)
gitlab-markup (~> 1.7.1) gitlab-markup (~> 1.7.1)
......
...@@ -13,7 +13,7 @@ require 'marginalia' ...@@ -13,7 +13,7 @@ require 'marginalia'
# matching against the raw SQL, and prepending the comment prevents color # matching against the raw SQL, and prepending the comment prevents color
# coding from working in the development log. # coding from working in the development log.
Marginalia::Comment.prepend_comment = true if Rails.env.production? Marginalia::Comment.prepend_comment = true if Rails.env.production?
Marginalia::Comment.components = [:application, :controller, :action, :correlation_id, :jid, :job_class, :endpoint_id] Marginalia::Comment.components = [:application, :correlation_id, :jid, :endpoint_id]
# As mentioned in https://github.com/basecamp/marginalia/pull/93/files, # As mentioned in https://github.com/basecamp/marginalia/pull/93/files,
# adding :line has some overhead because a regexp on the backtrace has # adding :line has some overhead because a regexp on the backtrace has
......
...@@ -20,9 +20,8 @@ and its application source from the comments. ...@@ -20,9 +20,8 @@ and its application source from the comments.
Queries generated from **Rails** include the following metadata in comments: Queries generated from **Rails** include the following metadata in comments:
- `application` - `application`
- `controller`
- `action`
- `correlation_id` - `correlation_id`
- `endpoint_id`
- `line` - `line`
Queries generated from **Sidekiq** workers will include the following metadata Queries generated from **Sidekiq** workers will include the following metadata
...@@ -30,20 +29,34 @@ in comments: ...@@ -30,20 +29,34 @@ in comments:
- `application` - `application`
- `jid` - `jid`
- `job_class`
- `correlation_id` - `correlation_id`
- `endpoint_id`
- `line` - `line`
Examples of queries with comments as observed in `development.log`: `endpoint_id` is a single field that can represent any endpoint in the application:
1. Rails: - For Rails controllers, it's the controller and action. For example, `Projects::BlobController#show`.
- For Grape API endpoints, it's the route. For example, `/api/:version/users/:id`.
- For Sidekiq workers, it's the worker class name. For example, `UserStatusCleanup::BatchWorker`.
`line` is not present in production logs due to the additional overhead required.
Examples of queries with comments:
- Rails:
```sql
/*application:web,controller:blob,action:show,correlation_id:01EZVMR923313VV44ZJDJ7PMEZ,endpoint_id:Projects::BlobController#show*/ SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 75 AND "routes"."source_type" = 'Namespace' LIMIT 1
```
- Grape:
```sql ```sql
/*application:web,controller:jobs,action:trace,correlation_id:rYF4mey9CH3,line:/app/policies/project_policy.rb:504:in `feature_available?'*/ SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = $1 LIMIT $2 [["project_id", 5], ["LIMIT", 1]] /*application:web,correlation_id:01EZVN0DAYGJF5XHG9N4VX8FAH,endpoint_id:/api/:version/users/:id*/ SELECT COUNT(*) FROM "users" INNER JOIN "user_follow_users" ON "users"."id" = "user_follow_users"."followee_id" WHERE "user_follow_users"."follower_id" = 1
``` ```
1. Sidekiq: - Sidekiq:
```sql ```sql
/*application:sidekiq,jid:e7d6668a39a991e323009833,job_class:ExpireJobCacheWorker,correlation_id:rYF4mey9CH3,line:/app/workers/expire_job_cache_worker.rb:14:in `perform'*/ SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 LIMIT $2 [["id", 64], ["LIMIT", 1]] /*application:sidekiq,correlation_id:df643992563683313bc0a0288fb55e23,jid:15fbc506590c625d7664b074,endpoint_id:UserStatusCleanup::BatchWorker,line:/app/workers/user_status_cleanup/batch_worker.rb:19:in `perform'*/ SELECT $1 AS one FROM "user_statuses" WHERE "user_statuses"."clear_status_at" <= $2 LIMIT $3
``` ```
...@@ -3,18 +3,28 @@ ...@@ -3,18 +3,28 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Marginalia spec' do RSpec.describe 'Marginalia spec' do
class MarginaliaTestController < ActionController::Base class MarginaliaTestController < ApplicationController
skip_before_action :authenticate_user!, :check_two_factor_requirement
def first_user def first_user
User.first User.first
render body: nil render body: nil
end end
private
[:auth_user, :current_user, :set_experimentation_subject_id_cookie, :signed_in?].each do |method|
define_method(method) { }
end
end end
class MarginaliaTestJob class MarginaliaTestJob
include Sidekiq::Worker include Sidekiq::Worker
def perform def perform
User.first Gitlab::ApplicationContext.with_context(caller_id: self.class.name) do
User.first
end
end end
end end
...@@ -30,10 +40,9 @@ RSpec.describe 'Marginalia spec' do ...@@ -30,10 +40,9 @@ RSpec.describe 'Marginalia spec' do
let(:component_map) do let(:component_map) do
{ {
"application" => "test", "application" => "test",
"controller" => "marginalia_test", "endpoint_id" => "MarginaliaTestController#first_user",
"action" => "first_user", "correlation_id" => correlation_id
"correlation_id" => correlation_id
} }
end end
...@@ -47,6 +56,7 @@ RSpec.describe 'Marginalia spec' do ...@@ -47,6 +56,7 @@ RSpec.describe 'Marginalia spec' do
describe 'for Sidekiq worker jobs' do describe 'for Sidekiq worker jobs' do
around do |example| around do |example|
with_sidekiq_server_middleware do |chain| with_sidekiq_server_middleware do |chain|
chain.add Labkit::Middleware::Sidekiq::Context::Server
chain.add Marginalia::SidekiqInstrumentation::Middleware chain.add Marginalia::SidekiqInstrumentation::Middleware
Marginalia.application_name = "sidekiq" Marginalia.application_name = "sidekiq"
example.run example.run
...@@ -66,10 +76,10 @@ RSpec.describe 'Marginalia spec' do ...@@ -66,10 +76,10 @@ RSpec.describe 'Marginalia spec' do
let(:component_map) do let(:component_map) do
{ {
"application" => "sidekiq", "application" => "sidekiq",
"job_class" => "MarginaliaTestJob", "endpoint_id" => "MarginaliaTestJob",
"correlation_id" => sidekiq_job['correlation_id'], "correlation_id" => sidekiq_job['correlation_id'],
"jid" => sidekiq_job['jid'] "jid" => sidekiq_job['jid']
} }
end end
...@@ -80,19 +90,34 @@ RSpec.describe 'Marginalia spec' do ...@@ -80,19 +90,34 @@ RSpec.describe 'Marginalia spec' do
end end
describe 'for ActionMailer delivery jobs' do describe 'for ActionMailer delivery jobs' do
# We need to ensure that this runs through Sidekiq to take
# advantage of the middleware. There is a Rails bug that means we
# have to do some extra steps to make this happen:
# https://github.com/rails/rails/issues/37270#issuecomment-553927324
around do |example|
original_queue_adapter = ActiveJob::Base.queue_adapter
descendants = ActiveJob::Base.descendants + [ActiveJob::Base]
ActiveJob::Base.queue_adapter = :sidekiq
descendants.each(&:disable_test_adapter)
example.run
descendants.each { |a| a.enable_test_adapter(ActiveJob::QueueAdapters::TestAdapter.new) }
ActiveJob::Base.queue_adapter = original_queue_adapter
end
let(:delivery_job) { MarginaliaTestMailer.first_user.deliver_later } let(:delivery_job) { MarginaliaTestMailer.first_user.deliver_later }
let(:recorded) do let(:recorded) do
ActiveRecord::QueryRecorder.new do ActiveRecord::QueryRecorder.new do
delivery_job.perform_now Sidekiq::Worker.drain_all
end end
end end
let(:component_map) do let(:component_map) do
{ {
"application" => "sidekiq", "application" => "sidekiq",
"jid" => delivery_job.job_id, "endpoint_id" => "ActionMailer::MailDeliveryJob",
"job_class" => delivery_job.arguments.first "jid" => delivery_job.job_id
} }
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