flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-4563) [metrics] scope caching not adjusted for multiple reporters
Date Mon, 14 Nov 2016 11:20:58 GMT

    [ https://issues.apache.org/jira/browse/FLINK-4563?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15663522#comment-15663522
] 

ASF GitHub Bot commented on FLINK-4563:
---------------------------------------

Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2650#discussion_r87780687
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroupTest.java
---
    @@ -44,4 +52,132 @@ protected QueryScopeInfo createQueryServiceMetricInfo(CharacterFilter
filter) {
     		
     		registry.shutdown();
     	}
    +
    +	// for test case: one filter for different reporters with different of scope delimiter
    +	protected static CharacterFilter staticCharacterFilter = new CharacterFilter() {
    +		@Override
    +		public String filterCharacters(String input) {
    +			return input.replace("C", "RR");
    +		}
    +	};
    +
    +	@Test
    +	public void filteringForMultipleReporters() {
    +		TestReporter1.countSuccessChecks = 0;
    +		Configuration config = new Configuration();
    +		config.setString(ConfigConstants.METRICS_SCOPE_NAMING_TM, "A.B.C.D");
    +		config.setString(ConfigConstants.METRICS_REPORTERS_LIST, "test1,test2");
    +		config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + "test1." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX,
TestReporter1.class.getName());
    +		config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + "test2." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX,
TestReporter2.class.getName());
    +		config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + "test1." + ConfigConstants.METRICS_REPORTER_SCOPE_DELIMITER,
"-");
    +		config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + "test2." + ConfigConstants.METRICS_REPORTER_SCOPE_DELIMITER,
"!");
    +
    +
    +		MetricRegistry testRegistry = new MetricRegistryTest(MetricRegistryConfiguration.fromConfiguration(config));
    +		TaskManagerMetricGroup tmGroup = new TaskManagerMetricGroup(testRegistry, "host", "id");
    +		tmGroup.counter(1);
    +		testRegistry.shutdown();
    +		assert TestReporter1.countSuccessChecks == 4;
    +	}
    +
    +	@Test
    +	public void filteringForNullReporters() {
    +		MetricRegistryTest.countSuccessChecks = 0;
    +		Configuration config = new Configuration();
    +		config.setString(ConfigConstants.METRICS_SCOPE_NAMING_TM, "A.B.C.D");
    +		MetricRegistry testRegistry = new MetricRegistryTest(MetricRegistryConfiguration.fromConfiguration(config));
    +		TaskManagerMetricGroup tmGroupForTestRegistry = new TaskManagerMetricGroup(testRegistry,
"host", "id");
    +		assert testRegistry.getReporters().size() == 0;
    +		tmGroupForTestRegistry.counter(1);
    +		testRegistry.shutdown();
    +		assert MetricRegistryTest.countSuccessChecks == 1;
    +	}
    +
    +	public static class TestReporter1 extends TestReporter {
    +		protected static int countSuccessChecks = 0;
    +		@Override
    +		public String filterCharacters(String input) {
    +			return input.replace("A", "RR");
    +		}
    +		@Override
    +		public void notifyOfAddedMetric(Metric metric, String metricName, MetricGroup group)
{
    +			assertEquals("A-B-C-D-1", group.getMetricIdentifier(metricName));
    +			// ignore all next filters for scope -  because scopeString cached with only first
filter
    +			assertEquals("A-B-C-D-1", group.getMetricIdentifier(metricName, staticCharacterFilter));
    +			assertEquals("A-B-C-D-1", group.getMetricIdentifier(metricName, this));
    +			assertEquals("A-B-C-D-4", group.getMetricIdentifier(metricName, new CharacterFilter()
{
    +				@Override
    +				public String filterCharacters(String input) {
    +					return input.replace("B", "RR").replace("1", "4");
    +				}
    +			}));
    +			countSuccessChecks++;
    +		}
    +	}
    +	public static class TestReporter2 extends TestReporter1 {
    +		@Override
    +		public String filterCharacters(String input) {
    +			return input.replace("B", "RR");
    +		}
    +		@Override
    +		public void notifyOfAddedMetric(Metric metric, String metricName, MetricGroup group)
{
    +			assertEquals("A!RR!C!D!1", group.getMetricIdentifier(metricName, this));
    +			// ignore all next filters -  because scopeString cached with only first filter
    +			assertEquals("A!RR!C!D!1", group.getMetricIdentifier(metricName));
    +			assertEquals("A!RR!C!D!1", group.getMetricIdentifier(metricName, staticCharacterFilter));
    +			assertEquals("A!RR!C!D!3", group.getMetricIdentifier(metricName, new CharacterFilter()
{
    +				@Override
    +				public String filterCharacters(String input) {
    +					return input.replace("A", "RR").replace("1", "3");
    +				}
    +			}));
    +			countSuccessChecks++;
    +		}
    +	}
    +	static class MetricRegistryTest extends MetricRegistry {
    +		protected static int countSuccessChecks = 0;
    --- End diff --
    
    Since we have access to the MetricRegistryTest object we can use a non-static field instead.


> [metrics] scope caching not adjusted for multiple reporters
> -----------------------------------------------------------
>
>                 Key: FLINK-4563
>                 URL: https://issues.apache.org/jira/browse/FLINK-4563
>             Project: Flink
>          Issue Type: Bug
>          Components: Metrics
>    Affects Versions: 1.1.0
>            Reporter: Chesnay Schepler
>            Assignee: Anton Mushin
>
> Every metric group contains a scope string, representing what entities (job/task/etc.)
a given metric belongs to, which is calculated on demand. 
> Before this string is cached a CharacterFilter is applied to it, which is provided by
the callee, usually a reporter. This was done since different reporters have different requirements
in regards to valid characters. The filtered string is cached so that we don't have to refilter
the string every time.
> This all works fine with a single reporter; with multiple however it is completely broken
as only the first filter is ever applied.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message