[Asterisk-code-review] Add core Prometheus support to Asterisk (...asterisk[master])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/11337 )

Change subject: Add core Prometheus support to Asterisk
......................................................................


Patch Set 7: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c 
File res/res_prometheus.c:

https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@283 
PS7, Line 283: 
Should have a check for NULL metric.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@295 
PS7, Line 295: 		AST_LIST_TRAVERSE_SAFE_BEGIN(&existing->children, child, entry) {
Not modifying the linked list so _SAFE_ isn't needed.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@314 
PS7, Line 314: 	AST_VECTOR_APPEND(&metrics, metric);
Can fail if vector needs to grow.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@333 
PS7, Line 333: 			if (!existing) {
I don't think this can happen.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@350 
PS7, Line 350: 						AST_LIST_INSERT_TAIL(&root->children, child, entry);
So the first child of the removed metric gets nominated to be the new parent of the rest of the list entries?


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@527 
PS7, Line 527: 
Probably want to check that callback->name and callback->callback_fn aren't NULL;


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@541 
PS7, Line 541: 		if (!entry) {
Don't think this can happen.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@601 
PS7, Line 601: 		if (!callback) {
Don't think this can happen.


https://gerrit.asterisk.org/#/c/11337/7/res/res_prometheus.c@611 
PS7, Line 611: 		if (!metric) {
Don't think this can happen.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11337
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I891433a272c92fd11c705a2c36d65479a415ec42
Gerrit-Change-Number: 11337
Gerrit-PatchSet: 7
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Comment-Date: Mon, 20 May 2019 16:03:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190520/ddbca70d/attachment.html>