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 + } + } + } + } + ] } }