This repository was archived by the owner on Oct 24, 2023. It is now read-only.
Add HTTP responses codes metric #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
alepuccetti force-pushed the alessandro/responses branch from 210a2a2 to a85dcda Compare 3 tasks
alepuccetti force-pushed the alessandro/responses branch 2 times, most recently from f50d398 to a0d27c2 Compare 
| @2opremio This should be good to go too. |
2opremio reviewed Oct 19, 2016
README.md Outdated
| Weave Scope UI shows this information as single value and graph which are updated every second. | ||
| The single value is the latest value measured, while the graph is the historical representation of recents values. | ||
| | ||
| ### HTTP Responses Codes |
2opremio reviewed Oct 19, 2016
README.md Outdated
| @@ -1,29 +1,64 @@ | |||
| # Scope HTTP Statistics Plugin | |||
| | |||
| The Scope HTTP Statistics plugin provide to the [Weave Scope](https://github.com/weaveworks/scope) user statistics about HTTP traffic at the process level using eBPF, as of now only the number of HTTP requests are collected. | |||
There was a problem hiding this comment.
Can you try to use a simpler phrasing? (sorry, I missed this in a prior review)
alepuccetti force-pushed the alessandro/responses branch from a0d27c2 to d6d3b84 Compare | @2opremio Updated |
2opremio reviewed Oct 19, 2016
http-statistics.py Outdated
| # Aggregate the kernel's per-task http request counts into userland's | ||
| # per-process counts | ||
| req_count_table = self.bpf.get_table(EBPF_TABLE_NAME) | ||
| req_count_table = self.bpf.get_table(EBPF_TABLE_REQUESTS_RATE_NAME) |
There was a problem hiding this comment.
s/TABLE_REQUESTS_RATE/REQUEST_RATE_TABLE/
There was a problem hiding this comment.
I suppose I should do also.
s/EBPF_TABLE_RESPONSES_CODE_NAME/EBPF_RESPONSE_CODE_TABLE_NAME/g
2opremio reviewed Oct 19, 2016
| }; | ||
| | ||
| /* HTTP responses look like "HTTP/1.1 XXX". We only need to read the first 12 characters */ | ||
| #define HTTP_CODE_MSG_LEN 12 |
There was a problem hiding this comment.
Can you do the same for kprobe__skb_copy_datagram_iter ? (we read 7 characters)
2opremio reviewed Oct 19, 2016
| override: | ||
| - echo "no dependencies" | ||
| | ||
| test: |
There was a problem hiding this comment.
Could you add a test to verify that the ebpf code compiles?
There was a problem hiding this comment.
It is difficult because the ebpf code is compiled at run-time and not at circleCI time. Could further tests be added later in another PR?
| LGTM (comments-aside). If possible I would like alban to sign-off the ebpf code (I am still a newbie myself). |
| I reviewed the ebpf code before and it LGTM. |
alepuccetti force-pushed the alessandro/responses branch from 857db0d to 76ba724 Compare added 6 commits October 19, 2016 13:46
This patch adds the new HTTP responses codes metric. The eBPF program searches into the buffer the string `HTTP/1.1 XXX`, where XXX is the 3 digits HTTP response code. The statistics are exposed to Weave Scope using the report interface. `tcp_sendmsg()` invokes `copy_from_iter()` multiple times to copy data from the iovec. We inspect the data (`addr`) passed to `copy_from_iter()` only for its first invocation. This limitation does not impact significantly the metric because is very unlikely that a response code is splitted in multiple iovec or is not at the beginning of the first iovec's element. Codes tracked: - 100: Continue - 200: OK - 201: Created - 202: Accepted - 204: No Content - 308: Permanent Redirect Redirect - 400: Bad Request - 401: Unauthorized - 403: Forbidden - 404: Not Found - 408: Request Timeout - 451: Unavailable For Legal Reasons - 500: Internal Server Error - 501: Not Implemented - 502: Bad Gateway - 503: Service Unavailable
This patch explains better HTTP Requests metrics and adds the description for HTTP Responses Codes metrics.
Use tabs, and proper comments formatting.
DockerHub does not allow the organization name to have dashes
This patch fixes a bug in computing the HTTP response metrics priority. Using `http_code` to compute the priority fails when the code is mapped to `OTHERS`.
alepuccetti force-pushed the alessandro/responses branch from 76ba724 to 0407f36 Compare added 2 commits October 19, 2016 13:48
git-subtree-dir: tools git-subtree-split: b990f488bdc7909b62d9245bc4b4caf58f5ae7ea
| @2opremio I addressed your comments and I also fixed a bug. |
| This PR is relevant to weaveworks/scope#1631 |
| This is good to go (as per my previous comment). @alepuccetti feel free to merge away! |
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This metric shows in the UI the HTTP responses codes rate for those codes that are seen in the time interval where Scope UI shows historical data.
For convenience, I added the
build-toolsand thecircle.ymlto use cricleci and dockerhub.This PR is rebased on #1
@2opremio PTAL