Skip to content

Add context dimension to DefaultQueryMetrics #10578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jihoonson merged 4 commits into from
Dec 2, 2020
Merged

Add context dimension to DefaultQueryMetrics #10578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b31ad67
Add context dimension to DefaultQueryMetrics
Nov 13, 2020
00efa42
remove redundant addition of context dimension from DruidMetrics now …
Nov 18, 2020
1d0ad2b
update SearchQueryMetrics to reflect the same pattern as other defaul…
Nov 18, 2020
bd3325e
add PublicApi annotation for context in QueryMetrics Interface
Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions processing/src/main/java/org/apache/druid/query/DefaultQueryMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public void query(QueryType query)
queryId(query);
subQueryId(query);
sqlQueryId(query);
context(query);
}

@Override
Expand Down
1 change: 0 additions & 1 deletion processing/src/main/java/org/apache/druid/query/DruidMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ public static <T> QueryMetrics<?> makeRequestMetrics(
} else {
queryMetrics = queryMetricsFactory.makeMetrics(query);
}
queryMetrics.context(query);
queryMetrics.remoteAddress(remoteAddr);
return queryMetrics;
}
Expand Down
5 changes: 3 additions & 2 deletions processing/src/main/java/org/apache/druid/query/QueryMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@
* implement extra methods from SegmentMetadataQueryMetrics interfaces with empty bodies, AND DELEGATE ALL OTHER
* METHODS TO A QueryMetrics OBJECT, provided as a sole parameter in DefaultSegmentMetadataQueryMetrics constructor.
*
* NOTE: query(), dataSource(), queryType(), interval(), hasFilters(), duration(), queryId() and sqlQueryId() methods
* or any "pre-query-execution-time" methods should either have a empty body or throw exception.
* NOTE: query(), dataSource(), queryType(), interval(), hasFilters(), duration(), queryId(), sqlQueryId(), and
* context() methods or any "pre-query-execution-time" methods should either have a empty body or throw exception.
*
* 3. Create `interface SegmentMetadataQueryMetricsFactory` with a single method
* `SegmentMetadataQueryMetrics makeMetrics(SegmentMetadataQuery query);`.
Expand Down Expand Up @@ -217,6 +217,7 @@ public interface QueryMetrics<QueryType extends Query<?>>
/**
* Sets {@link Query#getContext()} of the given query as dimension.
*/
@PublicApi
void context(QueryType query);

void server(String host);
Expand Down
2 changes: 1 addition & 1 deletion processing/src/main/java/org/apache/druid/query/search/DefaultSearchQueryMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void granularity(SearchQuery query)
@Override
public void context(SearchQuery query)
{
delegateQueryMetrics.context(query);
throw new ISE("Unsupported method in default query metrics implementation.");
}

@Override
Expand Down
5 changes: 4 additions & 1 deletion processing/src/test/java/org/apache/druid/query/DefaultQueryMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.query;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
Expand Down Expand Up @@ -62,11 +63,12 @@ public void testDefaultQueryMetricsQuery()
.aggregators(new CountAggregatorFactory("count"))
.threshold(5)
.filters(new SelectorDimFilter("tags", "t3", null))
.context(ImmutableMap.of("testKey", "testValue"))
.build();
queryMetrics.query(query);
queryMetrics.reportQueryTime(0).emit(serviceEmitter);
Map<String, Object> actualEvent = cachingEmitter.getLastEmittedEvent().toMap();
Assert.assertEquals(12, actualEvent.size());
Assert.assertEquals(13, actualEvent.size());
Assert.assertTrue(actualEvent.containsKey("feed"));
Assert.assertTrue(actualEvent.containsKey("timestamp"));
Assert.assertEquals("", actualEvent.get("host"));
Expand All @@ -82,6 +84,7 @@ public void testDefaultQueryMetricsQuery()
Assert.assertEquals("", actualEvent.get(DruidMetrics.ID));
Assert.assertEquals("query/time", actualEvent.get("metric"));
Assert.assertEquals(0L, actualEvent.get("value"));
Assert.assertEquals(ImmutableMap.of("testKey", "testValue"), actualEvent.get("context"));
}

@Test
Expand Down
3 changes: 2 additions & 1 deletion processing/src/test/java/org/apache/druid/query/groupby/DefaultGroupByQueryMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testDefaultGroupByQueryMetricsQuery()

queryMetrics.reportQueryTime(0).emit(serviceEmitter);
Map<String, Object> actualEvent = cachingEmitter.getLastEmittedEvent().toMap();
Assert.assertEquals(15, actualEvent.size());
Assert.assertEquals(16, actualEvent.size());
Assert.assertTrue(actualEvent.containsKey("feed"));
Assert.assertTrue(actualEvent.containsKey("timestamp"));
Assert.assertEquals("", actualEvent.get("host"));
Expand All @@ -87,6 +87,7 @@ public void testDefaultGroupByQueryMetricsQuery()
Assert.assertEquals("true", actualEvent.get("hasFilters"));
Assert.assertEquals(expectedInterval.toDuration().toString(), actualEvent.get("duration"));
Assert.assertEquals("", actualEvent.get(DruidMetrics.ID));
Assert.assertEquals(ImmutableMap.of("bySegment", true), actualEvent.get("context"));

