You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "moonchen (via GitHub)" <gi...@apache.org> on 2026/03/04 18:10:20 UTC
[PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
moonchen opened a new pull request, #12937:
URL: https://github.com/apache/trafficserver/pull/12937
(no comment)
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-4027318685
[approve ci autest 0]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-3999760541
[approve ci AuTest 0of4]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on code in PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#discussion_r2885662304
##########
src/proxy/http/HttpSM.cc:
##########
@@ -7732,11 +7732,14 @@ HttpSM::kill_this()
void
HttpSM::update_stats()
{
- ATS_PROBE1(milestone_sm_finish, sm_id);
+ const HTTPStatus status_code =
+ t_state.hdr_info.client_response.valid() ? t_state.hdr_info.client_response.status_get() : HTTPStatus::NONE;
+
+ ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
Review Comment:
The example demonstrates the use of sm_id. It doesn't need to use status_code, and the bpftrace code should keep working as is.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#discussion_r2885318411
##########
src/proxy/http/HttpSM.cc:
##########
@@ -7732,11 +7732,14 @@ HttpSM::kill_this()
void
HttpSM::update_stats()
{
- ATS_PROBE1(milestone_sm_finish, sm_id);
+ const HTTPStatus status_code =
+ t_state.hdr_info.client_response.valid() ? t_state.hdr_info.client_response.status_get() : HTTPStatus::NONE;
+
Review Comment:
`milestone_sm_finish` USDT tracepoint now has a second argument (HTTP status). The developer tracing docs currently show this probe only using `arg0` and don’t mention the additional argument; please update the documentation/example so users know `arg1` is the HTTP status code (and what 0/HTTPStatus::NONE means when no response is available).
```suggestion
// USDT probe milestone_sm_finish:
// arg0: state machine id (sm_id)
// arg1: HTTP status code as int; 0 (HTTPStatus::NONE) when no client response is available.
```
##########
src/proxy/http/HttpSM.cc:
##########
@@ -7732,11 +7732,14 @@ HttpSM::kill_this()
void
HttpSM::update_stats()
{
- ATS_PROBE1(milestone_sm_finish, sm_id);
+ const HTTPStatus status_code =
+ t_state.hdr_info.client_response.valid() ? t_state.hdr_info.client_response.status_get() : HTTPStatus::NONE;
+
+ ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
milestones[TS_MILESTONE_SM_FINISH] = ink_get_hrtime();
if (is_action_tag_set("bad_length_state_dump")) {
- if (t_state.hdr_info.client_response.valid() && t_state.hdr_info.client_response.status_get() == HTTPStatus::OK) {
+ if (t_state.hdr_info.client_response.valid() && status_code == HTTPStatus::OK) {
Review Comment:
`status_code` is already set to `HTTPStatus::NONE` when `client_response` is invalid, so the extra `client_response.valid()` check in this condition is redundant and re-reads `valid()`. Consider simplifying the condition to rely on `status_code` (or store the validity in a local) to keep the logic single-sourced.
```suggestion
if (status_code == HTTPStatus::OK) {
```
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-3999709811
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-4136945148
Cherry-picked to 10.2.x
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#discussion_r2885652466
##########
src/proxy/http/HttpSM.cc:
##########
@@ -7732,11 +7732,14 @@ HttpSM::kill_this()
void
HttpSM::update_stats()
{
- ATS_PROBE1(milestone_sm_finish, sm_id);
+ const HTTPStatus status_code =
+ t_state.hdr_info.client_response.valid() ? t_state.hdr_info.client_response.status_get() : HTTPStatus::NONE;
+
+ ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
Review Comment:
Changing `milestone_sm_finish` from `ATS_PROBE1` to `ATS_PROBE2` is a USDT interface change: existing bpftrace/perf scripts that assume only `arg0` (sm_id) will now see an additional `arg1` (HTTP status). The repo documentation example still treats `milestone_sm_finish` as having a single argument (doc/developer-guide/debugging/tracing.en.rst around the bpftrace example), so it should be updated to reflect the new signature and describe `arg1`.
```suggestion
ATS_PROBE1(milestone_sm_finish, sm_id);
```
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-3999765492
[approve ci autest 0]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937#issuecomment-3999698866
/approve ci autest
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add HTTP result code to USDT milestone_sm_finish [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen merged PR #12937:
URL: https://github.com/apache/trafficserver/pull/12937
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org