Skip to content

Commit 14749f7

Browse files
Fix ProfileScorer.getChildren() to expose wrapped scorer in scorer tree (opensearch-project#20607)
* Fix ProfileScorer.getChildren() to expose wrapped scorer in scorer tree Signed-off-by: Martin Gaievski <gaievski@amazon.com> * Changed approach to use new ScoreWrapper interface Signed-off-by: Martin Gaievski <gaievski@amazon.com> * Renamed interface to WrappedScorerAccessor, added unit test Signed-off-by: Martin Gaievski <gaievski@amazon.com> * Improving tests, corrected changelog Signed-off-by: Martin Gaievski <gaievski@amazon.com> * Changed to generic interface ProfilingWrapper Signed-off-by: Martin Gaievski <gaievski@amazon.com> * Adding ProfilingWrapper to ProfilingAggregator and ProfilingLeafBucketCollector Signed-off-by: Martin Gaievski <gaievski@amazon.com> --------- Signed-off-by: Martin Gaievski <gaievski@amazon.com>
1 parent d8d2a00 commit 14749f7

File tree

8 files changed

+180
-10
lines changed

8 files changed

+180
-10
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55

66
## [Unreleased 3.x]
77
### Added
8-
- Add getWrappedScorer method to ProfileScorer for plugin access to wrapped scorers ([#20548](https://github.com/opensearch-project/OpenSearch/issues/20548))
8+
- Add ProfilingWrapper interface for plugin access to delegates in profiling decorators ([#20607](https://github.com/opensearch-project/OpenSearch/pull/20607))
99
- Support expected cluster name with validation in CCS Sniff mode ([#20532](https://github.com/opensearch-project/OpenSearch/pull/20532))
1010
- Choose the best performing node when writing with append-only index ([#20065](https://github.com/opensearch-project/OpenSearch/pull/20065))
1111
- Add security policy to allow `accessUnixDomainSocket` in `transport-grpc` module ([#20463](https://github.com/opensearch-project/OpenSearch/pull/20463), [#20649](https://github.com/opensearch-project/OpenSearch/pull/20649))
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.search.profile;
10+
11+
import org.opensearch.common.annotation.PublicApi;
12+
13+
/**
14+
* Generic interface for profiling wrappers that decorate search components with timing instrumentation
15+
* while preserving access to the original (delegate) component.
16+
* <p>
17+
* When search profiling is enabled, OpenSearch wraps scorers, collectors, aggregators, and other components
18+
* in profiling decorators that are package-private. This interface provides a public contract for plugins
19+
* to detect and unwrap such wrappers, accessing the underlying component without requiring reflection
20+
* or knowledge of package-private implementation classes.
21+
* <p>
22+
* Example usage in a plugin to unwrap a profiled scorer:
23+
* <pre>{@code
24+
* if (scorer instanceof ProfilingWrapper) {
25+
* Scorer delegate = ((ProfilingWrapper<Scorer>) scorer).getDelegate();
26+
* // access custom scorer methods on the delegate
27+
* }
28+
* }</pre>
29+
*
30+
* @param <T> the type of the wrapped component (e.g., {@link org.apache.lucene.search.Scorer},
31+
* {@link org.apache.lucene.search.Collector},
32+
* {@link org.opensearch.search.aggregations.Aggregator})
33+
* @opensearch.api
34+
*/
35+
@PublicApi(since = "3.6.0")
36+
public interface ProfilingWrapper<T> {
37+
38+
/**
39+
* Returns the underlying delegate component that this profiling wrapper decorates.
40+
*
41+
* @return the original component that was wrapped for profiling
42+
*/
43+
T getDelegate();
44+
}

server/src/main/java/org/opensearch/search/profile/aggregation/ProfilingAggregator.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.opensearch.search.aggregations.LeafBucketCollector;
4040
import org.opensearch.search.aggregations.support.AggregationPath.PathElement;
4141
import org.opensearch.search.internal.SearchContext;
42+
import org.opensearch.search.profile.ProfilingWrapper;
4243
import org.opensearch.search.profile.Timer;
4344
import org.opensearch.search.sort.SortOrder;
4445

@@ -48,7 +49,7 @@
4849
/**
4950
* An aggregator that aggregates the performance profiling of other aggregations
5051
*/
51-
public class ProfilingAggregator extends Aggregator {
52+
public class ProfilingAggregator extends Aggregator implements ProfilingWrapper<Aggregator> {
5253

5354
private final Aggregator delegate;
5455
private final AggregationProfiler profiler;
@@ -162,6 +163,11 @@ public String toString() {
162163
return delegate.toString();
163164
}
164165

166+
@Override
167+
public Aggregator getDelegate() {
168+
return delegate;
169+
}
170+
165171
@Override
166172
public Aggregator unwrapAggregator() {
167173
return delegate;

server/src/main/java/org/opensearch/search/profile/aggregation/ProfilingLeafBucketCollector.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.apache.lucene.search.Scorable;
3636
import org.opensearch.search.aggregations.LeafBucketCollector;
37+
import org.opensearch.search.profile.ProfilingWrapper;
3738
import org.opensearch.search.profile.Timer;
3839

3940
import java.io.IOException;
@@ -43,9 +44,9 @@
4344
*
4445
* @opensearch.internal
4546
*/
46-
public class ProfilingLeafBucketCollector extends LeafBucketCollector {
47+
public class ProfilingLeafBucketCollector extends LeafBucketCollector implements ProfilingWrapper<LeafBucketCollector> {
4748

48-
private LeafBucketCollector delegate;
49+
private final LeafBucketCollector delegate;
4950
private Timer collectTimer;
5051

5152
public ProfilingLeafBucketCollector(LeafBucketCollector delegate, AggregationProfileBreakdown profileBreakdown) {
@@ -63,6 +64,11 @@ public void collect(int doc, long bucket) throws IOException {
6364
}
6465
}
6566

67+
@Override
68+
public LeafBucketCollector getDelegate() {
69+
return delegate;
70+
}
71+
6672
@Override
6773
public void setScorer(Scorable scorer) throws IOException {
6874
delegate.setScorer(scorer);

server/src/main/java/org/opensearch/search/profile/query/ProfileCollector.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.apache.lucene.search.LeafCollector;
4040
import org.apache.lucene.search.Scorable;
4141
import org.apache.lucene.search.ScoreMode;
42+
import org.opensearch.search.profile.ProfilingWrapper;
4243

4344
import java.io.IOException;
4445

@@ -47,7 +48,7 @@
4748
*
4849
* @opensearch.internal
4950
*/
50-
final class ProfileCollector extends FilterCollector {
51+
final class ProfileCollector extends FilterCollector implements ProfilingWrapper<Collector> {
5152

5253
private long time;
5354
private long sliceStartTime;

server/src/main/java/org/opensearch/search/profile/query/ProfileScorer.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.lucene.search.Scorer;
3737
import org.apache.lucene.search.TwoPhaseIterator;
3838
import org.opensearch.search.profile.AbstractProfileBreakdown;
39+
import org.opensearch.search.profile.ProfilingWrapper;
3940
import org.opensearch.search.profile.Timer;
4041

4142
import java.io.IOException;
@@ -47,7 +48,7 @@
4748
*
4849
* @opensearch.internal
4950
*/
50-
final class ProfileScorer extends Scorer {
51+
final class ProfileScorer extends Scorer implements ProfilingWrapper<Scorer> {
5152

5253
private final Scorer scorer;
5354

@@ -81,7 +82,8 @@ final class ProfileScorer extends Scorer {
8182
* @return the underlying wrapped scorer
8283
* @see ProfileCollector#getDelegate()
8384
*/
84-
public Scorer getWrappedScorer() {
85+
@Override
86+
public Scorer getDelegate() {
8587
return scorer;
8688
}
8789

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.search.profile.aggregation;
10+
11+
import org.opensearch.search.aggregations.Aggregator;
12+
import org.opensearch.search.aggregations.LeafBucketCollector;
13+
import org.opensearch.search.profile.ProfilingWrapper;
14+
import org.opensearch.test.OpenSearchTestCase;
15+
16+
import static org.mockito.Mockito.mock;
17+
import static org.mockito.Mockito.when;
18+
19+
/**
20+
* Tests that ProfilingAggregator and ProfilingLeafBucketCollector
21+
* correctly implement the ProfilingWrapper interface.
22+
*/
23+
public class ProfilingWrapperAggregationTests extends OpenSearchTestCase {
24+
25+
public void testProfilingAggregatorImplementsProfilingWrapper() {
26+
Aggregator mockAggregator = mock(Aggregator.class);
27+
when(mockAggregator.name()).thenReturn("test_agg");
28+
AggregationProfiler profiler = new AggregationProfiler();
29+
30+
ProfilingAggregator profilingAggregator = new ProfilingAggregator(mockAggregator, profiler);
31+
32+
// Verify implements ProfilingWrapper
33+
assertTrue("ProfilingAggregator should implement ProfilingWrapper", profilingAggregator instanceof ProfilingWrapper);
34+
35+
// Verify getDelegate returns the original aggregator
36+
assertSame("getDelegate() should return the original aggregator", mockAggregator, profilingAggregator.getDelegate());
37+
}
38+
39+
public void testProfilingLeafBucketCollectorImplementsProfilingWrapper() {
40+
LeafBucketCollector mockCollector = mock(LeafBucketCollector.class);
41+
AggregationProfileBreakdown profileBreakdown = new AggregationProfileBreakdown();
42+
43+
ProfilingLeafBucketCollector profilingCollector = new ProfilingLeafBucketCollector(mockCollector, profileBreakdown);
44+
45+
// Verify implements ProfilingWrapper
46+
assertTrue("ProfilingLeafBucketCollector should implement ProfilingWrapper", profilingCollector instanceof ProfilingWrapper);
47+
48+
// Verify getDelegate returns the original collector
49+
assertSame("getDelegate() should return the original collector", mockCollector, profilingCollector.getDelegate());
50+
}
51+
}

server/src/test/java/org/opensearch/search/profile/query/ProfileScorerTests.java

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,16 @@
3737
import org.apache.lucene.search.IndexSearcher;
3838
import org.apache.lucene.search.MatchAllDocsQuery;
3939
import org.apache.lucene.search.Query;
40+
import org.apache.lucene.search.Scorable;
4041
import org.apache.lucene.search.ScoreMode;
4142
import org.apache.lucene.search.Scorer;
4243
import org.apache.lucene.search.Weight;
4344
import org.opensearch.search.profile.ProfileMetricUtil;
45+
import org.opensearch.search.profile.ProfilingWrapper;
4446
import org.opensearch.test.OpenSearchTestCase;
4547

4648
import java.io.IOException;
49+
import java.util.Collection;
4750

4851
public class ProfileScorerTests extends OpenSearchTestCase {
4952

@@ -102,14 +105,71 @@ public void testPropagateMaxScore() throws IOException {
102105
assertEquals(42f, profileScorer.getMaxScore(DocIdSetIterator.NO_MORE_DOCS), 0f);
103106
}
104107

105-
public void testGetWrappedScorer() throws IOException {
108+
public void testGetDelegate() throws IOException {
106109
Query query = new MatchAllDocsQuery();
107110
Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f);
108111
FakeScorer fakeScorer = new FakeScorer(weight);
109112
QueryProfileBreakdown profile = new QueryProfileBreakdown(ProfileMetricUtil.getDefaultQueryProfileMetrics());
110113
ProfileScorer profileScorer = new ProfileScorer(fakeScorer, profile);
111114

112-
// Verify getWrappedScorer returns the original scorer
113-
assertSame(fakeScorer, profileScorer.getWrappedScorer());
115+
// Verify getDelegate returns the original scorer
116+
assertSame(fakeScorer, profileScorer.getDelegate());
117+
}
118+
119+
@SuppressWarnings("unchecked")
120+
public void testImplementsProfilingWrapper() throws IOException {
121+
Query query = new MatchAllDocsQuery();
122+
Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f);
123+
FakeScorer fakeScorer = new FakeScorer(weight);
124+
QueryProfileBreakdown profile = new QueryProfileBreakdown(ProfileMetricUtil.getDefaultQueryProfileMetrics());
125+
ProfileScorer profileScorer = new ProfileScorer(fakeScorer, profile);
126+
127+
// ProfileScorer implements ProfilingWrapper<Scorer>, allowing plugins to detect and unwrap
128+
// profiling wrappers using instanceof without reflection
129+
assertTrue("ProfileScorer should implement ProfilingWrapper", profileScorer instanceof ProfilingWrapper);
130+
ProfilingWrapper<Scorer> wrapper = (ProfilingWrapper<Scorer>) profileScorer;
131+
assertSame("ProfilingWrapper.getDelegate() should return the original scorer", fakeScorer, wrapper.getDelegate());
132+
}
133+
134+
@SuppressWarnings("unchecked")
135+
public void testUnwrapNestedProfilingScorerFromScorableReference() throws IOException {
136+
// Simulate nested profiling: a scorer wrapped in two ProfileScorer layers.
137+
// This tests that a plugin can iteratively unwrap through multiple profiling
138+
// layers via a generic Scorable reference (as received in LeafCollector.setScorer())
139+
Query query = new MatchAllDocsQuery();
140+
Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f);
141+
FakeScorer fakeScorer = new FakeScorer(weight);
142+
QueryProfileBreakdown innerProfile = new QueryProfileBreakdown(ProfileMetricUtil.getDefaultQueryProfileMetrics());
143+
QueryProfileBreakdown outerProfile = new QueryProfileBreakdown(ProfileMetricUtil.getDefaultQueryProfileMetrics());
144+
145+
// Create nested profiling: FakeScorer -> innerProfileScorer -> outerProfileScorer
146+
ProfileScorer innerProfileScorer = new ProfileScorer(fakeScorer, innerProfile);
147+
ProfileScorer outerProfileScorer = new ProfileScorer(innerProfileScorer, outerProfile);
148+
149+
// Plugin only sees a Scorable reference
150+
Scorable scorable = outerProfileScorer;
151+
152+
// Unwrap first layer
153+
assertTrue("Outer Scorable should be detected as ProfilingWrapper", scorable instanceof ProfilingWrapper);
154+
Scorer firstUnwrap = ((ProfilingWrapper<Scorer>) scorable).getDelegate();
155+
assertNotSame("First unwrap should not be the original scorer", fakeScorer, firstUnwrap);
156+
assertTrue("First unwrap should still be a ProfilingWrapper (inner ProfileScorer)", firstUnwrap instanceof ProfilingWrapper);
157+
158+
// Unwrap second layer to reach the original scorer
159+
Scorer secondUnwrap = ((ProfilingWrapper<Scorer>) firstUnwrap).getDelegate();
160+
assertSame("Second unwrap should be the original FakeScorer", fakeScorer, secondUnwrap);
161+
assertFalse("Original scorer should not implement ProfilingWrapper", secondUnwrap instanceof ProfilingWrapper);
162+
}
163+
164+
public void testGetChildren_delegatesToWrappedScorer() throws IOException {
165+
Query query = new MatchAllDocsQuery();
166+
Weight weight = query.createWeight(new IndexSearcher(new MultiReader()), ScoreMode.TOP_SCORES, 1f);
167+
FakeScorer fakeScorer = new FakeScorer(weight);
168+
QueryProfileBreakdown profile = new QueryProfileBreakdown(ProfileMetricUtil.getDefaultQueryProfileMetrics());
169+
ProfileScorer profileScorer = new ProfileScorer(fakeScorer, profile);
170+
171+
// getChildren() should delegate to the wrapped scorer's getChildren()
172+
Collection<Scorer.ChildScorable> children = profileScorer.getChildren();
173+
assertEquals("FakeScorer has no children, so ProfileScorer should return empty collection", 0, children.size());
114174
}
115175
}

0 commit comments

Comments
 (0)