Support Mixer HTTP filter to report sent.bytes and received.bytes attributes #1372
Conversation
src/istio/control/attribute_names.cc
Outdated
| @@ -28,6 +28,7 @@ const char AttributeName::kRequestPath[] = "request.path"; | |||
| const char AttributeName::kRequestReferer[] = "request.referer"; | |||
| const char AttributeName::kRequestScheme[] = "request.scheme"; | |||
| const char AttributeName::kRequestSize[] = "request.size"; | |||
| const char AttributeName::kReceivedBytes[] = "received.bytes"; | |||
There was a problem hiding this comment.
Hi @douglas-reid I felt it is better to name these two new attributes as "request.total_size" and "response.total_size" so it consists with existing attributes of "request.size"
There was a problem hiding this comment.
@qiwzhang request.total_size and response.total_size sgtm.
|
Please also update this file https://github.com/istio/api/blob/master/mixer/v1/global_dictionary.yaml for the new attributes you added |
|
and this document too. |
|
Please make sure to change Pilot code for the filter type change. |
9f71a81 to
e55fde3
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiwzhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue. |
| const char AttributeName::kRequestTime[] = "request.time"; | ||
| const char AttributeName::kRequestUserAgent[] = "request.useragent"; | ||
| const char AttributeName::kRequestApiKey[] = "request.api_key"; | ||
|
|
||
| const char AttributeName::kResponseCode[] = "response.code"; | ||
| const char AttributeName::kResponseDuration[] = "response.duration"; | ||
| const char AttributeName::kResponseHeaders[] = "response.headers"; | ||
| const char AttributeName::kResponseSize[] = "response.size"; | ||
| const char AttributeName::kResponseBodySize[] = "response.size"; | ||
| const char AttributeName::kResponseTotleSize[] = "response.totle_size"; |
There was a problem hiding this comment.
sorry to be late with this comment, but should these attributes be request.total_size and response.total_size ?
There was a problem hiding this comment.
request.total_size is added at line 32. Prior to this PR, we already let Mixer filter to report request.size and response.size, and they are actually request body size and response body size. So I update the variable name (e.g. s/kResponseSize/kResponseBodySize) to make it less confusing.
There was a problem hiding this comment.
@JimmyCYJ I'm actually wondering about the difference between totle and total (and what the name of the attribute is).
There was a problem hiding this comment.
That is typo. I will fix it soon. Thanks!
What this PR does / why we need it:Support Mixer HTTP filter to send attributes "request.total_size" and "response.total_size" in Report() calls. "response.total_size" records total response sent in bytes, including response headers, body, and trailers. "request.total_size" records total request received in bytes, including request headers, body, and trailers.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #1370Special notes for your reviewer:
Release note: