Make Gitlab::Metrics::Transaction more generic and clean
While I was working on #216835 (closed) and reviewing: !32605 (merged), I have noticed that:
- Gitlab::Metrics::Transaction is not generic enough. It already has a
counter
andset
method but it does not supportobserve
for histograms, and it forces name patterngitlab_transaction_*_total
for Prometheus metrics.
It would be nice if we could just simply write:
transaction.increment(:http_redis_requests_total, request_count)
transaction.observe(:http_redis_request_duration_seconds, query_time)
instead of
Gitlab::Metrics.counter(:http_redis_requests_total,
'Amount of calls to Redis servers during web requests',
Gitlab::Metrics::Transaction::BASE_LABELS).increment(labels, request_count)
Gitlab::Metrics.histogram(:http_redis_requests_duration_seconds,
'Query time for Redis servers during web requests',
Gitlab::Metrics::Transaction::BASE_LABELS,
Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS).observe(labels, query_time)
I saw several places where we could benefit of making more generic:
- gitlab/metrics/redis_rack_middleware.rb
- gitlab/metrics/subscribers/rails_cache.rb
- gitlab/middleware/rails_queue_duration.rb
- gitlab/metrics.rb
- gitlab/metrics/subscribers/action_view.rb
- We removed influx DB entry points (!31149 (merged)), part of #122311 (closed), but there is still a lot of
transaction.increment
andtransaction.set
calls that are just simply skipped since they are usinguse_prometheus
=false
.
def increment(name, value, use_prometheus = true)
self.class.transaction_metric(name, :counter).increment(labels, value) if use_prometheus
end
current_transaction.increment(:cache_read_miss_count, 1, false)
I found several places that this is the case:
Edited by Nikola Milojevic