You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/01/10 09:12:01 UTC
[GitHub] [apisix] jagerzhang opened a new pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
jagerzhang opened a new pull request #6063:
URL: https://github.com/apache/apisix/pull/6063
### What this PR does / why we need it:
supports to logging apisix_latency and upstream_latency(copy from tapisix by [kyroslin](https://github.com/kyroslin)): https://github.com/apache/apisix/issues/5910
Discuss: https://github.com/apache/apisix/discussions/6010
### Pre-submission checklist:
The original code is used directly without adding other new logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r781686346
##########
File path: apisix/utils/log-util.lua
##########
@@ -96,6 +96,8 @@ local function get_full_log(ngx, conf)
}
end
+ local latency, upstream_latency, apisix_latency = _M.latency_details_in_ms(ctx)
Review comment:
Better to refactor the method and call the method like `= latency_details_in_ms(ctx)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r781831175
##########
File path: apisix/utils/log-util.lua
##########
@@ -120,7 +122,9 @@ local function get_full_log(ngx, conf)
consumer = consumer,
client_ip = core.request.get_remote_client_ip(ngx.ctx.api_ctx),
start_time = ngx.req.start_time() * 1000,
- latency = (ngx_now() - ngx.req.start_time()) * 1000
+ latency = latency,
+ upstream_latency = upstream_latency,
+ apisix_latency = apisix_latency
Review comment:
Maybe in this PR do like
`apisix_latency = apisix_latency - upstream_latency` to get the real apisix_latency, cc @spacewander
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] jagerzhang commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
jagerzhang commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r782679511
##########
File path: apisix/utils/log-util.lua
##########
@@ -96,6 +96,8 @@ local function get_full_log(ngx, conf)
}
end
+ local latency, upstream_latency, apisix_latency = _M.latency_details_in_ms(ctx)
Review comment:
ok, already update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r781686346
##########
File path: apisix/utils/log-util.lua
##########
@@ -96,6 +96,8 @@ local function get_full_log(ngx, conf)
}
end
+ local latency, upstream_latency, apisix_latency = _M.latency_details_in_ms(ctx)
Review comment:
Better to refactor the method and avoid calling the method like `= latency_details_in_ms(ctx)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6063:
URL: https://github.com/apache/apisix/pull/6063
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r781726288
##########
File path: apisix/utils/log-util.lua
##########
@@ -120,7 +122,9 @@ local function get_full_log(ngx, conf)
consumer = consumer,
client_ip = core.request.get_remote_client_ip(ngx.ctx.api_ctx),
start_time = ngx.req.start_time() * 1000,
- latency = (ngx_now() - ngx.req.start_time()) * 1000
+ latency = latency,
+ upstream_latency = upstream_latency,
+ apisix_latency = apisix_latency
Review comment:
I think use `apisix_latency` is confused, refer another discussment https://github.com/apache/apisix/issues/6029.
The `apisix_latency` includes client sending data via network, not apisix exclusive. From my experience, it is large when tranfer big file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r782052157
##########
File path: apisix/utils/log-util.lua
##########
@@ -120,7 +122,9 @@ local function get_full_log(ngx, conf)
consumer = consumer,
client_ip = core.request.get_remote_client_ip(ngx.ctx.api_ctx),
start_time = ngx.req.start_time() * 1000,
- latency = (ngx_now() - ngx.req.start_time()) * 1000
+ latency = latency,
+ upstream_latency = upstream_latency,
+ apisix_latency = apisix_latency
Review comment:
The latency from the client exists through the whole request process.
For file upload, it affects the apisix latency much.
But for file download, it affects the upstream latency much.
Both latencies contain the latency from the client.
So it's OK to keep the current behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #6063: feat: supports to logging apisix_latency and upstream_latency.
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #6063:
URL: https://github.com/apache/apisix/pull/6063#discussion_r781726288
##########
File path: apisix/utils/log-util.lua
##########
@@ -120,7 +122,9 @@ local function get_full_log(ngx, conf)
consumer = consumer,
client_ip = core.request.get_remote_client_ip(ngx.ctx.api_ctx),
start_time = ngx.req.start_time() * 1000,
- latency = (ngx_now() - ngx.req.start_time()) * 1000
+ latency = latency,
+ upstream_latency = upstream_latency,
+ apisix_latency = apisix_latency
Review comment:
I think use `apisix_latency` is confused, refer another discussment https://github.com/apache/apisix/issues/6029.
The `apisix_latenct` includes client sending data via network, not apisix exclusive. From my experience, it is large when tranfer big file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org