Skip to content
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.

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

Add a log when sipp scenario ends due to rtp or echo errors #600

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smititelu
Copy link
Contributor

@smititelu smititelu commented Dec 6, 2022

Previously, when got some rtp/echo errors, sipp returned a failure code upon successful completion of xml scenario.

Now it returns success if xml scenario completes, and just logs rtp errors.

related to #569

Copy link
Member

@wdoekes wdoekes left a comment

Choose a reason for hiding this comment

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

Not fond of this.

I would like to see both the (xml) scenario and the RTP status in the exit code.

If the RTPCHECK_FAILED is a bit that is OR'ed (e.g. 64), you could ignore that and get the XML status (e.g. (status & ~64) == 0).

@smititelu
Copy link
Contributor Author

smititelu commented Dec 7, 2022

Let's see if I understood correctly:

If an SIPp xml scenario completes successfully from SIP point of view, but it had rtp errors, it should return (EXIT_TEST_OK|EXIT_RTPCHECK_FAILED)...

... and then, on OS side, check the return code of last ran (sipp) command and take out the EXIT_RTPCHECK_FAILED (e.g. $? & ~EXIT_RTPCHECK_FAILED) ?

If that is the case, I don't agree with it. Because an end user will see sipp scenario complete ok but "$?" OS var will be != 0. So basically, sipp scenario failed, even if you see it completing successfully. That would be misleading, imho. Logging an warning but still succeeding would be more clear, imho.

@smititelu smititelu changed the title Return ok if xml scenario finished ok. Log rtp or echo errors Add a log when sipp scenario ends due to rtp or echo errors Dec 7, 2022
@smititelu
Copy link
Contributor Author

Updated PR to just log when scenario ends due to rtp/echo errors.

src/sipp.cpp Outdated
@@ -1134,6 +1134,7 @@ void sipp_exit(int rc, int rtp_errors, int echo_errors)
// then everything is OK!
if ((rtp_errors > 0) || (echo_errors > 0))
{
ERROR("GOT rtp_errors = %d and echo_errors = %d", rtp_errors, echo_errors);
Copy link
Member

Choose a reason for hiding this comment

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

ERROR() does not return, so EXIT_RTPCHECK_FAILED is not returned to the OS. You'd want WARNING.

Maybe "Got %d rtp_errors and %d echo_errors" for more pleasant english.

@smititelu
Copy link
Contributor Author

Updated log to warning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants