From 97cf8a672e137ba02143fa29bd786c449139b9fd Mon Sep 17 00:00:00 2001
From: Kamil Kisiela <kamil.kisiela@gmail.com>
Date: Wed, 5 Oct 2022 14:09:56 +0200
Subject: [PATCH] Use Histogram instead of Summary for durations (#455)

---
 .../services/usage-ingestor/src/ingestor.ts    |  5 +----
 .../services/usage-ingestor/src/metrics.ts     |  5 -----
 packages/services/usage/src/index.ts           |  4 ++--
 packages/services/usage/src/metrics.ts         | 18 +++++++++---------
 packages/services/usage/src/usage.ts           |  8 ++++----
 5 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/packages/services/usage-ingestor/src/ingestor.ts b/packages/services/usage-ingestor/src/ingestor.ts
index 5d17a95b8..4286ed434 100644
--- a/packages/services/usage-ingestor/src/ingestor.ts
+++ b/packages/services/usage-ingestor/src/ingestor.ts
@@ -2,7 +2,6 @@ import { Kafka, KafkaMessage, logLevel } from 'kafkajs';
 import { decompress } from '@hive/usage-common';
 import {
   errors,
-  processTime,
   processDuration,
   reportMessageBytes,
   ingestedOperationsWrites,
@@ -150,8 +149,7 @@ export function createIngestor(config: {
       autoCommitThreshold: 2,
       partitionsConsumedConcurrently: config.kafka.concurrency,
       eachMessage({ message }) {
-        const stopTimer = processTime.startTimer();
-        const processDurationStop = processDuration.startTimer();
+        const stopTimer = processDuration.startTimer();
         return processMessage({
           message,
           logger,
@@ -164,7 +162,6 @@ export function createIngestor(config: {
           })
           .finally(() => {
             stopTimer();
-            processDurationStop();
           });
       },
     });
diff --git a/packages/services/usage-ingestor/src/metrics.ts b/packages/services/usage-ingestor/src/metrics.ts
index f3d5106fe..8d5031ab3 100644
--- a/packages/services/usage-ingestor/src/metrics.ts
+++ b/packages/services/usage-ingestor/src/metrics.ts
@@ -15,11 +15,6 @@ export const totalOperations = new metrics.Counter({
   help: 'Number of raw operations received by usage ingestor service',
 });
 
-export const processTime = new metrics.Summary({
-  name: 'usage_ingestor_process_time',
-  help: 'Time spent processing and writing reports',
-});
-
 export const processDuration = new metrics.Histogram({
   name: 'usage_ingestor_process_duration_seconds',
   help: 'Time spent processing and writing reports',
diff --git a/packages/services/usage/src/index.ts b/packages/services/usage/src/index.ts
index 61045c4cd..5f59f1fa8 100644
--- a/packages/services/usage/src/index.ts
+++ b/packages/services/usage/src/index.ts
@@ -8,7 +8,7 @@ import {
   httpRequestsWithoutToken,
   httpRequestsWithNonExistingToken,
   httpRequestsWithNoAccess,
-  collectLatency,
+  collectDuration,
   droppedReports,
 } from './metrics';
 import type { IncomingLegacyReport, IncomingReport } from './types';
@@ -124,7 +124,7 @@ async function main() {
 
         const retentionInfo = (await rateLimit?.getRetentionForTargetId?.(tokenInfo.target)) || null;
 
-        const stopTimer = collectLatency.startTimer();
+        const stopTimer = collectDuration.startTimer();
         try {
           await collect(req.body, tokenInfo, retentionInfo);
           stopTimer();
diff --git a/packages/services/usage/src/metrics.ts b/packages/services/usage/src/metrics.ts
index bcb946169..9d6d17658 100644
--- a/packages/services/usage/src/metrics.ts
+++ b/packages/services/usage/src/metrics.ts
@@ -66,19 +66,19 @@ export const invalidRawOperations = new metrics.Counter({
   help: 'Number of invalid raw operations dropped by usage service',
 });
 
-export const collectLatency = new metrics.Summary({
-  name: 'usage_raw_collect_latency',
-  help: 'Collect latency',
+export const collectDuration = new metrics.Histogram({
+  name: 'usage_raw_collect_duration_seconds',
+  help: 'Collect duration in seconds',
 });
 
-export const compressLatency = new metrics.Summary({
-  name: 'usage_raw_compress_latency',
-  help: 'Compress latency',
+export const compressDuration = new metrics.Histogram({
+  name: 'usage_raw_compress_duration_seconds',
+  help: 'Compress duration in seconds',
 });
 
-export const kafkaLatency = new metrics.Summary({
-  name: 'usage_raw_kafka_latency',
-  help: 'Kafka latency',
+export const kafkaDuration = new metrics.Histogram({
+  name: 'usage_raw_kafka_duration_seconds',
+  help: 'Kafka duration in seconds',
 });
 
 export const bufferFlushes = new metrics.Counter({
diff --git a/packages/services/usage/src/usage.ts b/packages/services/usage/src/usage.ts
index 6b8184cdd..c91790a60 100644
--- a/packages/services/usage/src/usage.ts
+++ b/packages/services/usage/src/usage.ts
@@ -9,8 +9,8 @@ import {
   totalOperations,
   totalReports,
   totalLegacyReports,
-  kafkaLatency,
-  compressLatency,
+  kafkaDuration,
+  compressDuration,
   bufferFlushes,
   estimationError,
 } from './metrics';
@@ -168,11 +168,11 @@ export function createUsage(config: {
     async sender(reports, estimatedSizeInBytes, batchId, validateSize) {
       const numOfOperations = reports.reduce((sum, report) => report.size + sum, 0);
       try {
-        const compressLatencyStop = compressLatency.startTimer();
+        const compressLatencyStop = compressDuration.startTimer();
         const value = await compress(JSON.stringify(reports)).finally(() => {
           compressLatencyStop();
         });
-        const stopTimer = kafkaLatency.startTimer();
+        const stopTimer = kafkaDuration.startTimer();
 
         estimationError.observe(Math.abs(estimatedSizeInBytes - value.byteLength) / value.byteLength);