Skip to content

Commit

Permalink
Fix profiling naming issues
Browse files Browse the repository at this point in the history 
Some code-paths use anonymous classes (such as NonCollectingAggregator
in terms agg), which messes up the display name of the profiler.  If
we encounter an anonymous class, we need to grab the super's name.

Another naming issue was that ProfileAggs were not delegating to the
wrapped agg's name for toString(), leading to ugly display.

This PR also fixes up the profile documentation.  Some of the examples were
executing against empty indices, which shows different profile results
than a populated index (and made for confusing examples).

Finally, I switched the agg display names from the fully qualified name
to the simple name, so that it's similar to how the query profiles work.

Closes #26405
  • Loading branch information
@polyfractal
polyfractal committed Nov 20, 2017
1 parent 5a6abb5 commit 5990364
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 115 deletions.
10 changes: 8 additions & 2 deletions ...ain/java/org/elasticsearch/search/profile/aggregation/InternalAggregationProfileTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,16 @@ protected AggregationProfileBreakdown createProfileBreakdown() {

@Override
protected String getTypeFromElement(Aggregator element) {

// Anonymous classes (such as NonCollectingAggregator in TermsAgg) won't have a name,
// we need to get the super class
if (element.getClass().getSimpleName().isEmpty() == true) {
return element.getClass().getSuperclass().getSimpleName();
}
if (element instanceof MultiBucketAggregatorWrapper) {
return ((MultiBucketAggregatorWrapper) element).getWrappedClass().getName();
return ((MultiBucketAggregatorWrapper) element).getWrappedClass().getSimpleName();
}
return element.getClass().getName();
return element.getClass().getSimpleName();
}

@Override
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,8 @@ public void postCollection() throws IOException {
delegate.postCollection();
}

@Override
public String toString() {
return delegate.toString();
}
}
5 changes: 5 additions & 0 deletions core/src/main/java/org/elasticsearch/search/profile/query/InternalQueryProfileTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ protected QueryProfileBreakdown createProfileBreakdown() {

@Override
protected String getTypeFromElement(Query query) {
// Anonymous classes won't have a name,
// we need to get the super class
if (query.getClass().getSimpleName().isEmpty() == true) {
return query.getClass().getSuperclass().getSimpleName();
}
return query.getClass().getSimpleName();
}

Expand Down
38 changes: 19 additions & 19 deletions core/src/test/java/org/elasticsearch/search/profile/aggregation/AggregationProfilerIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void testSimpleProfile() {
ProfileResult histoAggResult = aggProfileResultsList.get(0);
assertThat(histoAggResult, notNullValue());
assertThat(histoAggResult.getQueryName(),
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
equalTo("HistogramAggregator"));
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
assertThat(histoAggResult.getProfiledChildren().size(), equalTo(0));
assertThat(histoAggResult.getTime(), greaterThan(0L));
Expand Down Expand Up @@ -137,7 +137,7 @@ public void testMultiLevelProfile() {
ProfileResult histoAggResult = aggProfileResultsList.get(0);
assertThat(histoAggResult, notNullValue());
assertThat(histoAggResult.getQueryName(),
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
equalTo("HistogramAggregator"));
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
assertThat(histoAggResult.getTime(), greaterThan(0L));
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
Expand All @@ -154,7 +154,7 @@ public void testMultiLevelProfile() {

ProfileResult termsAggResult = histoAggResult.getProfiledChildren().get(0);
assertThat(termsAggResult, notNullValue());
assertThat(termsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getName()));
assertThat(termsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getSimpleName()));
assertThat(termsAggResult.getLuceneDescription(), equalTo("terms"));
assertThat(termsAggResult.getTime(), greaterThan(0L));
Map<String, Long> termsBreakdown = termsAggResult.getTimeBreakdown();
Expand All @@ -171,7 +171,7 @@ public void testMultiLevelProfile() {

ProfileResult avgAggResult = termsAggResult.getProfiledChildren().get(0);
assertThat(avgAggResult, notNullValue());
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
assertThat(avgAggResult.getTime(), greaterThan(0L));
Map<String, Long> avgBreakdown = termsAggResult.getTimeBreakdown();
Expand Down Expand Up @@ -207,7 +207,7 @@ public void testMultiLevelProfileBreadthFirst() {
ProfileResult histoAggResult = aggProfileResultsList.get(0);
assertThat(histoAggResult, notNullValue());
assertThat(histoAggResult.getQueryName(),
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
equalTo("HistogramAggregator"));
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
assertThat(histoAggResult.getTime(), greaterThan(0L));
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
Expand All @@ -224,7 +224,7 @@ public void testMultiLevelProfileBreadthFirst() {

ProfileResult termsAggResult = histoAggResult.getProfiledChildren().get(0);
assertThat(termsAggResult, notNullValue());
assertThat(termsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getName()));
assertThat(termsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getSimpleName()));
assertThat(termsAggResult.getLuceneDescription(), equalTo("terms"));
assertThat(termsAggResult.getTime(), greaterThan(0L));
Map<String, Long> termsBreakdown = termsAggResult.getTimeBreakdown();
Expand All @@ -241,7 +241,7 @@ public void testMultiLevelProfileBreadthFirst() {

ProfileResult avgAggResult = termsAggResult.getProfiledChildren().get(0);
assertThat(avgAggResult, notNullValue());
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
assertThat(avgAggResult.getTime(), greaterThan(0L));
Map<String, Long> avgBreakdown = termsAggResult.getTimeBreakdown();
Expand Down Expand Up @@ -277,7 +277,7 @@ public void testDiversifiedAggProfile() {
ProfileResult diversifyAggResult = aggProfileResultsList.get(0);
assertThat(diversifyAggResult, notNullValue());
assertThat(diversifyAggResult.getQueryName(),
equalTo(DiversifiedOrdinalsSamplerAggregator.class.getName()));
equalTo(DiversifiedOrdinalsSamplerAggregator.class.getSimpleName()));
assertThat(diversifyAggResult.getLuceneDescription(), equalTo("diversify"));
assertThat(diversifyAggResult.getTime(), greaterThan(0L));
Map<String, Long> histoBreakdown = diversifyAggResult.getTimeBreakdown();
Expand All @@ -294,7 +294,7 @@ public void testDiversifiedAggProfile() {

ProfileResult maxAggResult = diversifyAggResult.getProfiledChildren().get(0);
assertThat(maxAggResult, notNullValue());
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
assertThat(maxAggResult.getTime(), greaterThan(0L));
Map<String, Long> termsBreakdown = maxAggResult.getTimeBreakdown();
Expand Down Expand Up @@ -338,7 +338,7 @@ public void testComplexProfile() {
ProfileResult histoAggResult = aggProfileResultsList.get(0);
assertThat(histoAggResult, notNullValue());
assertThat(histoAggResult.getQueryName(),
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
equalTo("HistogramAggregator"));
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
assertThat(histoAggResult.getTime(), greaterThan(0L));
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
Expand All @@ -355,7 +355,7 @@ public void testComplexProfile() {

ProfileResult tagsAggResult = histoAggResult.getProfiledChildren().get(0);
assertThat(tagsAggResult, notNullValue());
assertThat(tagsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getName()));
assertThat(tagsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getSimpleName()));
assertThat(tagsAggResult.getLuceneDescription(), equalTo("tags"));
assertThat(tagsAggResult.getTime(), greaterThan(0L));
Map<String, Long> tagsBreakdown = tagsAggResult.getTimeBreakdown();
Expand All @@ -372,7 +372,7 @@ public void testComplexProfile() {

ProfileResult avgAggResult = tagsAggResult.getProfiledChildren().get(0);
assertThat(avgAggResult, notNullValue());
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
assertThat(avgAggResult.getTime(), greaterThan(0L));
Map<String, Long> avgBreakdown = tagsAggResult.getTimeBreakdown();
Expand All @@ -389,7 +389,7 @@ public void testComplexProfile() {

ProfileResult maxAggResult = tagsAggResult.getProfiledChildren().get(1);
assertThat(maxAggResult, notNullValue());
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
assertThat(maxAggResult.getTime(), greaterThan(0L));
Map<String, Long> maxBreakdown = tagsAggResult.getTimeBreakdown();
Expand All @@ -406,7 +406,7 @@ public void testComplexProfile() {

ProfileResult stringsAggResult = histoAggResult.getProfiledChildren().get(1);
assertThat(stringsAggResult, notNullValue());
assertThat(stringsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getName()));
assertThat(stringsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getSimpleName()));
assertThat(stringsAggResult.getLuceneDescription(), equalTo("strings"));
assertThat(stringsAggResult.getTime(), greaterThan(0L));
Map<String, Long> stringsBreakdown = stringsAggResult.getTimeBreakdown();
Expand All @@ -423,7 +423,7 @@ public void testComplexProfile() {

avgAggResult = stringsAggResult.getProfiledChildren().get(0);
assertThat(avgAggResult, notNullValue());
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
assertThat(avgAggResult.getTime(), greaterThan(0L));
avgBreakdown = stringsAggResult.getTimeBreakdown();
Expand All @@ -440,7 +440,7 @@ public void testComplexProfile() {

maxAggResult = stringsAggResult.getProfiledChildren().get(1);
assertThat(maxAggResult, notNullValue());
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
assertThat(maxAggResult.getTime(), greaterThan(0L));
maxBreakdown = stringsAggResult.getTimeBreakdown();
Expand All @@ -457,7 +457,7 @@ public void testComplexProfile() {

tagsAggResult = stringsAggResult.getProfiledChildren().get(2);
assertThat(tagsAggResult, notNullValue());
assertThat(tagsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getName()));
assertThat(tagsAggResult.getQueryName(), equalTo(GlobalOrdinalsStringTermsAggregator.class.getSimpleName()));
assertThat(tagsAggResult.getLuceneDescription(), equalTo("tags"));
assertThat(tagsAggResult.getTime(), greaterThan(0L));
tagsBreakdown = tagsAggResult.getTimeBreakdown();
Expand All @@ -474,7 +474,7 @@ public void testComplexProfile() {

avgAggResult = tagsAggResult.getProfiledChildren().get(0);
assertThat(avgAggResult, notNullValue());
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
assertThat(avgAggResult.getTime(), greaterThan(0L));
avgBreakdown = tagsAggResult.getTimeBreakdown();
Expand All @@ -491,7 +491,7 @@ public void testComplexProfile() {

maxAggResult = tagsAggResult.getProfiledChildren().get(1);
assertThat(maxAggResult, notNullValue());
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
assertThat(maxAggResult.getTime(), greaterThan(0L));
maxBreakdown = tagsAggResult.getTimeBreakdown();
Expand Down
Loading

0 comments on commit 5990364

Please sign in to comment.