From 76c8bf9478e1945ba45d777c79a5499d9c0367ea Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 27 Nov 2023 14:05:16 +0000 Subject: [PATCH] Properly encapsulate SearchRequestOperationsListener related APIs as package protected (internal) (#11315) Signed-off-by: Andriy Redko (cherry picked from commit 6d2cf13295dc4578f5f7fff687bbf52e15149a60) Signed-off-by: github-actions[bot] --- .../SearchRequestOperationsListener.java | 24 +++++++++---------- .../action/search/SearchRequestSlowLog.java | 12 +++++----- .../action/search/SearchRequestStats.java | 8 +++---- .../action/search/TransportSearchAction.java | 8 +++---- ...earchRequestOperationsListenerSupport.java | 22 +++++++++++++++++ .../index/search/stats/SearchStatsTests.java | 7 +++--- 6 files changed, 52 insertions(+), 29 deletions(-) create mode 100644 server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerSupport.java diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java index 056cb474eaf32..19ce0beb3c493 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java @@ -20,17 +20,17 @@ * @opensearch.internal */ @InternalApi -interface SearchRequestOperationsListener { +abstract class SearchRequestOperationsListener { - void onPhaseStart(SearchPhaseContext context); + abstract void onPhaseStart(SearchPhaseContext context); - void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext); + abstract void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext); - void onPhaseFailure(SearchPhaseContext context); + abstract void onPhaseFailure(SearchPhaseContext context); - default void onRequestStart(SearchRequestContext searchRequestContext) {} + void onRequestStart(SearchRequestContext searchRequestContext) {} - default void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} /** * Holder of Composite Listeners @@ -38,17 +38,17 @@ default void onRequestEnd(SearchPhaseContext context, SearchRequestContext searc * @opensearch.internal */ - final class CompositeListener implements SearchRequestOperationsListener { + static final class CompositeListener extends SearchRequestOperationsListener { private final List listeners; private final Logger logger; - public CompositeListener(List listeners, Logger logger) { + CompositeListener(List listeners, Logger logger) { this.listeners = listeners; this.logger = logger; } @Override - public void onPhaseStart(SearchPhaseContext context) { + void onPhaseStart(SearchPhaseContext context) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseStart(context); @@ -59,7 +59,7 @@ public void onPhaseStart(SearchPhaseContext context) { } @Override - public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseEnd(context, searchRequestContext); @@ -70,7 +70,7 @@ public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRe } @Override - public void onPhaseFailure(SearchPhaseContext context) { + void onPhaseFailure(SearchPhaseContext context) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onPhaseFailure(context); @@ -81,7 +81,7 @@ public void onPhaseFailure(SearchPhaseContext context) { } @Override - public void onRequestStart(SearchRequestContext searchRequestContext) { + void onRequestStart(SearchRequestContext searchRequestContext) { for (SearchRequestOperationsListener listener : listeners) { try { listener.onRequestStart(searchRequestContext); diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java b/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java index 6a0d60ffd3984..a55cfd463a78f 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java @@ -57,7 +57,7 @@ * * @opensearch.internal */ -public final class SearchRequestSlowLog implements SearchRequestOperationsListener { +public final class SearchRequestSlowLog extends SearchRequestOperationsListener { private static final Charset UTF_8 = StandardCharsets.UTF_8; private long warnThreshold; @@ -134,19 +134,19 @@ public SearchRequestSlowLog(ClusterService clusterService) { } @Override - public void onPhaseStart(SearchPhaseContext context) {} + void onPhaseStart(SearchPhaseContext context) {} @Override - public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} + void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {} @Override - public void onPhaseFailure(SearchPhaseContext context) {} + void onPhaseFailure(SearchPhaseContext context) {} @Override - public void onRequestStart(SearchRequestContext searchRequestContext) {} + void onRequestStart(SearchRequestContext searchRequestContext) {} @Override - public void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { long tookInNanos = System.nanoTime() - searchRequestContext.getAbsoluteStartNanos(); if (warnThreshold >= 0 && tookInNanos > warnThreshold && level.isLevelEnabledFor(SlowLogLevel.WARN)) { diff --git a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java index 2813c41e043ee..262750849eaa9 100644 --- a/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java +++ b/server/src/main/java/org/opensearch/action/search/SearchRequestStats.java @@ -23,7 +23,7 @@ * @opensearch.api */ @PublicApi(since = "2.11.0") -public final class SearchRequestStats implements SearchRequestOperationsListener { +public final class SearchRequestStats extends SearchRequestOperationsListener { Map phaseStatsMap = new EnumMap<>(SearchPhaseName.class); @Inject @@ -46,12 +46,12 @@ public long getPhaseMetric(SearchPhaseName searchPhaseName) { } @Override - public void onPhaseStart(SearchPhaseContext context) { + void onPhaseStart(SearchPhaseContext context) { phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.inc(); } @Override - public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { StatsHolder phaseStats = phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()); phaseStats.current.dec(); phaseStats.total.inc(); @@ -59,7 +59,7 @@ public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRe } @Override - public void onPhaseFailure(SearchPhaseContext context) { + void onPhaseFailure(SearchPhaseContext context) { phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec(); } diff --git a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java index 62886f7e9d981..05f4308df74fa 100644 --- a/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/opensearch/action/search/TransportSearchAction.java @@ -300,7 +300,7 @@ private Map resolveIndexBoosts(SearchRequest searchRequest, Clust * * @opensearch.internal */ - static final class SearchTimeProvider implements SearchRequestOperationsListener { + static final class SearchTimeProvider extends SearchRequestOperationsListener { private final long absoluteStartMillis; private final long relativeStartNanos; @@ -352,10 +352,10 @@ SearchResponse.PhaseTook getPhaseTook() { Map phaseStatsMap = new EnumMap<>(SearchPhaseName.class); @Override - public void onPhaseStart(SearchPhaseContext context) {} + void onPhaseStart(SearchPhaseContext context) {} @Override - public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { + void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) { phaseStatsMap.put( context.getCurrentPhase().getSearchPhaseName(), TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - context.getCurrentPhase().getStartTimeInNanos()) @@ -363,7 +363,7 @@ public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRe } @Override - public void onPhaseFailure(SearchPhaseContext context) {} + void onPhaseFailure(SearchPhaseContext context) {} public Long getPhaseTookTime(SearchPhaseName searchPhaseName) { return phaseStatsMap.get(searchPhaseName); diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerSupport.java b/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerSupport.java new file mode 100644 index 0000000000000..58a4c4a4e555d --- /dev/null +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestOperationsListenerSupport.java @@ -0,0 +1,22 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.search; + +/** + * Helper interface to access package protected {@link SearchRequestOperationsListener} from test cases. + */ +public interface SearchRequestOperationsListenerSupport { + default void onPhaseStart(SearchRequestOperationsListener listener, SearchPhaseContext context) { + listener.onPhaseStart(context); + } + + default void onPhaseEnd(SearchRequestOperationsListener listener, SearchPhaseContext context) { + listener.onPhaseEnd(context, new SearchRequestContext()); + } +} diff --git a/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java b/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java index 98c7b8e4b2bde..52b272094cd86 100644 --- a/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java +++ b/server/src/test/java/org/opensearch/index/search/stats/SearchStatsTests.java @@ -35,6 +35,7 @@ import org.opensearch.action.search.SearchPhase; import org.opensearch.action.search.SearchPhaseContext; import org.opensearch.action.search.SearchPhaseName; +import org.opensearch.action.search.SearchRequestOperationsListenerSupport; import org.opensearch.action.search.SearchRequestStats; import org.opensearch.index.search.stats.SearchStats.Stats; import org.opensearch.test.OpenSearchTestCase; @@ -47,7 +48,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class SearchStatsTests extends OpenSearchTestCase { +public class SearchStatsTests extends OpenSearchTestCase implements SearchRequestOperationsListenerSupport { // https://github.com/elastic/elasticsearch/issues/7644 public void testShardLevelSearchGroupStats() throws Exception { @@ -84,8 +85,8 @@ public void testShardLevelSearchGroupStats() throws Exception { when(mockSearchPhase.getStartTimeInNanos()).thenReturn(System.nanoTime() - TimeUnit.SECONDS.toNanos(paramValue)); when(mockSearchPhase.getSearchPhaseName()).thenReturn(searchPhaseName); for (int iterator = 0; iterator < paramValue; iterator++) { - testRequestStats.onPhaseStart(ctx); - testRequestStats.onPhaseEnd(ctx, null /* not needed */); + onPhaseStart(testRequestStats, ctx); + onPhaseEnd(testRequestStats, ctx); } } searchStats1.setSearchRequestStats(testRequestStats);