To clarify with the code below,


  1.  The parameter to check_important_sub_metrics is already a sub_metric name
  2.  This line, if sub_metric in self.important_sub_metrics:, should handle the Jmeter case you brought up
  3.  This line, if items[len(items)-1] in self.important_sub_metrics:, should handle special cases  (e.g. %sys) in SAR

Here is the code:


  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 sub_metric in self.important_sub_metrics:
      return True
    if items[len(items)-1] in self.important_sub_metrics:
      return True
    return False

  def plot_cdf(self, graphing_library = 'matplotlib'):
    """
    plot CDF for important sub-metrics
    """
    graphed = False
    for percentile_csv in self.percentiles_files:
      csv_filename = os.path.basename(percentile_csv)
      # The last element is .csv, don't need that in the name of the chart
      column = self.csv_column_map[percentile_csv.replace(".percentiles.", ".")]
      if not self.check_important_sub_metrics(column):
        Continue



From: Shaoshan Liu <shliu@linkedin.com<mailto:shliu@linkedin.com>>
Date: Thursday, March 13, 2014 3:08 PM
To: linkedin/naarad <reply@reply.github.com<mailto:reply@reply.github.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)

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>.