Repository: ambari
Updated Branches:
  refs/heads/trunk 6dfa24550 -> 21442eb8f


AMBARI-11041. API does not work for same service component metric with multiple 
function is requested. (swagle)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/21442eb8
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/21442eb8
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/21442eb8

Branch: refs/heads/trunk
Commit: 21442eb8fd765626e1a0284c37a15ec8a5307fb8
Parents: 6dfa245
Author: Siddharth Wagle <swa...@hortonworks.com>
Authored: Sun May 10 13:41:02 2015 -0700
Committer: Siddharth Wagle <swa...@hortonworks.com>
Committed: Mon May 11 17:49:35 2015 -0700

----------------------------------------------------------------------
 .../metrics/MetricsPropertyProvider.java        | 14 ++--
 .../metrics/MetricsPropertyProviderProxy.java   | 14 ++--
 .../timeline/AMSComponentPropertyProvider.java  | 77 +++++++++++---------
 .../metrics/timeline/AMSPropertyProvider.java   | 57 ++++++++-------
 .../StackDefinedPropertyProviderTest.java       | 51 +++++++++++++
 .../ganglia/GangliaPropertyProviderTest.java    |  4 -
 .../timeline/AMSPropertyProviderTest.java       |  2 +-
 .../HDP/2.0.5/services/HBASE/metrics.json       | 16 ++++
 8 files changed, 157 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java