// GroupBy-specific dimensions
Assert.assertEquals("1", actualEvent.get("numDimensions"));
Expand Down
5 changes: 4 additions & 1 deletion processing/src/test/java/org/apache/druid/query/search/DefaultSearchQueryMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.query.search;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.query.CachingEmitter;
Expand Down Expand Up @@ -57,6 +58,7 @@ public void testDefaultSearchQueryMetricsQuery()
ImmutableSet.of("t3"),
null
))
.context(ImmutableMap.of("testKey", "testValue"))
.build();

SearchQueryMetrics queryMetrics = DefaultSearchQueryMetricsFactory.instance().makeMetrics(query);
Expand All @@ -65,7 +67,7 @@ public void testDefaultSearchQueryMetricsQuery()

queryMetrics.reportQueryTime(0).emit(serviceEmitter);
Map<String, Object> actualEvent = cachingEmitter.getLastEmittedEvent().toMap();
Assert.assertEquals(12, actualEvent.size());
Assert.assertEquals(13, actualEvent.size());
Assert.assertTrue(actualEvent.containsKey("feed"));
Assert.assertTrue(actualEvent.containsKey("timestamp"));
Assert.assertEquals("", actualEvent.get("host"));
Expand All @@ -79,6 +81,7 @@ public void testDefaultSearchQueryMetricsQuery()
Assert.assertEquals("false", actualEvent.get("hasFilters"));
Assert.assertEquals(expectedIntervals.get(0).toDuration().toString(), actualEvent.get("duration"));
Assert.assertEquals("", actualEvent.get(DruidMetrics.ID));
Assert.assertEquals(ImmutableMap.of("testKey", "testValue"), actualEvent.get("context"));

// Metric
Assert.assertEquals("query/time", actualEvent.get("metric"));
Expand Down
5 changes: 4 additions & 1 deletion ...ng/src/test/java/org/apache/druid/query/timeseries/DefaultTimeseriesQueryMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.query.timeseries;

import com.google.common.collect.ImmutableMap;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.query.CachingEmitter;
import org.apache.druid.query.DefaultQueryMetricsTest;
Expand Down Expand Up @@ -54,12 +55,13 @@ public void testDefaultTimeseriesQueryMetricsQuery()
.aggregators(QueryRunnerTestHelper.ROWS_COUNT, QueryRunnerTestHelper.INDEX_DOUBLE_SUM)
.postAggregators(QueryRunnerTestHelper.ADD_ROWS_INDEX_CONSTANT)
.descending(true)
.context(ImmutableMap.of("testKey", "testValue"))
.build();
queryMetrics.query(query);

queryMetrics.reportQueryTime(0).emit(serviceEmitter);
Map<String, Object> actualEvent = cachingEmitter.getLastEmittedEvent().toMap();
Assert.assertEquals(15, actualEvent.size());
Assert.assertEquals(16, actualEvent.size());
Assert.assertTrue(actualEvent.containsKey("feed"));
Assert.assertTrue(actualEvent.containsKey("timestamp"));
Assert.assertEquals("", actualEvent.get("host"));
Expand All @@ -73,6 +75,7 @@ public void testDefaultTimeseriesQueryMetricsQuery()
Assert.assertEquals("false", actualEvent.get("hasFilters"));
Assert.assertEquals(expectedIntervals.get(0).toDuration().toString(), actualEvent.get("duration"));
Assert.assertEquals("", actualEvent.get(DruidMetrics.ID));
Assert.assertEquals(ImmutableMap.of("testKey", "testValue"), actualEvent.get("context"));

// Timeseries-specific dimensions
Assert.assertEquals("2", actualEvent.get("numMetrics"));
Expand Down
5 changes: 4 additions & 1 deletion processing/src/test/java/org/apache/druid/query/topn/DefaultTopNQueryMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.query.topn;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
Expand Down Expand Up @@ -64,12 +65,13 @@ public void testDefaultTopNQueryMetricsQuery()
.aggregators(new CountAggregatorFactory("count"))
.threshold(5)
.filters(new SelectorDimFilter("tags", "t3", null))
.context(ImmutableMap.of("testKey", "testValue"))
.build();
queryMetrics.query(query);

queryMetrics.reportQueryTime(0).emit(serviceEmitter);
Map<String, Object> actualEvent = cachingEmitter.getLastEmittedEvent().toMap();
Assert.assertEquals(16, actualEvent.size());
Assert.assertEquals(17, actualEvent.size());
Assert.assertTrue(actualEvent.containsKey("feed"));
Assert.assertTrue(actualEvent.containsKey("timestamp"));
Assert.assertEquals("", actualEvent.get("host"));
Expand All @@ -83,6 +85,7 @@ public void testDefaultTopNQueryMetricsQuery()
Assert.assertEquals("true", actualEvent.get("hasFilters"));
Assert.assertEquals(expectedIntervals.get(0).toDuration().toString(), actualEvent.get("duration"));
Assert.assertEquals("", actualEvent.get(DruidMetrics.ID));
Assert.assertEquals(ImmutableMap.of("testKey", "testValue"), actualEvent.get("context"));

// TopN-specific dimensions
Assert.assertEquals("5", actualEvent.get("threshold"));
Expand Down