diff --git a/userspace/sysdig/sysdig.cpp b/userspace/sysdig/sysdig.cpp index 05cb203ab6..a2480c31fa 100644 --- a/userspace/sysdig/sysdig.cpp +++ b/userspace/sysdig/sysdig.cpp @@ -196,7 +196,10 @@ static void usage() " Marathon url is optional and defaults to Mesos address, port 8080.\n" " The API servers can also be specified via the environment variable\n" " SYSDIG_MESOS_API.\n" -" -M Stop collecting after reached.\n" +" -M [a|f][a|f]\n" +" Stop collecting after reached. The two optional arguments\n" +" are whether to start and end the clock after any event (a) or the first\n" +" event that matches the filter (f). The default when unspecified is ff.\n" " -n , --numevents=\n" " Stop capturing after events\n" " --page-faults Capture user/kernel major/minor page faults\n" @@ -575,6 +578,7 @@ vector split_nextrun_args(string na) captureinfo do_inspect(sinsp* inspector, uint64_t cnt, uint64_t duration_to_tot_ns, + char *duration_flags, bool quiet, bool json, bool do_flush, @@ -613,14 +617,43 @@ captureinfo do_inspect(sinsp* inspector, if(res == SCAP_TIMEOUT) { - if(ev != NULL && ev->is_filtered_out()) - { - // - // The event has been dropped by the filtering system. - // Give the chisels a chance to run their timeout logic. - // - chisels_do_timeout(ev); - } + if(ev != NULL) { + // + // See #1696 - Sysdig capture running endless while using filters + // Previously this check was done after a filter which meant that + // we did not observe the duration rule. + // + // The choice of this compound if statement is intentional. We want + // this to effectively only "have cost" if we're using the -M option + // + // Note that we set duration_start below. Comments down there + // as to why. + // + if(duration_to_tot_ns > 0) { + + if(duration_flags[0] == 'a' && duration_start == 0) + { + duration_start = ev->get_ts(); + } + if(duration_flags[1] == 'a' && duration_start != 0) + { + if(ev->get_ts() - duration_start >= duration_to_tot_ns) + { + handle_end_of_file(print_progress, formatter); + break; + } + } + } + + if(ev->is_filtered_out()) + { + // + // The event has been dropped by the filtering system. + // Give the chisels a chance to run their timeout logic. + // + chisels_do_timeout(ev); + } + } continue; } @@ -640,17 +673,33 @@ captureinfo do_inspect(sinsp* inspector, throw sinsp_exception(inspector->getlasterr().c_str()); } - if (duration_start == 0) - { - duration_start = ev->get_ts(); - } else if(duration_to_tot_ns > 0) - { - if(ev->get_ts() - duration_start >= duration_to_tot_ns) - { - handle_end_of_file(print_progress, formatter); - break; - } - } + // + // Prior to the patch for #1696 - Sysdig capture running endless while using filters + // The duration_start would be set at the first match. This previously undocumented + // feature (it is now documented in the help text that can be seen in the commit hunk + // associated with this line of text) is useful because it allows someone to start up + // sysdig, then run their tests and not have to worry about the timing delay between + // those two events before the -M countdown timer begins. + // + // This likely unintended side-effect of the initial implementation has been preserved + // to accommodate people who have likely built workflows based on this. + // + if(duration_to_tot_ns > 0) + { + if(duration_flags[0] == 'f' && duration_start == 0) + { + duration_start = ev->get_ts(); + } + if(duration_flags[1] == 'f' && duration_start != 0) + { + if(ev->get_ts() - duration_start >= duration_to_tot_ns) + { + handle_end_of_file(print_progress, formatter); + break; + } + } + } + retval.m_nevts++; if(print_progress) @@ -773,6 +822,7 @@ sysdig_init_res sysdig_init(int argc, char **argv) sinsp_filter* display_filter = NULL; double duration = 1; int duration_to_tot = 0; + char duration_flags[] = "ff"; captureinfo cinfo; string output_format; uint32_t snaplen = 0; @@ -1034,13 +1084,22 @@ sysdig_init_res sysdig_init(int argc, char **argv) break; #endif // MINIMAL_BUILD case 'M': - duration_to_tot = atoi(optarg); + sscanf(optarg, "%d%2c", &duration_to_tot, duration_flags); if(duration_to_tot <= 0) { - throw sinsp_exception(string("invalid duration") + optarg); + throw sinsp_exception(string("invalid duration ") + optarg); res.m_res = EXIT_FAILURE; goto exit; } + if( !(duration_flags[0] == 'a' || duration_flags[0] == 'f') || + !(duration_flags[1] == 'a' || duration_flags[1] == 'f') + ) + { + throw sinsp_exception(string("invalid duration positioning ") + optarg); + res.m_res = EXIT_FAILURE; + goto exit; + } + break; case 'n': try @@ -1627,6 +1686,7 @@ sysdig_init_res sysdig_init(int argc, char **argv) cinfo = do_inspect(inspector, cnt, uint64_t(duration_to_tot*ONE_SECOND_IN_NS), + duration_flags, quiet, jflag, unbuf_flag,