index 569bb17..9fa9ca4 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java
@@ -58,13 +58,13 @@ public abstract class MetricsPropertyProvider extends 
AbstractPropertyProvider {
     new MetricsPaddingMethod(MetricsPaddingMethod.PADDING_STRATEGY.ZEROS);
 
   protected MetricsPropertyProvider(Map<String, Map<String,
-    PropertyInfo>> componentPropertyInfoMap,
-                                 StreamProvider streamProvider,
-                                 ComponentSSLConfiguration configuration,
-                                 MetricHostProvider hostProvider,
-                                 String clusterNamePropertyId,
-                                 String hostNamePropertyId,
-                                 String componentNamePropertyId) {
+       PropertyInfo>> componentPropertyInfoMap,
+       StreamProvider streamProvider,
+       ComponentSSLConfiguration configuration,
+       MetricHostProvider hostProvider,
+       String clusterNamePropertyId,
+       String hostNamePropertyId,
+       String componentNamePropertyId) {
 
     super(componentPropertyInfoMap);
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java
index 7fa9c5e..57a8e7d 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java
@@ -83,15 +83,19 @@ public class MetricsPropertyProviderProxy extends 
AbstractPropertyProvider {
     }
   }
 
+  /**
+   * Allow delegates to support special properties not stack defined.
+   */
   @Override
   public Set<String> checkPropertyIds(Set<String> propertyIds) {
+    MetricsService metricsService = 
metricsServiceProvider.getMetricsServiceType();
     Set<String> checkedPropertyIds = super.checkPropertyIds(propertyIds);
-    for (String propertyId : checkedPropertyIds) {
-      if (propertyId.startsWith(ZERO_PADDING_PARAM)) {
-        checkedPropertyIds.remove(propertyId);
-      }
+
+    if (metricsService != null && metricsService.equals(TIMELINE_METRICS)) {
+      return amsPropertyProvider.checkPropertyIds(checkedPropertyIds);
+    } else {
+      return checkedPropertyIds;
     }
-    return checkedPropertyIds;
   }
 
   private void createHostPropertyProviders(Map<String, Map<String, 
PropertyInfo>> componentPropertyInfoMap,

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSComponentPropertyProvider.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSComponentPropertyProvider.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSComponentPropertyProvider.java
index 3e32326..ded96cf 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSComponentPropertyProvider.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSComponentPropertyProvider.java
@@ -27,6 +27,7 @@ import 
org.apache.ambari.server.controller.utilities.PredicateHelper;
 import org.apache.ambari.server.controller.utilities.StreamProvider;
 import org.apache.commons.lang.StringUtils;
 
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -40,49 +41,57 @@ public class AMSComponentPropertyProvider extends 
AMSPropertyProvider {
                                           String clusterNamePropertyId,
                                           String componentNamePropertyId) {
 
-    super(componentPropertyInfoMap, streamProvider, configuration, 
hostProvider,
+    
super(updateComponentMetricsWithAggregateFunctionSupport(componentPropertyInfoMap),
+      streamProvider, configuration, hostProvider,
       clusterNamePropertyId, null, componentNamePropertyId);
   }
 
-  @Override
-  protected Set<String> getRequestPropertyIds(Request request, Predicate 
predicate) {
-    Set<String> supportedPropertyIds = super.getRequestPropertyIds(request, 
predicate);
+  /**
+   * This method adds supported propertyInfo for component metrics with
+   * aggregate function ids. API calls with multiple aggregate functions
+   * applied to a single metric need this support.
+   *
+   * Currently this support is added only for Component metrics,
+   * this can be easily extended to all levels by moving this method to the
+   * base class: @AMSPropertyProvider.
+   */
+  private static Map<String, Map<String, PropertyInfo>> 
updateComponentMetricsWithAggregateFunctionSupport(
+        Map<String, Map<String, PropertyInfo>> componentMetrics) {
 
-    Set<String> unsupportedPropertyIds = new 
HashSet<String>(request.getPropertyIds());
-    if (predicate != null) {
-      unsupportedPropertyIds.addAll(PredicateHelper.getPropertyIds(predicate));
-    }
-    unsupportedPropertyIds.removeAll(supportedPropertyIds);
-    // Allow for aggregate function names to be a part of metrics names
-    if (!unsupportedPropertyIds.isEmpty()) {
-      for (String propertyId : unsupportedPropertyIds) {
-        String[] idWithFunctionArray = stripFunctionFromMetricName(propertyId);
-        if (idWithFunctionArray != null) {
-          propertyIdAggregateFunctionMap.put(idWithFunctionArray[0], 
idWithFunctionArray[1]);
-          supportedPropertyIds.add(idWithFunctionArray[0]);
-        }
-      }
+    if (componentMetrics == null || componentMetrics.isEmpty()) {
+      return componentMetrics;
     }
 
-    return supportedPropertyIds;
-  }
+    // For every component
+    for (Map<String, PropertyInfo> componentMetricInfo : 
componentMetrics.values()) {
+      Map<String, PropertyInfo> aggregateMetrics = new HashMap<String, 
PropertyInfo>();
+      // For every metric
+      for (Map.Entry<String, PropertyInfo> metricEntry : 
componentMetricInfo.entrySet()) {
+        // For each aggregate function id
+        for (String identifierToAdd : aggregateFunctionIdentifierMap.values()) 
{
+          String metricInfoKey = metricEntry.getKey() + identifierToAdd;
+          // This disallows metric key suffix of the form "._sum._sum" for
+          // the sake of avoiding duplicates
+          if (componentMetricInfo.containsKey(metricInfoKey)) {
+            continue;
+          }
 
-  /**
-   * Return array of function identifier and metricsName stripped of function
-   * identifier for metricsNames with aggregate function identifiers as
-   * trailing suffixes.
-   */
-  protected String[] stripFunctionFromMetricName(String propertyId) {
-    for (Map.Entry<AGGREGATE_FUNCTION_IDENTIFIER, String> identifierEntry :
-        aggregateFunctionIdentifierMap.entrySet()) {
-      if (propertyId.endsWith(identifierEntry.getValue())) {
-        String[] retVal = new String[2];
-        retVal[0] = StringUtils.removeEnd(propertyId, 
identifierEntry.getValue());
-        retVal[1] = identifierEntry.getValue();
-        return retVal;
+          PropertyInfo propertyInfo = metricEntry.getValue();
+          PropertyInfo metricInfoValue = new PropertyInfo(
+            propertyInfo.getPropertyId() + identifierToAdd,
+            propertyInfo.isTemporal(),
+            propertyInfo.isPointInTime());
+          metricInfoValue.setAmsHostMetric(propertyInfo.isAmsHostMetric());
+          metricInfoValue.setAmsId(propertyInfo.getAmsId());
+          metricInfoValue.setUnit(propertyInfo.getUnit());
+
+          aggregateMetrics.put(metricInfoKey, metricInfoValue);
+        }
       }
+      componentMetricInfo.putAll(aggregateMetrics);
     }
-    return null;
+
+    return componentMetrics;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java
index c05e112..8bd1b2d 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java
@@ -53,6 +53,7 @@ import java.util.regex.Pattern;
 import static org.apache.ambari.server.Role.HBASE_MASTER;
 import static org.apache.ambari.server.Role.HBASE_REGIONSERVER;
 import static org.apache.ambari.server.Role.METRICS_COLLECTOR;
+import static 
org.apache.ambari.server.controller.metrics.MetricsPaddingMethod.ZERO_PADDING_PARAM;
 import static 
org.apache.ambari.server.controller.metrics.MetricsServiceProvider.MetricsService.TIMELINE_METRICS;
 import static org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
 
@@ -70,8 +71,6 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
   }
   protected static final EnumMap<AGGREGATE_FUNCTION_IDENTIFIER, String> 
aggregateFunctionIdentifierMap =
     new EnumMap<AGGREGATE_FUNCTION_IDENTIFIER, 
String>(AGGREGATE_FUNCTION_IDENTIFIER.class);
-  // Map to store aggregate functions that apply to metrics.
-  protected Map<String, String> propertyIdAggregateFunctionMap = new 
HashMap<String, String>();
 
   static {
     TIMELINE_APPID_MAP.put(HBASE_MASTER.name(), "HBASE");
@@ -114,6 +113,32 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
   }
 
   /**
+   * Support properties with aggregate functions and metrics padding method.
+   */
+  @Override
+  public Set<String> checkPropertyIds(Set<String> propertyIds) {
+    for (String propertyId : propertyIds) {
+      if (propertyId.startsWith(ZERO_PADDING_PARAM)
+          || hasAggregateFunctionSuffix(propertyId)) {
+        propertyIds.remove(propertyId);
+      }
+    }
+    return propertyIds;
+  }
+
+  /**
+   * Check if property ends with a trailing suffix
+   */
+  protected boolean hasAggregateFunctionSuffix(String propertyId) {
+    for (String aggregateFunctionId : aggregateFunctionIdentifierMap.values()) 
{
+      if (propertyId.endsWith(aggregateFunctionId)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
    * The information required to make a single call to the Metrics service.
    */
   class MetricsRequest {
@@ -437,9 +462,6 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
       }
     }
 
-    // Clear function map
-    propertyIdAggregateFunctionMap.clear();
-
     return resources;
   }
 
@@ -515,10 +537,7 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
         for (Map.Entry<String, PropertyInfo> entry : 
propertyInfoMap.entrySet()) {
           String propertyId = entry.getKey();
           PropertyInfo propertyInfo = entry.getValue();
-          // For regex properties this propertyId != id
-          String amsPropertyId = 
getPropertyIdWithAggregateFunctionId(propertyInfo, id, false);
-          String ambariPropertyId = 
getPropertyIdWithAggregateFunctionId(propertyInfo, id, true);
-          TemporalInfo temporalInfo = 
request.getTemporalInfo(ambariPropertyId);
+          TemporalInfo temporalInfo = request.getTemporalInfo(id);
 
           if ((temporalInfo == null && propertyInfo.isPointInTime()) ||
             (temporalInfo != null && propertyInfo.isTemporal())) {
@@ -531,10 +550,10 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
               requests.put(temporalInfo, metricsRequest);
             }
             metricsRequest.putResource(getHostName(resource), resource);
-            metricsRequest.putPropertyId(amsPropertyId, propertyId);
+            metricsRequest.putPropertyId(propertyInfo.getPropertyId(), 
propertyId);
             // If request is for a host metric we need to create multiple 
requests
             if (propertyInfo.isAmsHostMetric()) {
-              metricsRequest.putHosComponentHostMetric(amsPropertyId);
+              
metricsRequest.putHosComponentHostMetric(propertyInfo.getPropertyId());
             }
           }
         }
@@ -544,22 +563,6 @@ public abstract class AMSPropertyProvider extends 
MetricsPropertyProvider {
     return requestMap;
   }
 
-  /**
-   * Return a property Id with aggregate function identifier.
-   * @param propertyInfo @PropertyInfo
-   * @param propertyId Property Id from request / predicate
-   * @param ambariPropertyId True: Return ambari property id,
-   *                         else return id using PropertyInfo
-   */
-  private String getPropertyIdWithAggregateFunctionId(PropertyInfo 
propertyInfo,
-                                  String propertyId, boolean ambariPropertyId) 
{
-    if (propertyIdAggregateFunctionMap.containsKey(propertyId)) {
-      return (ambariPropertyId ? propertyId : propertyInfo.getPropertyId()) +
-        propertyIdAggregateFunctionMap.get(propertyId);
-    }
-    return ambariPropertyId ? propertyId : propertyInfo.getPropertyId();
-  }
-
   static URIBuilder getAMSUriBuilder(String hostname, int port) {
     URIBuilder uriBuilder = new URIBuilder();
     uriBuilder.setScheme("http");

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProviderTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProviderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProviderTest.java
index 3fe0131..b8e0596 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProviderTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackDefinedPropertyProviderTest.java
@@ -29,6 +29,7 @@ import java.util.Set;
 
 import org.apache.ambari.server.controller.jmx.TestStreamProvider;
 import org.apache.ambari.server.controller.metrics.JMXPropertyProviderTest;
+import org.apache.ambari.server.controller.metrics.MetricsServiceProvider;
 import 
org.apache.ambari.server.controller.metrics.ganglia.GangliaPropertyProviderTest.TestGangliaHostProvider;
 import 
org.apache.ambari.server.controller.metrics.ganglia.GangliaPropertyProviderTest.TestGangliaServiceProvider;
 import org.apache.ambari.server.controller.spi.Predicate;
@@ -1034,4 +1035,54 @@ public class StackDefinedPropertyProviderTest {
 
   }
 
+  @Test
+  public void testPopulateResourcesWithAggregateFunctionMetrics() throws 
Exception {
+
+    String metric = "metrics/rpc/NumOpenConnections._sum";
+
+    org.apache.ambari.server.controller.metrics.ganglia.TestStreamProvider 
streamProvider =
+      new 
org.apache.ambari.server.controller.metrics.ganglia.TestStreamProvider("ams/aggregate_component_metric.json");
+
+    JMXPropertyProviderTest.TestJMXHostProvider jmxHostProvider = new 
JMXPropertyProviderTest.TestJMXHostProvider(true);
+    TestGangliaHostProvider hostProvider = new TestGangliaHostProvider();
+    MetricsServiceProvider serviceProvider = new MetricsServiceProvider() {
+      @Override
+      public MetricsService getMetricsServiceType() {
+        return MetricsService.TIMELINE_METRICS;
+      }
+    };
+
+    StackDefinedPropertyProvider propertyProvider = new 
StackDefinedPropertyProvider(
+      Resource.Type.Component,
+      jmxHostProvider,
+      hostProvider,
+      serviceProvider,
+      streamProvider,
+      PropertyHelper.getPropertyId("HostRoles", "cluster_name"),
+      PropertyHelper.getPropertyId("HostRoles", "host_name"),
+      PropertyHelper.getPropertyId("HostRoles", "component_name"),
+      PropertyHelper.getPropertyId("HostRoles", "state"),
+      new EmptyPropertyProvider(),
+      new EmptyPropertyProvider());
+
+
+    Resource resource = new ResourceImpl(Resource.Type.Component);
+
+    resource.setProperty("HostRoles/cluster_name", "c1");
+    resource.setProperty("HostRoles/service_name", "HBASE");
+    resource.setProperty(HOST_COMPONENT_COMPONENT_NAME_PROPERTY_ID, 
"HBASE_REGIONSERVER");
+
+    // only ask for one property
+    Map<String, TemporalInfo> temporalInfoMap = new HashMap<String, 
TemporalInfo>();
+    temporalInfoMap.put(metric, new TemporalInfoImpl(1429824611300L, 
1429825241400L, 1L));
+    Request request = 
PropertyHelper.getReadRequest(Collections.singleton(metric), temporalInfoMap);
+
+    Assert.assertEquals(1, 
propertyProvider.populateResources(Collections.singleton(resource), request, 
null).size());
+
+    Assert.assertEquals(4, PropertyHelper.getProperties(resource).size());
+    Assert.assertNotNull(resource.getPropertyValue(metric));
+    Number[][] metricsArray = (Number[][]) resource.getPropertyValue(metric);
+    Assert.assertEquals(32, metricsArray.length);
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java
index befd062..1eaa9cc 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java
@@ -789,11 +789,7 @@ public class GangliaPropertyProviderTest {
     Assert.assertEquals(11, PropertyHelper.getProperties(resource).size());
     
Assert.assertNotNull(resource.getPropertyValue(FLUME_CHANNEL_CAPACITY_PROPERTY));
   }
-  
-
 
-
-  
   private boolean isUrlParamsEquals(URIBuilder actualUri, URIBuilder 
expectedUri) {
     for (final NameValuePair expectedParam : expectedUri.getQueryParams()) {
       NameValuePair actualParam = (NameValuePair) 
CollectionUtils.find(actualUri.getQueryParams(), new Predicate() {

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java
index b72dcaf..1573708 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java
@@ -404,7 +404,7 @@ public class AMSPropertyProviderTest {
     uriBuilder.addParameter("startTime", "1429824611300");
     uriBuilder.addParameter("endTime", "1429825241400");
     Assert.assertEquals(uriBuilder.toString(), streamProvider.getLastSpec());
-    Number[][] val = (Number[][]) 
res.getPropertyValue(propertyProvider.stripFunctionFromMetricName(propertyId)[0]);
+    Number[][] val = (Number[][]) res.getPropertyValue(propertyId);
     Assert.assertEquals(32, val.length);
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/21442eb8/ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HBASE/metrics.json
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HBASE/metrics.json 
b/ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HBASE/metrics.json
index 2a46f29..955c90a 100644
--- 
a/ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HBASE/metrics.json
+++ 
b/ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HBASE/metrics.json
@@ -3186,5 +3186,21 @@
         }
       }
     ]
+  },
+  "HBASE_REGIONSERVER": {
+    "Component": [
+      {
+        "type": "ganglia",
+        "metrics": {
+          "default": {
+            "metrics/rpc/NumOpenConnections": {
+              "metric": "rpc.rpc.NumOpenConnections",
+              "pointInTime": true,
+              "temporal": true
+            }
+          }
+        }
+      }
+    ]
   }
 }

Reply via email to