Skip to content

Add HTTP result code to USDT milestone_sm_finish #12937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
moonchen wants to merge 1 commit into
base: master
Choose a base branch
from
+6 −7
Open

Add HTTP result code to USDT milestone_sm_finish #12937

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
ATS_PROBE1(milestone_sm_finish, sm_id);

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@moonchen moonchen Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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 (status_code == HTTPStatus::OK) {
int64_t p_resp_cl = t_state.hdr_info.client_response.get_content_length();
int64_t resp_size = client_response_body_bytes;
if (!((p_resp_cl == -1 || p_resp_cl == resp_size || resp_size == 0))) {
Expand Down Expand Up @@ -7815,11 +7818,7 @@ HttpSM::update_stats()
fd = -1;
}
}
// get the status code, lame that we have to check to see if it is valid or we will assert in the method call
int status = 0;
if (t_state.hdr_info.client_response.valid()) {
status = static_cast<int>(t_state.hdr_info.client_response.status_get());
}
int status = static_cast<int>(status_code);
char client_ip[INET6_ADDRSTRLEN];
ats_ip_ntop(&t_state.client_info.src_addr, client_ip, sizeof(client_ip));
Error("[%" PRId64 "] Slow Request: "
Expand Down