> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote: > > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/LensMetricsRegistry.java, > > line 40 > > <https://reviews.apache.org/r/31669/diff/1/?file=882716#file882716line40> > > > > I don't think we should start metrics registry in server-api. It should > > be done in metrics service only. Server-API should not force any instances > > on dependent modules. Same goes for the static factory instance. If not > > possible to move at all, make these lazy allocations. > > Amareshwari Sriramadasu wrote: > Sure. To pass registry across different modules and methods, the easiest > way to pass it around is through static instance. Otherwise we need to have > class level holders and setter. And set them from LensService all over. I > will see if static can be avoided by another means.
I think it is fine to have MetricsRegistry as static instance available for all than everyone get the instance of it passed around. And other entities like Drivers and Query rewriters should have access to registry than to metrics service itself. - Amareshwari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31669/#review75153 ----------------------------------------------------------- On March 3, 2015, 7:11 a.m., Amareshwari Sriramadasu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31669/ > ----------------------------------------------------------- > > (Updated March 3, 2015, 7:11 a.m.) > > > Review request for lens, Himanshu Gahlaut, Jaideep dhok, and Rajat Khandelwal. > > > Bugs: LENS-283 > https://issues.apache.org/jira/browse/LENS-283 > > > Repository: lens > > > Description > ------- > > Changes include : > > - Added query level configuration to instrument estimate flow > - Methods instrumented to create gauges. > - Moved metrics related common classes to lens-server-api sothat they can be > used in other modules > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java > ba19125 > lens-cube/src/main/java/org/apache/lens/driver/cube/RewriteUtil.java > 0e9ca25 > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > 1257092 > lens-server-api/pom.xml 8472840 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > ca87fa3 > > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/LensMetricsRegistry.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricGauge.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetrics.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java > f141428 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/DriverSelectorQueryContext.java > 1c04952 > > lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/model/LensResourceMethod.java > PRE-CREATION > lens-server/src/main/java/org/apache/lens/server/metrics/MethodMetrics.java > 6a60421 > > lens-server/src/main/java/org/apache/lens/server/metrics/MethodMetricsFactory.java > f035848 > > lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java > 3f20bfd > > lens-server/src/main/java/org/apache/lens/server/model/LensContainerRequest.java > 307ae10 > > lens-server/src/main/java/org/apache/lens/server/model/LensResourceMethod.java > a3dca46 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 15b9849 > lens-server/src/main/resources/lenssession-default.xml f145358 > > lens-server/src/test/java/org/apache/lens/server/metrics/TestResourceMethodMetrics.java > 9e1c23f > > lens-server/src/test/java/org/apache/lens/server/query/TestResultFormatting.java > a0abb0a > src/site/apt/admin/config.apt 6445fe7 > src/site/apt/admin/session-config.apt d1c36bb > > Diff: https://reviews.apache.org/r/31669/diff/ > > > Testing > ------- > > Pending unit tests. > > Will update full test run once done. > > > Thanks, > > Amareshwari Sriramadasu > >
