Commit a30f267e authored by Grzegorz Bizon's avatar Grzegorz Bizon

Improve live traces observability using counters

This commit extends code around live traces with additional
observability mechanisms through Prometheus counters.
parent 8787a3c4
......@@ -11,6 +11,8 @@ module Ci
default_value_for :data_store, :redis
after_create { metrics.increment_trace_operation(operation: :chunked) }
CHUNK_SIZE = 128.kilobytes
WRITE_LOCK_RETRY = 10
WRITE_LOCK_SLEEP = 0.01.seconds
......@@ -182,6 +184,8 @@ module Ci
end
current_store.append_data(self, value, offset).then do |stored|
metrics.increment_trace_operation(operation: :appended)
raise ArgumentError, 'Trace appended incorrectly' if stored != new_size
end
......@@ -205,5 +209,9 @@ module Ci
retries: WRITE_LOCK_RETRY,
sleep_sec: WRITE_LOCK_SLEEP }]
end
def metrics
@metrics ||= ::Gitlab::Ci::Trace::Metrics.new
end
end
end
......@@ -6,17 +6,21 @@ module Gitlab
class Metrics
extend Gitlab::Utils::StrongMemoize
OPERATIONS = [:appended, :mutated, :overwrite, :accepted,
:finalized, :discarded, :flaky].freeze
OPERATIONS = [:appended, :streamed, :chunked, :mutated, :overwrite,
:accepted, :finalized, :discarded, :conflict].freeze
def increment_trace_operation(operation: :unknown)
unless OPERATIONS.include?(operation)
raise ArgumentError, 'unknown trace operation'
raise ArgumentError, "unknown trace operation: #{operation}"
end
self.class.trace_operations.increment(operation: operation)
end
def increment_trace_bytes(size)
self.class.trace_bytes.increment(by: size.to_i)
end
def self.trace_operations
strong_memoize(:trace_operations) do
name = :gitlab_ci_trace_operations_total
......@@ -25,6 +29,15 @@ module Gitlab
Gitlab::Metrics.counter(name, comment)
end
end
def self.trace_bytes
strong_memoize(:trace_bytes) do
name = :gitlab_ci_trace_bytes_total
comment = 'Total amount of build trace bytes transferred'
Gitlab::Metrics.counter(name, comment)
end
end
end
end
end
......
......@@ -8,7 +8,7 @@ module Gitlab
BUFFER_SIZE = 4096
LIMIT_SIZE = 500.kilobytes
attr_reader :stream
attr_reader :stream, :metrics
delegate :close, :tell, :seek, :size, :url, :truncate, to: :stream, allow_nil: true
......@@ -16,9 +16,10 @@ module Gitlab
alias_method :present?, :valid?
def initialize
def initialize(metrics = Trace::Metrics.new)
@stream = yield
@stream&.binmode
@metrics = metrics
end
def valid?
......@@ -43,6 +44,9 @@ module Gitlab
def append(data, offset)
data = data.force_encoding(Encoding::BINARY)
metrics.increment_trace_operation(operation: :streamed)
metrics.increment_trace_bytes(data.bytesize)
stream.seek(offset, IO::SEEK_SET)
stream.write(data)
stream.truncate(offset + data.bytesize)
......
......@@ -151,6 +151,28 @@ RSpec.describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
it_behaves_like 'appends'
end
describe 'metrics' do
let(:metrics) { spy('metrics') }
let(:io) { StringIO.new }
let(:stream) { described_class.new(metrics) { io } }
it 'increments trace streamed operation' do
stream.append(+'123456', 0)
expect(metrics)
.to have_received(:increment_trace_operation)
.with(operation: :streamed)
end
it 'increments trace bytes counter' do
stream.append(+'123456', 0)
expect(metrics)
.to have_received(:increment_trace_bytes)
.with(6)
end
end
end
describe '#set' do
......
......@@ -21,6 +21,25 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
stub_artifacts_object_storage
end
describe 'chunk creation' do
let(:metrics) { spy('metrics') }
before do
allow(::Gitlab::Ci::Trace::Metrics)
.to receive(:new)
.and_return(metrics)
end
it 'increments trace operation chunked metric' do
build_trace_chunk.save!
expect(metrics)
.to have_received(:increment_trace_operation)
.with(operation: :chunked)
.once
end
end
context 'FastDestroyAll' do
let(:parent) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: parent) }
......@@ -346,6 +365,24 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
it_behaves_like 'Scheduling no sidekiq worker'
end
end
describe 'append metrics' do
let(:metrics) { spy('metrics') }
before do
allow(::Gitlab::Ci::Trace::Metrics)
.to receive(:new)
.and_return(metrics)
end
it 'increments trace operation appended metric' do
build_trace_chunk.append('123456', 0)
expect(metrics)
.to have_received(:increment_trace_operation)
.with(operation: :appended)
end
end
end
describe '#truncate' do
......
......@@ -8,7 +8,7 @@ RSpec.describe Ci::UpdateBuildStateService do
let(:build) { create(:ci_build, :running, pipeline: pipeline) }
let(:metrics) { spy('metrics') }
subject { described_class.new(build, params, metrics) }
subject { described_class.new(build, params) }
before do
stub_feature_flags(ci_enable_live_trace: true)
......@@ -31,7 +31,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'does not increment finalized trace metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.not_to have_received(:increment_trace_operation)
......@@ -50,11 +50,16 @@ RSpec.describe Ci::UpdateBuildStateService do
context 'when request payload carries a trace' do
let(:params) { { state: 'success', trace: 'overwritten' } }
it 'overwrites a trace and updates trace operation metric' do
it 'overwrites a trace' do
result = subject.execute
expect(build.trace.raw).to eq 'overwritten'
expect(result.status).to eq 200
end
it 'updates overwrite operation metric' do
execute_with_stubbed_metrics!
expect(metrics)
.to have_received(:increment_trace_operation)
.with(operation: :overwrite)
......@@ -96,7 +101,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'increments trace finalized operation metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.to have_received(:increment_trace_operation)
......@@ -130,7 +135,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'increments trace accepted operation metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.to have_received(:increment_trace_operation)
......@@ -172,7 +177,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'increments discarded traces metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.to have_received(:increment_trace_operation)
......@@ -180,7 +185,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'does not increment finalized trace metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.not_to have_received(:increment_trace_operation)
......@@ -203,7 +208,7 @@ RSpec.describe Ci::UpdateBuildStateService do
end
it 'increments conflict trace metric' do
subject.execute
execute_with_stubbed_metrics!
expect(metrics)
.to have_received(:increment_trace_operation)
......@@ -224,4 +229,10 @@ RSpec.describe Ci::UpdateBuildStateService do
end
end
end
def execute_with_stubbed_metrics!
described_class
.new(build, params, metrics)
.execute
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