Skip to content

Commit 73a9391

Browse files
naved001naved001
authored
Merge pull request #52 from naved001/bug-fix/dont-charge-stopped-pods
Bug fix/dont charge stopped pods
2 parents 905a43a + c3c4075 commit 73a9391

File tree

2 files changed

+137
-25
lines changed

2 files changed

+137
-25
lines changed

‎openshift_metrics/tests/test_utils.py

Lines changed: 95 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ def test_condense_metrics(self):
397397
"cpu": 10,
398398
"mem": 15,
399399
},
400-
60: {
400+
900: {
401401
"cpu": 10,
402402
"mem": 15,
403403
}
@@ -409,7 +409,7 @@ def test_condense_metrics(self):
409409
"cpu": 2,
410410
"mem": 256,
411411
},
412-
100: {
412+
900: {
413413
"cpu": 2,
414414
"mem": 256,
415415
}
@@ -422,7 +422,7 @@ def test_condense_metrics(self):
422422
0: {
423423
"cpu": 10,
424424
"mem": 15,
425-
"duration": 120
425+
"duration": 1800
426426
}
427427
}
428428
},
@@ -431,7 +431,7 @@ def test_condense_metrics(self):
431431
0: {
432432
"cpu": 2,
433433
"mem": 256,
434-
"duration": 200
434+
"duration": 1800
435435
}
436436
}
437437
},
@@ -473,15 +473,15 @@ def test_condense_metrics_with_change(self):
473473
"cpu": 20,
474474
"mem": 25,
475475
},
476-
60: {
476+
900: {
477477
"cpu": 20,
478478
"mem": 25,
479479
},
480-
120: {
480+
1800: {
481481
"cpu": 25,
482482
"mem": 25,
483483
},
484-
180: {
484+
2700: {
485485
"cpu": 20,
486486
"mem": 25,
487487
}
@@ -494,17 +494,17 @@ def test_condense_metrics_with_change(self):
494494
0: {
495495
"cpu": 20,
496496
"mem": 25,
497-
"duration": 120
497+
"duration": 1800
498498
},
499-
120: {
499+
1800: {
500500
"cpu": 25,
501501
"mem": 25,
502-
"duration": 60
502+
"duration": 900
503503
},
504-
180: {
504+
2700: {
505505
"cpu": 20,
506506
"mem": 25,
507-
"duration": 60
507+
"duration": 900
508508
}
509509
}
510510
},
@@ -521,7 +521,7 @@ def test_condense_metrics_skip_metric(self):
521521
"mem": 35,
522522
"gpu": 1,
523523
},
524-
60: {
524+
900: {
525525
"cpu": 30,
526526
"mem": 35,
527527
"gpu": 2,
@@ -536,14 +536,95 @@ def test_condense_metrics_skip_metric(self):
536536
"cpu": 30,
537537
"mem": 35,
538538
"gpu": 1,
539-
"duration": 120
539+
"duration": 1800
540540
}
541541
}
542542
},
543543
}
544544
condensed_dict = utils.condense_metrics(test_input_dict,['cpu','mem'])
545545
self.assertEqual(condensed_dict, expected_condensed_dict)
546546

547+
def test_condense_metrics_with_timeskips(self):
548+
test_input_dict = {
549+
"pod1": {
550+
"metrics": {
551+
0: {
552+
"cpu": 1,
553+
"mem": 4,
554+
},
555+
900: {
556+
"cpu": 1,
557+
"mem": 4,
558+
},
559+
1800: {
560+
"cpu": 1,
561+
"mem": 4,
562+
},
563+
5400: { # time skipped
564+
"cpu": 1,
565+
"mem": 4,
566+
},
567+
6300: {
568+
"cpu": 1,
569+
"mem": 4,
570+
},
571+
8100: { # metric changed and time skipped
572+
"cpu": 2,
573+
"mem": 8,
574+
},
575+
9000: {
576+
"cpu": 2,
577+
"mem": 8,
578+
},
579+
}
580+
},
581+
"pod2": {
582+
"metrics": {
583+
0: {
584+
"cpu": 2,
585+
"mem": 16,
586+
},
587+
900: {
588+
"cpu": 2,
589+
"mem": 16,
590+
}
591+
}
592+
},
593+
}
594+
expected_condensed_dict = {
595+
"pod1": {
596+
"metrics": {
597+
0: {
598+
"cpu": 1,
599+
"mem": 4,
600+
"duration": 2700
601+
},
602+
5400: {
603+
"cpu": 1,
604+
"mem": 4,
605+
"duration": 1800
606+
},
607+
8100: {
608+
"cpu": 2,
609+
"mem": 8,
610+
"duration": 1800
611+
},
612+
}
613+
},
614+
"pod2": {
615+
"metrics": {
616+
0: {
617+
"cpu": 2,
618+
"mem": 16,
619+
"duration": 1800
620+
}
621+
}
622+
},
623+
}
624+
condensed_dict = utils.condense_metrics(test_input_dict,['cpu','mem'])
625+
self.assertEqual(condensed_dict, expected_condensed_dict)
626+
627+
547628
class TestWriteMetricsByPod(TestCase):
548629

