You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Amareshwari Sriramadasu <am...@apache.org> on 2015/03/03 08:11:44 UTC

Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/
-----------------------------------------------------------

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


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Jaideep dhok <ja...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review75669
-----------------------------------------------------------

Ship it!


Ship It!

- Jaideep dhok


On March 9, 2015, 6:20 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31669/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 6:20 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-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java PRE-CREATION 
>   lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java df81054 
>   lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java 98aeb25 
>   lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java 774f43f 
>   lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java 1257092 
>   lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java 68bd19b 
>   lens-server-api/pom.xml 8472840 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java ff25fa2 
>   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-api/src/test/java/org/apache/lens/server/api/driver/MockQueryContext.java 175eab3 
>   lens-server-api/src/test/java/org/apache/lens/server/api/metrics/TestMethodMetricsFactory.java PRE-CREATION 
>   lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java PRE-CREATION 
>   lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.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 2f44938 
>   lens-server/src/main/resources/lenssession-default.xml f145358 
>   lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java a695b5d 
>   lens-server/src/test/java/org/apache/lens/server/api/metrics/LensMetricsUtil.java PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/metrics/TestResourceMethodMetrics.java 9e1c23f 
>   lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java ea991b7 
>   src/site/apt/admin/session-config.apt d1c36bb 
> 
> Diff: https://reviews.apache.org/r/31669/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for all the new code introduced.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.188s]
> [INFO] Lens .............................................. SUCCESS [1.738s]
> [INFO] Lens API .......................................... SUCCESS [5.248s]
> [INFO] Lens API for server and extensions ................ SUCCESS [7.774s]
> [INFO] Lens Cube ......................................... SUCCESS [8:52.675s]
> [INFO] Lens DB storage ................................... SUCCESS [34.426s]
> [INFO] Lens Query Library ................................ SUCCESS [19.669s]
> [INFO] Lens Hive Driver .................................. SUCCESS [3:56.653s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [25.594s]
> [INFO] Lens Server ....................................... SUCCESS [4:30.527s]
> [INFO] Lens client ....................................... SUCCESS [25.469s]
> [INFO] Lens CLI .......................................... SUCCESS [2:08.664s]
> [INFO] Lens Examples ..................................... SUCCESS [1.274s]
> [INFO] Lens Distribution ................................. SUCCESS [11.660s]
> [INFO] Lens ML Lib ....................................... SUCCESS [52.988s]
> [INFO] Lens Regression ................................... SUCCESS [0.486s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 22:37.988s
> [INFO] Finished at: Mon Mar 09 06:19:10 UTC 2015
> [INFO] Final Memory: 111M/1251M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/
-----------------------------------------------------------

(Updated March 9, 2015, 6:20 a.m.)


Review request for lens, Himanshu Gahlaut, Jaideep dhok, and Rajat Khandelwal.


Changes
-------

Fixed clear registry not be public and made only available for tests


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 (updated)
-----

  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-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java PRE-CREATION 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java df81054 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java 98aeb25 
  lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java 774f43f 
  lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java 1257092 
  lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java 68bd19b 
  lens-server-api/pom.xml 8472840 
  lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java ff25fa2 
  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-api/src/test/java/org/apache/lens/server/api/driver/MockQueryContext.java 175eab3 
  lens-server-api/src/test/java/org/apache/lens/server/api/metrics/TestMethodMetricsFactory.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.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 2f44938 
  lens-server/src/main/resources/lenssession-default.xml f145358 
  lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java a695b5d 
  lens-server/src/test/java/org/apache/lens/server/api/metrics/LensMetricsUtil.java PRE-CREATION 
  lens-server/src/test/java/org/apache/lens/server/metrics/TestResourceMethodMetrics.java 9e1c23f 
  lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java ea991b7 
  src/site/apt/admin/session-config.apt d1c36bb 

Diff: https://reviews.apache.org/r/31669/diff/


Testing (updated)
-------

Unit tests added for all the new code introduced.

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Lens Checkstyle Rules ............................. SUCCESS [2.188s]
[INFO] Lens .............................................. SUCCESS [1.738s]
[INFO] Lens API .......................................... SUCCESS [5.248s]
[INFO] Lens API for server and extensions ................ SUCCESS [7.774s]
[INFO] Lens Cube ......................................... SUCCESS [8:52.675s]
[INFO] Lens DB storage ................................... SUCCESS [34.426s]
[INFO] Lens Query Library ................................ SUCCESS [19.669s]
[INFO] Lens Hive Driver .................................. SUCCESS [3:56.653s]
[INFO] Lens Driver for JDBC .............................. SUCCESS [25.594s]
[INFO] Lens Server ....................................... SUCCESS [4:30.527s]
[INFO] Lens client ....................................... SUCCESS [25.469s]
[INFO] Lens CLI .......................................... SUCCESS [2:08.664s]
[INFO] Lens Examples ..................................... SUCCESS [1.274s]
[INFO] Lens Distribution ................................. SUCCESS [11.660s]
[INFO] Lens ML Lib ....................................... SUCCESS [52.988s]
[INFO] Lens Regression ................................... SUCCESS [0.486s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 22:37.988s
[INFO] Finished at: Mon Mar 09 06:19:10 UTC 2015
[INFO] Final Memory: 111M/1251M
[INFO] ------------------------------------------------------------------------


Thanks,

Amareshwari Sriramadasu


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/
-----------------------------------------------------------

(Updated March 6, 2015, 12:41 p.m.)


Review request for lens, Himanshu Gahlaut, Jaideep dhok, and Rajat Khandelwal.


Changes
-------

Fixed test failures


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 (updated)
-----

  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-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java PRE-CREATION 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java df81054 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java 98aeb25 
  lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java 774f43f 
  lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java 1257092 
  lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java 68bd19b 
  lens-server-api/pom.xml 8472840 
  lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java ff25fa2 
  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-api/src/test/java/org/apache/lens/server/api/driver/MockQueryContext.java 175eab3 
  lens-server-api/src/test/java/org/apache/lens/server/api/metrics/TestMethodMetricsFactory.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.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 2f44938 
  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/TestQueryService.java ea991b7 
  src/site/apt/admin/session-config.apt d1c36bb 

Diff: https://reviews.apache.org/r/31669/diff/


Testing (updated)
-------

Unit tests added for all the new code introduced.

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Lens Checkstyle Rules ............................. SUCCESS [2.051s]
[INFO] Lens .............................................. SUCCESS [1.699s]
[INFO] Lens API .......................................... SUCCESS [6.288s]
[INFO] Lens API for server and extensions ................ SUCCESS [5.945s]
[INFO] Lens Cube ......................................... SUCCESS [6:24.162s]
[INFO] Lens DB storage ................................... SUCCESS [9.850s]
[INFO] Lens Query Library ................................ SUCCESS [4.599s]
[INFO] Lens Hive Driver .................................. SUCCESS [2:36.144s]
[INFO] Lens Driver for JDBC .............................. SUCCESS [25.220s]
[INFO] Lens Server ....................................... SUCCESS [4:32.364s]
[INFO] Lens client ....................................... SUCCESS [20.868s]
[INFO] Lens CLI .......................................... SUCCESS [1:51.512s]
[INFO] Lens Examples ..................................... SUCCESS [0.887s]
[INFO] Lens Distribution ................................. SUCCESS [9.053s]
[INFO] Lens ML Lib ....................................... SUCCESS [47.731s]
[INFO] Lens Regression ................................... SUCCESS [0.462s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 17:39.804s
[INFO] Finished at: Fri Mar 06 12:33:14 UTC 2015
[INFO] Final Memory: 99M/1298M
[INFO] ------------------------------------------------------------------------


Thanks,

Amareshwari Sriramadasu


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Jaideep dhok <ja...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review75474
-----------------------------------------------------------

Ship it!


Ship It!

- Jaideep dhok


On March 6, 2015, 6:04 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31669/
> -----------------------------------------------------------
> 
> (Updated March 6, 2015, 6:04 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-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java PRE-CREATION 
>   lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java df81054 
>   lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java 98aeb25 
>   lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java 774f43f 
>   lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java 1257092 
>   lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java 68bd19b 
>   lens-server-api/pom.xml 8472840 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java ff25fa2 
>   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-api/src/test/java/org/apache/lens/server/api/driver/MockQueryContext.java 175eab3 
>   lens-server-api/src/test/java/org/apache/lens/server/api/metrics/TestMethodMetricsFactory.java PRE-CREATION 
>   lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java PRE-CREATION 
>   lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.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 2f44938 
>   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/TestQueryService.java ea991b7 
>   src/site/apt/admin/session-config.apt d1c36bb 
> 
> Diff: https://reviews.apache.org/r/31669/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for all the new code introduced.
> 
> Will update full test run once done.
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/
-----------------------------------------------------------

(Updated March 6, 2015, 6:04 a.m.)


Review request for lens, Himanshu Gahlaut, Jaideep dhok, and Rajat Khandelwal.


Changes
-------

Added unit tests and incorporated review comments


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 (updated)
-----

  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-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java PRE-CREATION 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java df81054 
  lens-cube/src/test/java/org/apache/lens/driver/cube/TestRewriting.java 98aeb25 
  lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java 774f43f 
  lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java 1257092 
  lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java 68bd19b 
  lens-server-api/pom.xml 8472840 
  lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java ff25fa2 
  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-api/src/test/java/org/apache/lens/server/api/driver/MockQueryContext.java 175eab3 
  lens-server-api/src/test/java/org/apache/lens/server/api/metrics/TestMethodMetricsFactory.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java PRE-CREATION 
  lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.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 2f44938 
  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/TestQueryService.java ea991b7 
  src/site/apt/admin/session-config.apt d1c36bb 

Diff: https://reviews.apache.org/r/31669/diff/


Testing (updated)
-------

Unit tests added for all the new code introduced.

Will update full test run once done.


Thanks,

Amareshwari Sriramadasu


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review74915
-----------------------------------------------------------


Ran examples and here are all the gauges for one of the query :

    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-ALL_CUBE_REWRITES" : {
      "value" : 257
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-ALL_DRIVER_ESTIMATES" : {
      "value" : 274
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-DRIVER_SELECTION" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-driverEstimate" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.AggregateResolver-ITER-8" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.AliasReplacer-ITER-2" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.CandidateTableResolver-ITER-4" : {
      "value" : 2
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.CandidateTableResolver-ITER-7" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.ColumnResolver-ITER-1" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.DenormalizationResolver-ITER-14" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.DenormalizationResolver-ITER-3" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.ExpressionResolver-ITER-0" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.GroupbyResolver-ITER-9" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.JoinResolver-ITER-5" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.LeastPartitionResolver-ITER-15" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.LightestDimensionResolver-ITER-17" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.LightestFactResolver-ITER-12" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.LightestFactResolver-ITER-16" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-10" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-11" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-13" : {
      "value" : 51
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.cube.parse.TimerangeResolver-ITER-6" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.driver.cube.RewriteUtil-rewriteQuery" : {
      "value" : 112
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-HiveDriver-org.apache.lens.driver.cube.RewriteUtil-rewriteQuery-toHQL" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-columnar-sql-rewrite" : {
      "value" : 272
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-driverEstimate" : {
      "value" : 274
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-jdbc-prepare-statement" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.AggregateResolver-ITER-8" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.AliasReplacer-ITER-2" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.CandidateTableResolver-ITER-4" : {
      "value" : 2
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.CandidateTableResolver-ITER-7" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.ColumnResolver-ITER-1" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.DenormalizationResolver-ITER-14" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.DenormalizationResolver-ITER-3" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.ExpressionResolver-ITER-0" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.GroupbyResolver-ITER-9" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.JoinResolver-ITER-5" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.LeastPartitionResolver-ITER-15" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.LightestDimensionResolver-ITER-17" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.LightestFactResolver-ITER-12" : {
      "value" : 1
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.LightestFactResolver-ITER-16" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-10" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-11" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.StorageTableResolver-ITER-13" : {
      "value" : 16
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.cube.parse.TimerangeResolver-ITER-6" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.driver.cube.RewriteUtil-rewriteQuery" : {
      "value" : 71
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-org.apache.lens.driver.cube.RewriteUtil-rewriteQuery-toHQL" : {
      "value" : 0
    },
    "lens.MethodMetricGauge.46f0ab75-1586-41d8-a102-37c6cb1167c3-JDBCDriver-validate-thru-prepare" : {
      "value" : 273
    },

- Amareshwari Sriramadasu


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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> On March 3, 2015, 3:39 p.m., Himanshu Gahlaut wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java, line 43
> > <https://reviews.apache.org/r/31669/diff/1/?file=882711#file882711line43>
> >
> >     Since we are adding this logger for the first time, will it be fine to replace this logger with Slf4j so that we can use parametrized logging and also replace the underlying implementation with logback when we have time ?
> >     
> >     http://slf4j.org/faq.html#logging_performance
> >     http://logback.qos.ch/reasonsToSwitch.html

Happened to remove this logger. No changes required.


- Amareshwari


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review74959
-----------------------------------------------------------


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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Himanshu Gahlaut <ga...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review74959
-----------------------------------------------------------



lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java
<https://reviews.apache.org/r/31669/#comment121837>

    Since we are adding this logger for the first time, will it be fine to replace this logger with Slf4j so that we can use parametrized logging and also replace the underlying implementation with logback when we have time ?
    
    http://slf4j.org/faq.html#logging_performance
    http://logback.qos.ch/reasonsToSwitch.html



lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java
<https://reviews.apache.org/r/31669/#comment121838>

    Will adding a comment here which explains the need of ITER_STR string and also explains how it helps in overall logic help in understanding the code faster ?



lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java
<https://reviews.apache.org/r/31669/#comment121844>

    If we aim to create methods which can never return null, we can avoid null checks like this in code.



lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java
<https://reviews.apache.org/r/31669/#comment121843>

    


- Himanshu Gahlaut


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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> 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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java, line 831
> > <https://reviews.apache.org/r/31669/diff/1/?file=882715#file882715line831>
> >
> >     Why is this required?

This is to pass unique id for query across the code flow.


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java, line 836
> > <https://reviews.apache.org/r/31669/diff/1/?file=882715#file882715line836>
> >
> >     Which stack is this?

When each query looked at by driver, the metric needs to be different for each driver. This name capture the stack from which driver the code reached there. Will rename to driver stack name, if we dont anticipate any other stacks.


> 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.

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.


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetrics.java, line 30
> > <https://reviews.apache.org/r/31669/diff/1/?file=882718#file882718line30>
> >
> >     This and the above class have same Javadoc What is the difference?

Will update doc


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java, line 53
> > <https://reviews.apache.org/r/31669/diff/1/?file=882719#file882719line53>
> >
> >     I wonder why we need more singletons. Do we really need more static variables? Is it not possible to contain them in some service?

Will check


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java, line 101
> > <https://reviews.apache.org/r/31669/diff/1/?file=882719#file882719line101>
> >
> >     Null check repeated twice?

Looks like. This was moved code from lens-server to lens-server-api.


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java, line 149
> > <https://reviews.apache.org/r/31669/diff/1/?file=882719#file882719line149>
> >
> >     Are we appending request values to the metric name?

Again, moved code from lens-server to lens-server-api.

To answer your question, yes. For some methods which use multipurpose param, query() is used for all ESTIMATE/EXPLAIN/EXECUTE/EXECUTE_WITH_TIMEOUT


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java, line 115
> > <https://reviews.apache.org/r/31669/diff/1/?file=882720#file882720line115>
> >
> >     Can we use Query handle here instead of one more UUID?

No. Query handle is present only for queries accepted for execution. Others like ESTIMATE and EXPLAIN, do not have a handle.


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java, line 36
> > <https://reviews.apache.org/r/31669/diff/1/?file=882722#file882722line36>
> >
> >     This comment is too abstract. I couldn't understand why this class is needed. Which server code is this useful for? What do you mean by model object? Is this model from the MVC pattern?

Again, moved code from lens-server to lens-server-api.

Not doing any changes.


> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java, line 71
> > <https://reviews.apache.org/r/31669/diff/1/?file=882722#file882722line71>
> >
> >     Why IllegalArgumentException? Please include error message. Also, will this exception ovverride any HTTP errors?

Again, moved code from lens-server to lens-server-api.

Not doing any changes.


- 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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Jaideep dhok <ja...@inmobi.com>.

> On March 4, 2015, 7:07 a.m., Jaideep dhok wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java, line 36
> > <https://reviews.apache.org/r/31669/diff/1/?file=882722#file882722line36>
> >
> >     This comment is too abstract. I couldn't understand why this class is needed. Which server code is this useful for? What do you mean by model object? Is this model from the MVC pattern?
> 
> Amareshwari Sriramadasu wrote:
>     Again, moved code from lens-server to lens-server-api.
>     
>     Not doing any changes.

Let's do it now since we haven't committed this. Otherwise it will stay there forever.


- Jaideep


-----------------------------------------------------------
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
> 
>


Re: Review Request 31669: LENS-283 : Time gauge metrics for instrumenting estimate flow

Posted by Jaideep dhok <ja...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31669/#review75153
-----------------------------------------------------------



lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
<https://reviews.apache.org/r/31669/#comment122111>

    Why is this required?



lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java
<https://reviews.apache.org/r/31669/#comment122112>

    Which stack is this?



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/LensMetricsRegistry.java
<https://reviews.apache.org/r/31669/#comment122117>

    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.



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetrics.java
<https://reviews.apache.org/r/31669/#comment122104>

    This and the above class have same Javadoc What is the difference?



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetrics.java
<https://reviews.apache.org/r/31669/#comment122105>

    This and the above class have same Javadoc What is the difference?



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java
<https://reviews.apache.org/r/31669/#comment122114>

    I wonder why we need more singletons. Do we really need more static variables? Is it not possible to contain them in some service?



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java
<https://reviews.apache.org/r/31669/#comment122108>

    Null check repeated twice?



lens-server-api/src/main/java/org/apache/lens/server/api/metrics/MethodMetricsFactory.java
<https://reviews.apache.org/r/31669/#comment122109>

    Are we appending request values to the metric name?



lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java
<https://reviews.apache.org/r/31669/#comment122113>

    Can we use Query handle here instead of one more UUID?



lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java
<https://reviews.apache.org/r/31669/#comment122115>

    This comment is too abstract. I couldn't understand why this class is needed. Which server code is this useful for? What do you mean by model object? Is this model from the MVC pattern?



lens-server-api/src/main/java/org/apache/lens/server/model/LensContainerRequest.java
<https://reviews.apache.org/r/31669/#comment122116>

    Why IllegalArgumentException? Please include error message. Also, will this exception ovverride any HTTP errors?



lens-server/src/test/java/org/apache/lens/server/query/TestResultFormatting.java
<https://reviews.apache.org/r/31669/#comment122118>

    This fix has gone in already. Diff might need rebase.


- Jaideep dhok


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
> 
>