Fixes #1182: Refactor Prometheus metrics reporting (ISSUE-1182 part 2) #1200
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1200 +/- ##
==========================================
+ Coverage 78.22% 78.31% +0.08%
==========================================
Files 244 244
Lines 63144 63219 +75
Branches 5846 5852 +6
==========================================
+ Hits 49392 49507 +115
+ Misses 11098 11075 -23
+ Partials 2654 2637 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…UE-1182 part 2) o Implement dynamic registration of each alloc_pool metric o Refactor http-libwebsockets.c /metrics implementation o Update unit tests
kgiusti
force-pushed
the
ISSUE-1182-PART02
branch
from
6fa9753 to
6860923
Compare
| size_t current; | ||
| bool headers_sent; | ||
| stats_request_state_t *context; | ||
| stats_request_state_t *state; |
There was a problem hiding this comment.
Why not simply move the response_complete flag into stats_request_state_t and get rid of stats_t ?
There was a problem hiding this comment.
Most excellent question. It's because I'm paranoid.
It might be possible to get a spurious WRITEABLE event after the request is complete and we've freed the state. Can't tell for sure from the LWS code. Thought best to keep the flag out of the free-able state in case the spurious WRITEABLE occurred after the response was completed (and state freed).
|
|
||
| // encode stats into buffer | ||
|
|
||
| if (lws_add_http_header_status(wsi, HTTP_STATUS_OK, &start, end) |
There was a problem hiding this comment.
Can we please add a comment here.
Looking at the code, I am assuming that it is trying to first set the response header status to 200, sets the content type and text/plain and adds a connection: header and finally finishes off the http header and if any of these operations fail, the whole request fails.
There was a problem hiding this comment.
Can we please add a comment here.
Don't worry about this - the log message Metrics request failed: cannot send headers is good enough
| # sanity check that all expected stats are reported | ||
| resp = urlopen(f"http://localhost:{port}/metrics", cafile=self.ssl_file('ca-certificate.pem')) | ||
| self.assertEqual(200, resp.getcode()) | ||
| metrics = [x for x in resp.read().decode('utf-8').splitlines() if not x.startswith("#")] |
There was a problem hiding this comment.
WHOA !! I just got Conway-ed !!
o Implement dynamic registration of each alloc_pool metric
o Refactor http-libwebsockets.c /metrics implementation
o Update unit tests