Skip to content

Commit

Permalink
Properly encapsulate SearchRequestOperationsListener related APIs as …
Browse files Browse the repository at this point in the history
…package protected (internal)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
  • Loading branch information
reta committed Nov 23, 2023
1 parent fe2d585 commit fdc183c
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,35 +20,35 @@
* @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) {}

Check warning on line 31 in server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java#L31

Added line #L31 was not covered by tests

default void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {}
void onRequestEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {}

Check warning on line 33 in server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestOperationsListener.java#L33

Added line #L33 was not covered by tests

/**
* Holder of Composite Listeners
*
* @opensearch.internal
*/

final class CompositeListener implements SearchRequestOperationsListener {
static final class CompositeListener extends SearchRequestOperationsListener {
private final List<SearchRequestOperationsListener> listeners;
private final Logger logger;

public CompositeListener(List<SearchRequestOperationsListener> listeners, Logger logger) {
CompositeListener(List<SearchRequestOperationsListener> 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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -134,19 +134,19 @@ public SearchRequestSlowLog(ClusterService clusterService) {
}

@Override
public void onPhaseStart(SearchPhaseContext context) {}
void onPhaseStart(SearchPhaseContext context) {}

Check warning on line 137 in server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java#L137

Added line #L137 was not covered by tests

@Override
public void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {}
void onPhaseEnd(SearchPhaseContext context, SearchRequestContext searchRequestContext) {}

Check warning on line 140 in server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java#L140

Added line #L140 was not covered by tests

@Override
public void onPhaseFailure(SearchPhaseContext context) {}
void onPhaseFailure(SearchPhaseContext context) {}

Check warning on line 143 in server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java#L143

Added line #L143 was not covered by tests

@Override
public void onRequestStart(SearchRequestContext searchRequestContext) {}
void onRequestStart(SearchRequestContext searchRequestContext) {}

Check warning on line 146 in server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/search/SearchRequestSlowLog.java#L146

Added line #L146 was not covered by tests

@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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* @opensearch.api
*/
@PublicApi(since = "2.11.0")
public final class SearchRequestStats implements SearchRequestOperationsListener {
public final class SearchRequestStats extends SearchRequestOperationsListener {
Map<SearchPhaseName, StatsHolder> phaseStatsMap = new EnumMap<>(SearchPhaseName.class);

@Inject
Expand All @@ -46,20 +46,20 @@ 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();
phaseStats.timing.inc(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - context.getCurrentPhase().getStartTimeInNanos()));
}

@Override
public void onPhaseFailure(SearchPhaseContext context) {
void onPhaseFailure(SearchPhaseContext context) {
phaseStatsMap.get(context.getCurrentPhase().getSearchPhaseName()).current.dec();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ private Map<String, Float> 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;
Expand Down Expand Up @@ -352,18 +352,18 @@ SearchResponse.PhaseTook getPhaseTook() {
Map<SearchPhaseName, Long> 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())
);
}

@Override
public void onPhaseFailure(SearchPhaseContext context) {}
void onPhaseFailure(SearchPhaseContext context) {}

public Long getPhaseTookTime(SearchPhaseName searchPhaseName) {
return phaseStatsMap.get(searchPhaseName);
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit fdc183c

Please sign in to comment.