Skip to content

Commit eeb84c5

Browse files
committed
Register quota metrics unconditionally
Both quota gates in monitoringHandler tested config.isQuotaEnabled -- the function reference rather than a call -- so both were always true and all six quota metrics were always registered. Drop the conditionals rather than calling the function. Always registering is what has actually happened since these checks were written, nothing depends on the metrics disappearing when quota is off, and gating registration would mean guarding every metric touch point against a metric that was never created: crrCacheToProm reached bucketsWithQuota unguarded, and the scuba wrapper observes its histogram in a finally block, on a path the Veeam capacity route hits with no quota gating at all. Behaviour is unchanged -- the six s3_cloudserver_quota_* series are exposed exactly as before -- so this is a dead code removal. Issue: CLDSRV-972
1 parent 2535370 commit eeb84c5

2 files changed

Lines changed: 63 additions & 40 deletions

File tree

lib/utilities/monitoringHandler.js

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -65,48 +65,39 @@ const httpResponseSizeBytes = new client.Summary({
6565
help: 'Cloudserver HTTP response sizes in bytes',
6666
});
6767

68-
let quotaEvaluationDuration;
69-
let utilizationMetricsRetrievalDuration;
70-
let utilizationServiceAvailable;
71-
let bucketsWithQuota;
72-
let accountsWithQuota;
73-
let requestWithQuotaMetricsUnavailable;
74-
75-
if (config.isQuotaEnabled) {
76-
quotaEvaluationDuration = new client.Histogram({
77-
name: 's3_cloudserver_quota_evaluation_duration_seconds',
78-
help: 'Duration of the quota evaluation operation',
79-
labelNames: ['action', 'code', 'type'],
80-
buckets: [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5, 1],
81-
});
68+
const quotaEvaluationDuration = new client.Histogram({
69+
name: 's3_cloudserver_quota_evaluation_duration_seconds',
70+
help: 'Duration of the quota evaluation operation',
71+
labelNames: ['action', 'code', 'type'],
72+
buckets: [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5, 1],
73+
});
8274

83-
utilizationMetricsRetrievalDuration = new client.Histogram({
84-
name: 's3_cloudserver_quota_metrics_retrieval_duration_seconds',
85-
help: 'Duration of the utilization metrics retrieval operation',
86-
labelNames: ['code', 'class'],
87-
buckets: [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5],
88-
});
75+
const utilizationMetricsRetrievalDuration = new client.Histogram({
76+
name: 's3_cloudserver_quota_metrics_retrieval_duration_seconds',
77+
help: 'Duration of the utilization metrics retrieval operation',
78+
labelNames: ['code', 'class'],
79+
buckets: [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.2, 0.5],
80+
});
8981

90-
utilizationServiceAvailable = new client.Gauge({
91-
name: 's3_cloudserver_quota_utilization_service_available',
92-
help: 'Availability of the utilization service',
93-
});
82+
const utilizationServiceAvailable = new client.Gauge({
83+
name: 's3_cloudserver_quota_utilization_service_available',
84+
help: 'Availability of the utilization service',
85+
});
9486

95-
bucketsWithQuota = new client.Gauge({
96-
name: 's3_cloudserver_quota_buckets_count',
97-
help: 'Total number of buckets quota',
98-
});
87+
const bucketsWithQuota = new client.Gauge({
88+
name: 's3_cloudserver_quota_buckets_count',
89+
help: 'Total number of buckets quota',
90+
});
9991

100-
accountsWithQuota = new client.Gauge({
101-
name: 's3_cloudserver_quota_accounts_count',
102-
help: 'Total number of account quota',
103-
});
92+
const accountsWithQuota = new client.Gauge({
93+
name: 's3_cloudserver_quota_accounts_count',
94+
help: 'Total number of account quota',
95+
});
10496

105-
requestWithQuotaMetricsUnavailable = new client.Counter({
106-
name: 's3_cloudserver_quota_unavailable_count',
107-
help: 'Total number of requests with quota metrics unavailable',
108-
});
109-
}
97+
const requestWithQuotaMetricsUnavailable = new client.Counter({
98+
name: 's3_cloudserver_quota_unavailable_count',
99+
help: 'Total number of requests with quota metrics unavailable',
100+
});
110101

111102
// Lifecycle duration metric, to track the completion of restore.
112103
// This metric is used to track the time it takes to complete the lifecycle operation (restore).
@@ -189,10 +180,10 @@ function crrCacheToProm(crrResults) {
189180
if (crrResults.getObjectCount) {
190181
numberOfBuckets.set(crrResults.getObjectCount.buckets || 0);
191182
numberOfObjects.set(crrResults.getObjectCount.objects || 0);
183+
bucketsWithQuota.set(crrResults.getObjectCount.bucketWithQuotaCount || 0);
192184
}
193-
if (config.isQuotaEnabled) {
194-
bucketsWithQuota.set(crrResults?.getObjectCount?.bucketWithQuotaCount || 0);
195-
accountsWithQuota.set(crrResults?.getVaultReport?.accountWithQuotaCount || 0);
185+
if (crrResults.getVaultReport) {
186+
accountsWithQuota.set(crrResults.getVaultReport.accountWithQuotaCount || 0);
196187
}
197188
if (crrResults.getDataDiskUsage) {
198189
dataDiskAvailable.set(crrResults.getDataDiskUsage.available || 0);

tests/unit/utils/monitoring.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const promclient = require('prom-client');
33
const sinon = require('sinon');
44

55
const monitoring = require('../../../lib/utilities/monitoringHandler');
6+
const { config } = require('../../../lib/Config');
67

78
describe('Monitoring: endpoint', () => {
89
const sandbox = sinon.createSandbox();
@@ -139,4 +140,35 @@ describe('Monitoring: endpoint', () => {
139140
await fetchMetrics({ method: 'GET', url: '/metrics' }, res);
140141
assert(parseHttpResponseSize(res.end.args[1][0]) === responseSize + 7532);
141142
});
143+
144+
const quotaMetricNames = [
145+
's3_cloudserver_quota_evaluation_duration_seconds',
146+
's3_cloudserver_quota_metrics_retrieval_duration_seconds',
147+
's3_cloudserver_quota_utilization_service_available',
148+
's3_cloudserver_quota_buckets_count',
149+
's3_cloudserver_quota_accounts_count',
150+
's3_cloudserver_quota_unavailable_count',
151+
];
152+
153+
it('should register quota metrics even when quota is disabled', async () => {
154+
assert.strictEqual(config.isQuotaEnabled(), false);
155+
156+
await fetchMetrics({ method: 'GET', url: '/metrics' }, res);
157+
const metrics = res.end.args[0][0];
158+
159+
quotaMetricNames.forEach(name => assert(metrics.includes(name), `${name} is not registered`));
160+
});
161+
162+
it('should report the quota counts from crrCacheToProm', async () => {
163+
monitoring.crrCacheToProm({
164+
getObjectCount: { buckets: 1, objects: 2, bucketWithQuotaCount: 3 },
165+
getVaultReport: { accountWithQuotaCount: 4 },
166+
});
167+
168+
await fetchMetrics({ method: 'GET', url: '/metrics' }, res);
169+
const metrics = res.end.args[0][0];
170+
171+
assert(metrics.includes('\ns3_cloudserver_quota_buckets_count 3'));
172+
assert(metrics.includes('\ns3_cloudserver_quota_accounts_count 4'));
173+
});
142174
});

0 commit comments

Comments
 (0)