549630
@mock.patch('openshift_metrics.utils.get_namespace_attributes')

‎openshift_metrics/utils.py

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,37 @@ def merge_metrics(metric_name, metric_list, output_dict):
252252
def condense_metrics(input_metrics_dict, metrics_to_check):
253253
"""
254254
Checks if the value of metrics is the same, and removes redundant
255-
metrics while updating the duration
255+
metrics while updating the duration. If there's a gap in the reported
256+
metrics then don't count that as part of duration.
257+
258+
Here's a sample input dictionary in which I have separated missing metrics
259+
or different metrics by empty lines.
260+
261+
{'naved-test+test-pod': {'gpu_type': 'No GPU',
262+
'metrics': {1711741500: {'cpu_request': '1',
263+
'memory_request': '3221225472'},
264+
1711742400: {'cpu_request': '1',
265+
'memory_request': '3221225472'},
266+
1711743300: {'cpu_request': '1',
267+
'memory_request': '3221225472'},
268+
1711744200: {'cpu_request': '1',
269+
'memory_request': '3221225472'},
270+
271+
1711746000: {'cpu_request': '1',
272+
'memory_request': '3221225472'},
273+
274+
1711746900: {'cpu_request': '1',
275+
'memory_request': '4294967296'},
276+
1711747800: {'cpu_request': '1',
277+
'memory_request': '4294967296'},
278+
1711748700: {'cpu_request': '1',
279+
'memory_request': '4294967296'},
280+
281+
1711765800: {'cpu_request': '1',
282+
'memory_request': '4294967296'}},
283+
'namespace': 'naved-test'}}
256284
"""
285+
interval = STEP_MIN * 60
257286
condensed_dict = {}
258287
for pod, pod_dict in input_metrics_dict.items():
259288
metrics_dict = pod_dict["metrics"]
@@ -262,26 +291,28 @@ def condense_metrics(input_metrics_dict, metrics_to_check):
262291

263292
start_epoch_time = epoch_times_list[0]
264293

265-
# calculate the interval if we have more than 1 measurement, otherwise
266-
# use the STEP_MIN from the query as best guess
267-
if len(epoch_times_list) > 1:
268-
interval = epoch_times_list[1] - epoch_times_list[0]
269-
else:
270-
interval = STEP_MIN * 60
271-
272294
start_metric_dict = metrics_dict[start_epoch_time].copy()
273-
for epoch_time in epoch_times_list:
295+
296+
for i in range(len(epoch_times_list)):
297+
epoch_time = epoch_times_list[i]
274298
same_metrics = True
299+
continuous_metrics = True
275300
for metric in metrics_to_check:
276301
if metrics_dict[start_epoch_time].get(metric, 0) != metrics_dict[epoch_time].get(metric, 0): # fmt: skip
277302
same_metrics = False
278303

279-
if not same_metrics:
280-
duration = epoch_time - start_epoch_time
304+
if i !=0 and epoch_time - epoch_times_list[i-1]> interval:
305+
# i.e. if the difference between 2 consecutive timestamps
306+
# is more than the expected frequency then the pod was stopped
307+
continuous_metrics = False
308+
309+
if not same_metrics or not continuous_metrics:
310+
duration = epoch_times_list[i-1] - start_epoch_time + interval
281311
start_metric_dict["duration"] = duration
282312
new_metrics_dict[start_epoch_time] = start_metric_dict
283313
start_epoch_time = epoch_time
284314
start_metric_dict = metrics_dict[start_epoch_time].copy()
315+
285316
duration = epoch_time - start_epoch_time + interval
286317
start_metric_dict["duration"] = duration
287318
new_metrics_dict[start_epoch_time] = start_metric_dict

0 commit comments

Comments
 (0)