Uploaded image for project: 'Samza'
  1. Samza
  2. SAMZA-407

Add metrics for counting exceptions by type

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.0
    • Component/s: container
    • Labels:
      None

      Description

      Currently, the container does not expose any metrics for tracking the number of exceptions happening in the process() method.

      We can catch all these exceptions and maintain a count per exception type (before rethrowing the exception). These metrics can then be exposed for further monitoring

      1. SAMZA-407.3.patch
        23 kB
        David Chen
      2. SAMZA-407.2.patch
        21 kB
        David Chen
      3. SAMZA-407.1.patch
        21 kB
        David Chen
      4. SAMZA-407.0.patch
        21 kB
        David Chen

        Activity

        Hide
        criccomini Chris Riccomini added a comment -

        +1 Thanks, merged and committed!

        Show
        criccomini Chris Riccomini added a comment - +1 Thanks, merged and committed!
        Hide
        davidzchen David Chen added a comment -

        Attaching new patch and RB addressing new review comments.

        Show
        davidzchen David Chen added a comment - Attaching new patch and RB addressing new review comments.
        Hide
        criccomini Chris Riccomini added a comment -

        Looking good. I posted a few minor nits on the RB. One more pass, and I think we should be good to commit.

        Show
        criccomini Chris Riccomini added a comment - Looking good. I posted a few minor nits on the RB. One more pass, and I think we should be good to commit.
        Hide
        davidzchen David Chen added a comment -

        Attaching a new patch rebased on master with merge conflicts resolved.

        Show
        davidzchen David Chen added a comment - Attaching a new patch rebased on master with merge conflicts resolved.
        Hide
        davidzchen David Chen added a comment -
        Show
        davidzchen David Chen added a comment - Attaching patch. RB: https://reviews.apache.org/r/25677/
        Hide
        criccomini Chris Riccomini added a comment -

        If we add this feature, we should definitely make it configurable, and off by default. Catching exceptions thrown in process() is dangerous, because it means we can drop output.

        In my mind, this is a very similar feature to SAMZA-59. There are cases where developers would rather drop messages than fail outright when an exception is thrown in the process() method. To accommodate this, we can add a config to allow this, and track the failures in a metrics counter, based on exception type.

        Show
        criccomini Chris Riccomini added a comment - If we add this feature, we should definitely make it configurable, and off by default. Catching exceptions thrown in process() is dangerous, because it means we can drop output. In my mind, this is a very similar feature to SAMZA-59 . There are cases where developers would rather drop messages than fail outright when an exception is thrown in the process() method. To accommodate this, we can add a config to allow this, and track the failures in a metrics counter, based on exception type.

          People

          • Assignee:
            davidzchen David Chen
            Reporter:
            cpsoman Chinmay Soman
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development