Flume
  1. Flume
  2. FLUME-1427

Syslog Facility calculation is wrong

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: v1.2.0
    • Fix Version/s: None
    • Component/s: Sinks+Sources
    • Labels:

      Description

      As per Syslog RFC, priority = (facility * 8) + severity, given this logic, the code to calculate facility and severity from priority should be

      severity = priority % 8 ;
      facility = (priority - severity) / 8 ;

      But in SyslogUtils's buildEvent method
      facility = priority - severity

      i.e. the / 8 is missing.

      1. syslog.patch
        0.6 kB
        Bhaskar Karambelkar

        Issue Links

          Activity

          Hide
          Bhaskar Karambelkar added a comment -

          Patch to correctly calculate facility.

          Show
          Bhaskar Karambelkar added a comment - Patch to correctly calculate facility.
          Hide
          NO NAME added a comment -

          Hey Bhaskar - thanks for the contribution!

          As a heads up, there is a common naming scheme for JIRA patches:

          e.g. FLUME-1427.patch.v1

          Just a note, hopefully the first of many contributions

          Show
          NO NAME added a comment - Hey Bhaskar - thanks for the contribution! As a heads up, there is a common naming scheme for JIRA patches: e.g. FLUME-1427 .patch.v1 Just a note, hopefully the first of many contributions
          Hide
          Mike Percy added a comment -

          Oops, apologies Bhaskar, I just submitted a similar patch over in FLUME-1470. I had missed this JIRA previously.

          Some review feedback:

          • The unit tests will fail if this patch is applied directly. If you correct the unit tests and update your patch then I would be happy to get it committed. It's a best practice to run "mvn clean install" and allow all unit tests to complete before submitting your patch.
          • Nit: The integer calculation for facility can be reduced to (priority / 8). No subtraction necessary.

          On the patch naming thing, I agree with Patrick, it's a best practice... JIRA was being problematic last night when I created the patch in FLUME-1470 and didn't bother renaming it once JIRA was back up and I was able to file a ticket

          Regards,
          Mike

          Show
          Mike Percy added a comment - Oops, apologies Bhaskar, I just submitted a similar patch over in FLUME-1470 . I had missed this JIRA previously. Some review feedback: The unit tests will fail if this patch is applied directly. If you correct the unit tests and update your patch then I would be happy to get it committed. It's a best practice to run "mvn clean install" and allow all unit tests to complete before submitting your patch. Nit: The integer calculation for facility can be reduced to (priority / 8). No subtraction necessary. On the patch naming thing, I agree with Patrick, it's a best practice... JIRA was being problematic last night when I created the patch in FLUME-1470 and didn't bother renaming it once JIRA was back up and I was able to file a ticket Regards, Mike
          Hide
          Bhaskar Karambelkar added a comment -

          Mike,
          I'll fix the Unit test, and upload the patch as per Patrick's recommendation tomorrow.

          Show
          Bhaskar Karambelkar added a comment - Mike, I'll fix the Unit test, and upload the patch as per Patrick's recommendation tomorrow.
          Hide
          Mike Percy added a comment -

          Sounds good!

          Show
          Mike Percy added a comment - Sounds good!
          Hide
          Brock Noland added a comment - - edited

          I assume this is still relevant? It looks like that code has changed a litte.

              if(!isBadEvent){
                pri = Integer.parseInt(prio.toString());
                sev = pri % 8;
                facility = pri / 8;
                formatHeaders();
              }
          
          Show
          Brock Noland added a comment - - edited I assume this is still relevant? It looks like that code has changed a litte. if (!isBadEvent){ pri = Integer .parseInt(prio.toString()); sev = pri % 8; facility = pri / 8; formatHeaders(); }
          Hide
          Mike Percy added a comment -

          FLUME-1470 was committed, this one is now a dup.

          Show
          Mike Percy added a comment - FLUME-1470 was committed, this one is now a dup.

            People

            • Assignee:
              Unassigned
              Reporter:
              Bhaskar Karambelkar
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development