Uploaded image for project: 'Flume'
  1. Flume
  2. FLUME-2004

Need to capture metrics on the Flume exec source such as events received, rejected, etc.

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.4.0
    • Component/s: Channel, Sinks+Sources
    • Labels:
      None

      Description

      To give you a background, we have configured our flume agents with "exec" source to tail some logs and stream it to a set of collectors. We have monitoring setup that polls "/metrics" on these agents and verifies a few metrics like channel size, event put count etc. The "/metrics" response returns data about the 'channel' and 'sink' (we have an avro sink) components, however there is no data returned about the "source" (exec) component. We are interested in monitoring the number of events received, rejected, etc.

      I couldn't find any documentation around this; I checked the flume source and saw the metrics are not getting captured in the code as well for 'exec' source.

      Please improve the metrics captured for the 'exec' source for flume.

      1. FLUME-2004.patch
        4 kB
        Mike Percy
      2. FLUME-2004.patch
        8 kB
        Venkatesh Sivasubramanian

        Activity

        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Hi,

        I was the person who had this request. I have created a patch for the same; pls. find the same attached.

        The changes are made to ExecSource class. I have made used of SourceCounter in place of CounterGroup and have incremented the metrics for events received and accepted in the appropriate place.

        Please review and let me know if you have any comments/feedback. Thank you for all the help!

        Regards,
        Venkatesh

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Hi, I was the person who had this request. I have created a patch for the same; pls. find the same attached. The changes are made to ExecSource class. I have made used of SourceCounter in place of CounterGroup and have incremented the metrics for events received and accepted in the appropriate place. Please review and let me know if you have any comments/feedback. Thank you for all the help! Regards, Venkatesh
        Hide
        rjustice Robert Justice added a comment -

        Thanks Venkatesh! We'll review and let you know if we have any further suggestions.

        Show
        rjustice Robert Justice added a comment - Thanks Venkatesh! We'll review and let you know if we have any further suggestions.
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Thanks Robert!

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Thanks Robert!
        Hide
        mpercy Mike Percy added a comment -

        Looking at this. BTW Venkatesh Sivasubramanian, I renamed your attachment filename.

        Show
        mpercy Mike Percy added a comment - Looking at this. BTW Venkatesh Sivasubramanian , I renamed your attachment filename.
        Hide
        mpercy Mike Percy added a comment -

        Hi Venkatesh Sivasubramanian,
        Thanks for the patch! The changes look pretty good. I have one suggestion and a couple of requests for you:

        1. In ExecSource on line 298 and 304 we are incrementing the same counter via addToEventAcceptedCount(). The call on line 298 should be removed.
        2. What kind of testing has been done on this so far? Can we add a unit test for this functionality?
        3. After updating the patch, please also post your change to review board and post a link to your review here on this JIRA. Review board makes it easier for committers to review your changes and provide feedback.

        I didn't test this yet but other than that, the changes look good to me.

        Regards,
        Mike

        Show
        mpercy Mike Percy added a comment - Hi Venkatesh Sivasubramanian , Thanks for the patch! The changes look pretty good. I have one suggestion and a couple of requests for you: 1. In ExecSource on line 298 and 304 we are incrementing the same counter via addToEventAcceptedCount() . The call on line 298 should be removed. 2. What kind of testing has been done on this so far? Can we add a unit test for this functionality? 3. After updating the patch, please also post your change to review board and post a link to your review here on this JIRA. Review board makes it easier for committers to review your changes and provide feedback. I didn't test this yet but other than that, the changes look good to me. Regards, Mike
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Hi Mike Percy,

        Thanks for reviewing!!

        1. The line 298 gets invoked only when the buffer size (batchsize) is reached and in that case, it calls the processEventBatch() and clears the events list. And the line 304 is for all spillovers when the stream has stopped without reaching the batch size. That is the reason, I have added in both the places. Pls. feel free to let me know if my understanding is incorrect.
        2. On the testing front, I did local testing by adding an interceptor that selectively returned null based on some pattern in the message/event. Sure, let me try to add a unit test and submit the patch to review board.

        Best regards,
        Venkatesh

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Hi Mike Percy , Thanks for reviewing!! 1. The line 298 gets invoked only when the buffer size (batchsize) is reached and in that case, it calls the processEventBatch() and clears the events list. And the line 304 is for all spillovers when the stream has stopped without reaching the batch size. That is the reason, I have added in both the places. Pls. feel free to let me know if my understanding is incorrect. 2. On the testing front, I did local testing by adding an interceptor that selectively returned null based on some pattern in the message/event. Sure, let me try to add a unit test and submit the patch to review board. Best regards, Venkatesh
        Hide
        mpercy Mike Percy added a comment -

        Venkatesh Sivasubramanian: you are right... I misread that part.

        +1 on this patch. Let me know if you have time to add a unit test, otherwise because this is metrics-related I think this is good to go.

        Show
        mpercy Mike Percy added a comment - Venkatesh Sivasubramanian : you are right... I misread that part. +1 on this patch. Let me know if you have time to add a unit test, otherwise because this is metrics-related I think this is good to go.
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Cool, thanks Mike Percy! Yes, I have added a Unit Test for this in TestExecSource. I have attached the updated Patch here. Pls. take a look and let me know if there are any feedbacks/suggestions.

        I tried to submit the same to the review board, having some difficulties with the tool. Pls. feel free to let me know if you would like me to still add to the same, I will do it in the morning (after going through the documentation).

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Cool, thanks Mike Percy ! Yes, I have added a Unit Test for this in TestExecSource. I have attached the updated Patch here. Pls. take a look and let me know if there are any feedbacks/suggestions. I tried to submit the same to the review board, having some difficulties with the tool. Pls. feel free to let me know if you would like me to still add to the same, I will do it in the morning (after going through the documentation).
        Hide
        mpercy Mike Percy added a comment -

        Venkatesh this looks great! I already reviewed / tested this, we can use review board next time.

        +1

        Show
        mpercy Mike Percy added a comment - Venkatesh this looks great! I already reviewed / tested this, we can use review board next time. +1
        Hide
        mpercy Mike Percy added a comment -

        Pushed to trunk & flume-1.4 branches. Thanks for your contribution, Venkatesh!

        Show
        mpercy Mike Percy added a comment - Pushed to trunk & flume-1.4 branches. Thanks for your contribution, Venkatesh!
        Hide
        mpercy Mike Percy added a comment -

        Venkatesh Sivasubramanian: I've just added you as a contributor in JIRA, so now you can assign yourself FLUME JIRAs when you want to work on something.

        Regards,
        Mike

        Show
        mpercy Mike Percy added a comment - Venkatesh Sivasubramanian : I've just added you as a contributor in JIRA, so now you can assign yourself FLUME JIRAs when you want to work on something. Regards, Mike
        Hide
        hudson Hudson added a comment -

        Integrated in flume-trunk #395 (See https://builds.apache.org/job/flume-trunk/395/)
        FLUME-2004. Capture JMX metrics for Exec source. (Revision 41ca44be52e65845b359307f637282d794a345e4)

        Result = FAILURE
        mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=41ca44be52e65845b359307f637282d794a345e4
        Files :

        • flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java
        • flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java
        Show
        hudson Hudson added a comment - Integrated in flume-trunk #395 (See https://builds.apache.org/job/flume-trunk/395/ ) FLUME-2004 . Capture JMX metrics for Exec source. (Revision 41ca44be52e65845b359307f637282d794a345e4) Result = FAILURE mpercy : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=41ca44be52e65845b359307f637282d794a345e4 Files : flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Cool! Thank a lot Mike Percy!!

        Let me check on the Build Failure and get back.

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Cool! Thank a lot Mike Percy !! Let me check on the Build Failure and get back.
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Looks like the build is failing for the same problem even before this commit. Also verified the new unit test case passed without issues.

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Looks like the build is failing for the same problem even before this commit. Also verified the new unit test case passed without issues.
        Hide
        mpercy Mike Percy added a comment -

        Hi Venkatesh, no worries, please disregard the Jenkins failure for now. I think ASF Jenkins is misconfigured somehow. I looked into it a bit last night but so far have not gotten to the bottom of the issue.

        Show
        mpercy Mike Percy added a comment - Hi Venkatesh, no worries, please disregard the Jenkins failure for now. I think ASF Jenkins is misconfigured somehow. I looked into it a bit last night but so far have not gotten to the bottom of the issue.
        Hide
        venkyz Venkatesh Sivasubramanian added a comment -

        Great, Thanks Mike!

        Show
        venkyz Venkatesh Sivasubramanian added a comment - Great, Thanks Mike!

          People

          • Assignee:
            venkyz Venkatesh Sivasubramanian
            Reporter:
            rjustice Robert Justice
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development