[GitHub] [incubator-iotdb] jixuan1989 commented on pull request #1303: Add Metrics Service based on Micrometer.

[GitBox <gi...@apache.org>]

jixuan1989 commented on pull request #1303:
URL: https://github.com/apache/incubator-iotdb/pull/1303#issuecomment-641168499


   some suggestions:
   
   1. use `latency` rather than `duration`;
   
   2. use InsertPlan to write data rather than use SQL;
   
   3. I think it is better to keep the server module as lightweight as possible;
     - add a switch to enable/disable collect metrics (please read the 4th suggestion. If the performance impaction can be ignored, we do not need this switch);
     - add a switch to enable/disable writing metrics into IoTDB 
     - move the HTTP service and Prometheus into a new module, e.g., Prometheus-registry. Then the dependency of micrometer-registry-prometheus can be moved out of the server module. 
   (1) To implement it, you may have to extract some implement in MicrometerServerService.java as an interface, and just retain the IoTDBRegistry as the default implementation; 
   (2) In `prometheus-registry` module, you can add the dependency and reimplement the interface. 
   (3) suppose we have a new module `prometheus-registry`, do not let the `server` module depends on it. If so, we may need using the Java reflect strategy to load the new implementation in (2).
   
   4. better to attach a report that the performance impaction after adding this feature


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org