Ah thanks, I have updated the RB to make it clear.

Also, I have put all test results here:

http://shliu-ld1/naarad/155

Note that Jmeter does not support diff yet and Sharad is going to add that support.


Thanks!

Shaoshan

From: Ritesh Maheshwari <notifications@github.com<mailto:notifications@github.com>>
Reply-To: linkedin/naarad <reply@reply.github.com<mailto:reply@reply.github.com>>
Date: Thursday, March 13, 2014 9:04 AM
To: linkedin/naarad <naarad@noreply.github.com<mailto:naarad@noreply.github.com>>
Cc: Shaoshan Liu <shliu@linkedin.com<mailto:shliu@linkedin.com>>
Subject: Re: [naarad] Extend Diff to support URL based CDF csv file paths, also Fix issue 149: cdf not plotted for sar important metrics (#155)


In src/naarad/metrics/metric.py:

> @@ -284,6 +284,15 @@ def plot_timeseries(self, graphing_library = 'matplotlib'):
>        if graphed:
>          self.plot_files.append(div_file)
>      return True
> +
> +  def check_important_sub_metrics(self, sub_metric):
> +    """
> +    check whether the given sub metric is in important_sub_metrics list
> +    """
> +    items = sub_metric.split('.')
> +    if items[len(items)-1] not in self.important_sub_metrics:


Yes, sorry I didn't make it clear! The problem I see with current approach is that it checks for only sub_metric name in self.important_sub_metrics. This is checking only the last suffix after splitting by '.'. This line:

if items[len(items)-1] in self.important_sub_metrics:


This misses the JMeter like cases where self.important_sub_metrics set/tuple contains the transaction/device name also (Overall_Summary) along with the sub_metric name (ResponseTime). See the definition in naarad_imports:

important_sub_metrics_import = {
    'GC': ('GCPause', 'used'),
    'LINKEDINANDROIDRUM': ('launch_time', 'nus_update_time'),
    'SAR-cpuusage': ('%sys', '%usr'),
    'SAR-device': ('%util', 'await'),
    'JMETER': ('Overall_Summary.ResponseTime', 'Overall_Summary.DataThroughput', 'Overall_Summary.qps')
}


—
Reply to this email directly or view it on GitHub<https://github.com/linkedin/naarad/pull/155/files#r10574326>.