> On Sept. 17, 2014, 4:42 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/container/TaskInstanceExceptionHandler.scala, > > line 41 > > <https://reviews.apache.org/r/25677/diff/4/?file=691315#file691315line41> > > > > Should use new TaskInstanceMetrics here.
Oops. Missed this. Fixed. > On Sept. 17, 2014, 4:42 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala, > > line 63 > > <https://reviews.apache.org/r/25677/diff/4/?file=691314#file691314line63> > > > > Just a nit, but it's legal and a bit more succinct to do: > > > > TaskInstanceExceptionHandler(metrics, config) > > > > Also, seems like we might want exceptionHandler in the constructor, so > > it's injectible for tests. Done. > On Sept. 17, 2014, 4:42 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/container/TaskInstanceExceptionHandler.scala, > > line 73 > > <https://reviews.apache.org/r/25677/diff/4/?file=691315#file691315line73> > > > > Either "Counting exception " + className, or debug("Counting > > exception", exception). Fixed. > On Sept. 17, 2014, 4:42 p.m., Chris Riccomini wrote: > > samza-core/src/main/scala/org/apache/samza/container/TaskInstanceExceptionHandler.scala, > > line 78 > > <https://reviews.apache.org/r/25677/diff/4/?file=691315#file691315line78> > > > > Need a more descriptive counter name here. The metric we'd get out > > right now would just be "com.foo.bar.MyException". > > > > Some sort of prefix like "exception-ignored-%s" format className, would > > be useful, I think. I have added the "exception-ignored-" prefix. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25677/#review53689 ----------------------------------------------------------- On Sept. 17, 2014, 12:42 a.m., David Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25677/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2014, 12:42 a.m.) > > > Review request for samza. > > > Bugs: SAMZA-407 > https://issues.apache.org/jira/browse/SAMZA-407 > > > Repository: samza > > > Description > ------- > > SAMZA-407: Add metric for counting exceptions by type > > > Diffs > ----- > > docs/learn/documentation/versioned/jobs/configuration-table.html > 526ca9ff43534e1cc82f76bbd60c00745803db28 > samza-api/src/main/java/org/apache/samza/metrics/Gauge.java > c37bfbbd6c0beb319e7b2af164f45689bde5b158 > samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala > 21d8903ccb1d80b48e54d72d391acbbeb7832b63 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 3288bf7dfaad81f28352d7c370eb72f984d3544b > samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala > b86fb0dfccbf0c179eb5aa77c4d4a15aa91cdf81 > > samza-core/src/main/scala/org/apache/samza/container/TaskInstanceExceptionHandler.scala > PRE-CREATION > samza-core/src/test/scala/org/apache/samza/container/TestRunLoop.scala > ff425da2e76d9da2f37b0abb2941d6ffaa37b29b > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala > b7a9569de78ba5e794a05e336421588f5e197804 > samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala > c31a74e60ceeefb67bab92a6c39cc39184d6e3e6 > samza-core/src/test/scala/org/apache/samza/metrics/TestMetricsHelper.scala > 0a62bd001c60504cf566f5e59eb442ca8107befd > > Diff: https://reviews.apache.org/r/25677/diff/ > > > Testing > ------- > > Unit tests pass > > > Thanks, > > David Chen > >
