Skip to content

Commit 6e9e07d

Browse files
polyfractalpolyfractal
authored
Fix profiling naming issues (#27133)
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
1 parent 766d29e commit 6e9e07d

File tree

5 files changed

+139
-115
lines changed

5 files changed

+139
-115
lines changed

‎core/src/main/java/org/elasticsearch/search/profile/aggregation/InternalAggregationProfileTree.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,16 @@ protected AggregationProfileBreakdown createProfileBreakdown() {
3232

3333
@Override
3434
protected String getTypeFromElement(Aggregator element) {
35+
36+
// Anonymous classes (such as NonCollectingAggregator in TermsAgg) won't have a name,
37+
// we need to get the super class
38+
if (element.getClass().getSimpleName().isEmpty() == true) {
39+
return element.getClass().getSuperclass().getSimpleName();
40+
}
3541
if (element instanceof MultiBucketAggregatorWrapper) {
36-
return ((MultiBucketAggregatorWrapper) element).getWrappedClass().getName();
42+
return ((MultiBucketAggregatorWrapper) element).getWrappedClass().getSimpleName();
3743
}
38-
return element.getClass().getName();
44+
return element.getClass().getSimpleName();
3945
}
4046

4147
@Override

‎core/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingAggregator.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,8 @@ public void postCollection() throws IOException {
110110
delegate.postCollection();
111111
}
112112

113+
@Override
114+
public String toString() {
115+
return delegate.toString();
116+
}
113117
}

‎core/src/main/java/org/elasticsearch/search/profile/query/InternalQueryProfileTree.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ protected QueryProfileBreakdown createProfileBreakdown() {
4141

4242
@Override
4343
protected String getTypeFromElement(Query query) {
44+
// Anonymous classes won't have a name,
45+
// we need to get the super class
46+
if (query.getClass().getSimpleName().isEmpty() == true) {
47+
return query.getClass().getSuperclass().getSimpleName();
48+
}
4449
return query.getClass().getSimpleName();
4550
}
4651

‎core/src/test/java/org/elasticsearch/search/profile/aggregation/AggregationProfilerIT.java

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void testSimpleProfile() {
100100
ProfileResult histoAggResult = aggProfileResultsList.get(0);
101101
assertThat(histoAggResult, notNullValue());
102102
assertThat(histoAggResult.getQueryName(),
103-
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
103+
equalTo("HistogramAggregator"));
104104
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
105105
assertThat(histoAggResult.getProfiledChildren().size(), equalTo(0));
106106
assertThat(histoAggResult.getTime(), greaterThan(0L));
@@ -137,7 +137,7 @@ public void testMultiLevelProfile() {
137137
ProfileResult histoAggResult = aggProfileResultsList.get(0);
138138
assertThat(histoAggResult, notNullValue());
139139
assertThat(histoAggResult.getQueryName(),
140-
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
140+
equalTo("HistogramAggregator"));
141141
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
142142
assertThat(histoAggResult.getTime(), greaterThan(0L));
143143
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
@@ -154,7 +154,7 @@ public void testMultiLevelProfile() {
154154

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

172172
ProfileResult avgAggResult = termsAggResult.getProfiledChildren().get(0);
173173
assertThat(avgAggResult, notNullValue());
174-
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
174+
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
175175
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
176176
assertThat(avgAggResult.getTime(), greaterThan(0L));
177177
Map<String, Long> avgBreakdown = termsAggResult.getTimeBreakdown();
@@ -207,7 +207,7 @@ public void testMultiLevelProfileBreadthFirst() {
207207
ProfileResult histoAggResult = aggProfileResultsList.get(0);
208208
assertThat(histoAggResult, notNullValue());
209209
assertThat(histoAggResult.getQueryName(),
210-
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
210+
equalTo("HistogramAggregator"));
211211
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
212212
assertThat(histoAggResult.getTime(), greaterThan(0L));
213213
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
@@ -224,7 +224,7 @@ public void testMultiLevelProfileBreadthFirst() {
224224

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

242242
ProfileResult avgAggResult = termsAggResult.getProfiledChildren().get(0);
243243
assertThat(avgAggResult, notNullValue());
244-
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getName()));
244+
assertThat(avgAggResult.getQueryName(), equalTo(AvgAggregator.class.getSimpleName()));
245245
assertThat(avgAggResult.getLuceneDescription(), equalTo("avg"));
246246
assertThat(avgAggResult.getTime(), greaterThan(0L));
247247
Map<String, Long> avgBreakdown = termsAggResult.getTimeBreakdown();
@@ -277,7 +277,7 @@ public void testDiversifiedAggProfile() {
277277
ProfileResult diversifyAggResult = aggProfileResultsList.get(0);
278278
assertThat(diversifyAggResult, notNullValue());
279279
assertThat(diversifyAggResult.getQueryName(),
280-
equalTo(DiversifiedOrdinalsSamplerAggregator.class.getName()));
280+
equalTo(DiversifiedOrdinalsSamplerAggregator.class.getSimpleName()));
281281
assertThat(diversifyAggResult.getLuceneDescription(), equalTo("diversify"));
282282
assertThat(diversifyAggResult.getTime(), greaterThan(0L));
283283
Map<String, Long> histoBreakdown = diversifyAggResult.getTimeBreakdown();
@@ -294,7 +294,7 @@ public void testDiversifiedAggProfile() {
294294

295295
ProfileResult maxAggResult = diversifyAggResult.getProfiledChildren().get(0);
296296
assertThat(maxAggResult, notNullValue());
297-
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
297+
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
298298
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
299299
assertThat(maxAggResult.getTime(), greaterThan(0L));
300300
Map<String, Long> termsBreakdown = maxAggResult.getTimeBreakdown();
@@ -338,7 +338,7 @@ public void testComplexProfile() {
338338
ProfileResult histoAggResult = aggProfileResultsList.get(0);
339339
assertThat(histoAggResult, notNullValue());
340340
assertThat(histoAggResult.getQueryName(),
341-
equalTo("org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregator"));
341+
equalTo("HistogramAggregator"));
342342
assertThat(histoAggResult.getLuceneDescription(), equalTo("histo"));
343343
assertThat(histoAggResult.getTime(), greaterThan(0L));
344344
Map<String, Long> histoBreakdown = histoAggResult.getTimeBreakdown();
@@ -355,7 +355,7 @@ public void testComplexProfile() {
355355

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

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

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

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

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

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

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

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

492492
maxAggResult = tagsAggResult.getProfiledChildren().get(1);
493493
assertThat(maxAggResult, notNullValue());
494-
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getName()));
494+
assertThat(maxAggResult.getQueryName(), equalTo(MaxAggregator.class.getSimpleName()));
495495
assertThat(maxAggResult.getLuceneDescription(), equalTo("max"));
496496
assertThat(maxAggResult.getTime(), greaterThan(0L));
497497
maxBreakdown = tagsAggResult.getTimeBreakdown();

0 commit comments

Comments
 (0)