Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JAV-539] & [SCB-9] Implement CallCount and TPS Metrics #451

Merged
merged 12 commits into from Dec 27, 2017

Conversation

zhengyangyong
Copy link

@zhengyangyong zhengyangyong commented Dec 21, 2017

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [JAV-XXX] Fixes bug in ApproximateQuantiles, where you replace JAV-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

JAV-539 & SC-9 Implement CallCount and TPS Metrics

In core module we need add more build-in Metrics,include:
1.consumer total call count
2.provider total call count
3.consumer call tps
4.provider call tps

@zhengyangyong zhengyangyong changed the title JAV-539 & SCB-9 JAV-539 & SCB-9 Implement CallCount and TPS Metrics Dec 21, 2017
@zhengyangyong zhengyangyong changed the title JAV-539 & SCB-9 Implement CallCount and TPS Metrics [JAV-539] & [SCB-9] Implement CallCount and TPS Metrics Dec 21, 2017
@@ -19,7 +19,8 @@

public class InstanceMetric extends ModelMetric {
public InstanceMetric(long waitInQueue, TimerMetric lifeTimeInQueue,
TimerMetric executionTime, TimerMetric consumerLatency, TimerMetric producerLatency) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should know if the invocation is consumer or producer to avoid storing other two metrics here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done,Separate into tow metric named "ConsumerInvocationMetric" and "ProducerInvocationMetric"

@@ -31,5 +32,5 @@ servicecomb:
metrics:
#polling setting
polling:
#Time Unit is MILLISECONDS
#Support Muti Polling Time (MILLISECONDS) like 10000,60000 (10s and 60s)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which polling time does metric use?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both , support muti polling time ,use pollerIndex make a selection
RegistryMetric getRegistryMetric(int pollerIndex)

private final TimerMonitor consumerLatency;

private final TimerMonitor producerLatency;
private final CallMonitor producerCall;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have the prducerXXX and consumerXXX here!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because can't known invocation type before starting process ,all monitor temp keep in this class; when it convert to metric while polling ,will be separate into ConsumerMetric and ProducerMetric.
After Java Chassis improved invocation generation logic, will be refactor this code immediately

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f6332fa on zhengyangyong:JAV-539&SCB-9 into ** on apache:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7511c98 on zhengyangyong:JAV-539&SCB-9 into ** on apache:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 5e76ae1 on zhengyangyong:JAV-539&SCB-9 into ** on apache:master**.

consumerLatency = consumerLatency.merge(metric.getConsumerLatency());
producerLatency = producerLatency.merge(metric.getProducerLatency());
if (metric != null) {
if (metric instanceof ConsumerInvocationMetric) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think of open closed principle


//Counting use System.nano get more precise time
//so we need change unit to millisecond when ouput
public double convertNanosecondToMillisecond(long nanoValue){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why double for milliseconds?

@zhengyangyong
Copy link
Author

Rebase onto master and resolve conflicts

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 87.137% when pulling 7722848 on zhengyangyong:JAV-539&SCB-9 into 7fb4030 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 87.121% when pulling dafe6fb on zhengyangyong:JAV-539&SCB-9 into 7fb4030 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 87.169% when pulling dafe6fb on zhengyangyong:JAV-539&SCB-9 into 7fb4030 on apache:master.

@@ -41,9 +41,14 @@ public InvocationStartProcessingEventListener(RegistryMonitor registryMonitor) {
public void process(Event data) {
InvocationStartProcessingEvent event = (InvocationStartProcessingEvent) data;
InvocationMonitor monitor = registryMonitor.getInvocationMonitor(event.getOperationName());
//TODO:current java chassis unable know invocation type before starting process,so all type WaitInQueue increment(-1) (decrement)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?
event.getInvocationType is not you want?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and in fact, we only trigger InvocationStartProcessingEvent in producer invocation
missed it in consumer invocation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great,it's a important info for me ,thank a lot

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done,Consumer Side Event will be trigger in VertxHttpClient and HighwayClient,https://issues.apache.org/jira/browse/SCB-126

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 87.153% when pulling 2269965 on zhengyangyong:JAV-539&SCB-9 into 7fb4030 on apache:master.

zhengyangyong added 9 commits December 26, 2017 08:48
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
…ion metric

SCB-91 All Monitor can convert to Map with plain-key when polling data

Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
…nymore)

Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
…cond

Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
…temMonitor"

Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
zhengyangyong added 3 commits December 26, 2017 08:48
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
@zhengyangyong
Copy link
Author

resolve conflict

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 87.106% when pulling 2a0f1b8 on zhengyangyong:JAV-539&SCB-9 into a3e96ec on apache:master.

@WillemJiang WillemJiang merged commit fca6aeb into apache:master Dec 27, 2017
@zhengyangyong zhengyangyong deleted the JAV-539&SCB-9 branch March 27, 2018 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants