From a7ab5d4fd1223126f3877417a640a99b27c654f8 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Tue, 3 Dec 2024 16:10:52 +0100 Subject: [PATCH 1/8] feat: implemented stats and counters for total requests, error requests and cache. Signed-off-by: Johannes Peter --- .../org/opensearch/ltr/stats/LTRStat.java | 68 ++++++++++++++ .../org/opensearch/ltr/stats/LTRStats.java | 93 +++++++++++++++++++ .../org/opensearch/ltr/stats/StatName.java | 44 +++++++++ .../suppliers/CacheStatsOnNodeSupplier.java | 60 ++++++++++++ .../ltr/stats/suppliers/CounterSupplier.java | 45 +++++++++ .../opensearch/ltr/stats/LTRStatTests.java | 59 ++++++++++++ .../opensearch/ltr/stats/LTRStatsTests.java | 90 ++++++++++++++++++ .../CacheStatsOnNodeSupplierTests.java | 90 ++++++++++++++++++ .../stats/suppliers/CounterSupplierTests.java | 29 ++++++ 9 files changed, 578 insertions(+) create mode 100644 src/main/java/org/opensearch/ltr/stats/LTRStat.java create mode 100644 src/main/java/org/opensearch/ltr/stats/LTRStats.java create mode 100644 src/main/java/org/opensearch/ltr/stats/StatName.java create mode 100644 src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java create mode 100644 src/main/java/org/opensearch/ltr/stats/suppliers/CounterSupplier.java create mode 100644 src/test/java/org/opensearch/ltr/stats/LTRStatTests.java create mode 100644 src/test/java/org/opensearch/ltr/stats/LTRStatsTests.java create mode 100644 src/test/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplierTests.java create mode 100644 src/test/java/org/opensearch/ltr/stats/suppliers/CounterSupplierTests.java diff --git a/src/main/java/org/opensearch/ltr/stats/LTRStat.java b/src/main/java/org/opensearch/ltr/stats/LTRStat.java new file mode 100644 index 0000000..56c57d7 --- /dev/null +++ b/src/main/java/org/opensearch/ltr/stats/LTRStat.java @@ -0,0 +1,68 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats; + +import org.opensearch.ltr.stats.suppliers.CounterSupplier; + +import java.util.function.Supplier; + +/** + * Class represents a stat the plugin keeps track of + */ +public class LTRStat { + private Boolean clusterLevel; + private Supplier supplier; + + /** + * Constructor + * + * @param clusterLevel whether the stat has clusterLevel scope or nodeLevel scope + * @param supplier supplier that returns the stat's value + */ + public LTRStat(Boolean clusterLevel, Supplier supplier) { + this.clusterLevel = clusterLevel; + this.supplier = supplier; + } + + /** + * Determines whether the stat is cluster specific or node specific + * + * @return true is stat is cluster level; false otherwise + */ + public Boolean isClusterLevel() { + return clusterLevel; + } + + /** + * Get the value of the statistic + * + * @return T value of the stat + */ + public T getValue() { + return supplier.get(); + } + + /** + * Increments the supplier if it can be incremented + */ + public void increment() { + if (!(supplier instanceof CounterSupplier)) { + throw new UnsupportedOperationException( + "cannot increment the supplier: " + supplier.getClass().getName()); + } + ((CounterSupplier) supplier).increment(); + } +} diff --git a/src/main/java/org/opensearch/ltr/stats/LTRStats.java b/src/main/java/org/opensearch/ltr/stats/LTRStats.java new file mode 100644 index 0000000..0e21d80 --- /dev/null +++ b/src/main/java/org/opensearch/ltr/stats/LTRStats.java @@ -0,0 +1,93 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats; + +import java.util.Map; +import java.util.stream.Collectors; + +/** + * This class is the main entry-point for access to the stats that the LTR plugin keeps track of. + */ +public class LTRStats { + private Map> stats; + + /** + * Constructor + * + * @param stats Map of the stats that are to be kept + */ + public LTRStats(Map> stats) { + this.stats = stats; + } + + /** + * Get the stats + * + * @return all of the stats + */ + public Map> getStats() { + return stats; + } + + /** + * Get individual stat by stat name + * + * @param key Name of stat + * @return LTRStat + * @throws IllegalArgumentException thrown on illegal statName + */ + public LTRStat getStat(String key) throws IllegalArgumentException { + if (!stats.containsKey(key)) { + throw new IllegalArgumentException("Stat=\"" + key + "\" does not exist"); + } + return stats.get(key); + } + + /** + * Add specific stat to stats map + * @param key stat name + * @param stat Stat + */ + public void addStats(String key, LTRStat stat) { + this.stats.put(key, stat); + } + + + /** + * Get a map of the stats that are kept at the node level + * + * @return Map of stats kept at the node level + */ + public Map> getNodeStats() { + return getClusterOrNodeStats(false); + } + + /** + * Get a map of the stats that are kept at the cluster level + * + * @return Map of stats kept at the cluster level + */ + public Map> getClusterStats() { + return getClusterOrNodeStats(true); + } + + private Map> getClusterOrNodeStats(Boolean isClusterStats) { + return stats.entrySet() + .stream() + .filter(e -> e.getValue().isClusterLevel() == isClusterStats) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } +} diff --git a/src/main/java/org/opensearch/ltr/stats/StatName.java b/src/main/java/org/opensearch/ltr/stats/StatName.java new file mode 100644 index 0000000..d04bbd0 --- /dev/null +++ b/src/main/java/org/opensearch/ltr/stats/StatName.java @@ -0,0 +1,44 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats; + +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + +public enum StatName { + LTR_PLUGIN_STATUS("status"), + LTR_STORES_STATS("stores"), + LTR_REQUEST_TOTAL_COUNT("request_total_count"), + LTR_REQUEST_ERROR_COUNT("request_error_count"), + LTR_CACHE_STATS("cache"); + + private String name; + + StatName(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public static Set getNames() { + return Arrays.stream(StatName.values()) + .map(StatName::getName) + .collect(Collectors.toSet()); + } +} diff --git a/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java b/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java new file mode 100644 index 0000000..4d54f0a --- /dev/null +++ b/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java @@ -0,0 +1,60 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats.suppliers; + +import com.o19s.es.ltr.feature.store.index.Caches; +import org.opensearch.common.cache.Cache; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Supplier; + +/** + * Aggregate stats on the cache used by the plugin per node. + */ +public class CacheStatsOnNodeSupplier implements Supplier>> { + private static final String LTR_CACHE_HIT_COUNT = "hit_count"; + private static final String LTR_CACHE_MISS_COUNT = "miss_count"; + private static final String LTR_CACHE_EVICTION_COUNT = "eviction_count"; + private static final String LTR_CACHE_ENTRY_COUNT = "entry_count"; + private static final String LTR_CACHE_MEMORY_USAGE_IN_BYTES = "memory_usage_in_bytes"; + + private Caches caches; + + public CacheStatsOnNodeSupplier(Caches caches) { + this.caches = caches; + } + + @Override + public Map> get() { + Map> values = new HashMap<>(); + values.put("feature", getCacheStats(caches.featureCache())); + values.put("featureset", getCacheStats(caches.featureSetCache())); + values.put("model", getCacheStats(caches.modelCache())); + return Collections.unmodifiableMap(values); + } + + private Map getCacheStats(Cache cache) { + Map stat = new HashMap<>(); + stat.put(LTR_CACHE_HIT_COUNT, cache.stats().getHits()); + stat.put(LTR_CACHE_MISS_COUNT, cache.stats().getMisses()); + stat.put(LTR_CACHE_EVICTION_COUNT, cache.stats().getEvictions()); + stat.put(LTR_CACHE_ENTRY_COUNT, cache.count()); + stat.put(LTR_CACHE_MEMORY_USAGE_IN_BYTES, cache.weight()); + return Collections.unmodifiableMap(stat); + } +} diff --git a/src/main/java/org/opensearch/ltr/stats/suppliers/CounterSupplier.java b/src/main/java/org/opensearch/ltr/stats/suppliers/CounterSupplier.java new file mode 100644 index 0000000..68fd6ae --- /dev/null +++ b/src/main/java/org/opensearch/ltr/stats/suppliers/CounterSupplier.java @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats.suppliers; + +import java.util.concurrent.atomic.LongAdder; +import java.util.function.Supplier; + +/** + * CounterSupplier provides a stateful count as the value + */ +public class CounterSupplier implements Supplier { + private LongAdder counter; + + /** + * Constructor + */ + public CounterSupplier() { + this.counter = new LongAdder(); + } + + @Override + public Long get() { + return counter.longValue(); + } + + /** + * Increments the value of the counter by 1 + */ + public void increment() { + counter.increment(); + } +} diff --git a/src/test/java/org/opensearch/ltr/stats/LTRStatTests.java b/src/test/java/org/opensearch/ltr/stats/LTRStatTests.java new file mode 100644 index 0000000..f94c635 --- /dev/null +++ b/src/test/java/org/opensearch/ltr/stats/LTRStatTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats; + +import org.junit.Test; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class LTRStatTests { + @Test + public void testIsClusterLevel() { + LTRStat stat1 = new LTRStat<>(true, () -> "test"); + assertTrue(stat1.isClusterLevel()); + + LTRStat stat2 = new LTRStat<>(false, () -> "test"); + assertFalse(stat2.isClusterLevel()); + } + + @Test + public void testGetValue() { + LTRStat stat1 = new LTRStat<>(false, new CounterSupplier()); + assertEquals(0L, stat1.getValue().longValue()); + + LTRStat stat2 = new LTRStat<>(false, () -> "test"); + assertEquals("test", stat2.getValue()); + } + + @Test + public void testIncrementCounterSupplier() { + LTRStat incrementStat = new LTRStat<>(false, new CounterSupplier()); + + for (long i = 0L; i < 100; i++) { + assertEquals(i, incrementStat.getValue().longValue()); + incrementStat.increment(); + } + } + + @Test(expected = UnsupportedOperationException.class) + public void testThrowExceptionIncrementNonCounterSupplier(){ + LTRStat nonIncStat = new LTRStat<>(false, () -> "test"); + nonIncStat.increment(); + } +} diff --git a/src/test/java/org/opensearch/ltr/stats/LTRStatsTests.java b/src/test/java/org/opensearch/ltr/stats/LTRStatsTests.java new file mode 100644 index 0000000..47eec70 --- /dev/null +++ b/src/test/java/org/opensearch/ltr/stats/LTRStatsTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats; + +import org.junit.Before; +import org.junit.Test; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +public class LTRStatsTests { + + private Map> statsMap; + private LTRStats ltrStats; + + @Before + public void setUp() { + statsMap = new HashMap<>(); + statsMap.put(StatName.LTR_PLUGIN_STATUS.getName(), new LTRStat<>(true, () -> "test")); + statsMap.put(StatName.LTR_CACHE_STATS.getName(), new LTRStat<>(false, () -> "test")); + ltrStats = new LTRStats(statsMap); + } + + @Test + public void testStatNamesGetNames() { + assertEquals(StatName.getNames().size(), StatName.values().length); + } + + @Test + public void testGetStats() { + Map> stats = ltrStats.getStats(); + assertEquals(stats.size(), statsMap.size()); + + for (Map.Entry> stat : stats.entrySet()) { + assertStatPresence(stat.getKey(), stat.getValue()); + } + } + + @Test + public void testGetStat() { + LTRStat stat = ltrStats.getStat(StatName.LTR_PLUGIN_STATUS.getName()); + assertStatPresence(StatName.LTR_PLUGIN_STATUS.getName(), stat); + } + + private void assertStatPresence(String statName, LTRStat stat) { + assertTrue(ltrStats.getStats().containsKey(statName)); + assertSame(ltrStats.getStats().get(statName), stat); + } + + @Test + public void testGetNodeStats() { + Map> stats = ltrStats.getStats(); + Set> nodeStats = new HashSet<>(ltrStats.getNodeStats().values()); + + for (LTRStat stat : stats.values()) { + assertTrue((stat.isClusterLevel() && !nodeStats.contains(stat)) || + (!stat.isClusterLevel() && nodeStats.contains(stat))); + } + } + + @Test + public void testGetClusterStats() { + Map> stats = ltrStats.getStats(); + Set> clusterStats = new HashSet<>(ltrStats.getClusterStats().values()); + + for (LTRStat stat : stats.values()) { + assertTrue((stat.isClusterLevel() && clusterStats.contains(stat)) || + (!stat.isClusterLevel() && !clusterStats.contains(stat))); + } + } +} diff --git a/src/test/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplierTests.java b/src/test/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplierTests.java new file mode 100644 index 0000000..7b701c0 --- /dev/null +++ b/src/test/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplierTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats.suppliers; + +import com.o19s.es.ltr.feature.Feature; +import com.o19s.es.ltr.feature.FeatureSet; +import com.o19s.es.ltr.feature.store.CompiledLtrModel; +import com.o19s.es.ltr.feature.store.index.Caches; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opensearch.common.cache.Cache; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Map; + +import static org.mockito.Mockito.when; + +public class CacheStatsOnNodeSupplierTests extends OpenSearchTestCase { + @Mock + private Caches caches; + + @Mock + private Cache featureCache; + + @Mock + private Cache featureSetCache; + + @Mock + private Cache modelCache; + + private CacheStatsOnNodeSupplier cacheStatsOnNodeSupplier; + + @Before + public void setup() { + MockitoAnnotations.openMocks(this); + + when(caches.featureCache()).thenReturn(featureCache); + when(caches.featureSetCache()).thenReturn(featureSetCache); + when(caches.modelCache()).thenReturn(modelCache); + + cacheStatsOnNodeSupplier = new CacheStatsOnNodeSupplier(caches); + } + + @Test + public void testGetCacheStats() { + when(featureCache.stats()).thenReturn(new Cache.CacheStats(4, 4, 1)); + when(featureCache.count()).thenReturn(4); + when(featureCache.weight()).thenReturn(500L); + + when(featureSetCache.stats()).thenReturn(new Cache.CacheStats(2, 2, 1)); + when(featureSetCache.count()).thenReturn(2); + when(featureSetCache.weight()).thenReturn(600L); + + when(modelCache.stats()).thenReturn(new Cache.CacheStats(1, 1, 0)); + when(modelCache.count()).thenReturn(1); + when(modelCache.weight()).thenReturn(800L); + + Map> values = cacheStatsOnNodeSupplier.get(); + assertCacheStats(values.get("feature"), + 4, 4, 1, 4, 500); + assertCacheStats(values.get("featureset"), + 2, 2, 1, 2, 600); + assertCacheStats(values.get("model"), + 1, 1, 0, 1, 800); + } + + private void assertCacheStats(Map stat, long hits, + long misses, long evictions, int entries, long memUsage) { + assertEquals(hits, stat.get("hit_count")); + assertEquals(misses, stat.get("miss_count")); + assertEquals(evictions, stat.get("eviction_count")); + assertEquals(entries, stat.get("entry_count")); + assertEquals(memUsage, stat.get("memory_usage_in_bytes")); + } +} diff --git a/src/test/java/org/opensearch/ltr/stats/suppliers/CounterSupplierTests.java b/src/test/java/org/opensearch/ltr/stats/suppliers/CounterSupplierTests.java new file mode 100644 index 0000000..ab3bd03 --- /dev/null +++ b/src/test/java/org/opensearch/ltr/stats/suppliers/CounterSupplierTests.java @@ -0,0 +1,29 @@ +/* + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.opensearch.ltr.stats.suppliers; + +import org.junit.Test; +import org.opensearch.test.OpenSearchTestCase; + +public class CounterSupplierTests extends OpenSearchTestCase { + @Test + public void testGetAndIncrement() { + CounterSupplier counterSupplier = new CounterSupplier(); + assertEquals((Long) 0L, counterSupplier.get()); + counterSupplier.increment(); + assertEquals((Long) 1L, counterSupplier.get()); + } +} \ No newline at end of file From 7b5ac9104a3c0ba9659b9ea187861f89c7f826b9 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Tue, 3 Dec 2024 18:46:32 +0100 Subject: [PATCH 2/8] feat: integrated stats and counters for total requests, error requests and caches into query implementations Signed-off-by: Johannes Peter --- .../o19s/es/ltr/action/LTRStatsActionIT.java | 178 ------------------ .../com/o19s/es/explore/ExplorerQuery.java | 25 ++- .../o19s/es/explore/ExplorerQueryBuilder.java | 18 +- .../com/o19s/es/ltr/LtrQueryParserPlugin.java | 81 ++++---- .../action/TransportFeatureStoreAction.java | 14 +- .../o19s/es/ltr/query/LtrQueryBuilder.java | 32 +++- .../com/o19s/es/ltr/query/RankerQuery.java | 68 +++++-- .../es/ltr/query/StoredLtrQueryBuilder.java | 29 ++- .../ltr/query/ValidatingLtrQueryBuilder.java | 36 +++- .../java/com/o19s/es/ltr/stats/LTRStat.java | 1 + .../java/com/o19s/es/ltr/stats/LTRStats.java | 1 + .../java/com/o19s/es/ltr/stats/StatName.java | 1 + .../suppliers/CacheStatsOnNodeSupplier.java | 1 + .../suppliers/PluginHealthStatusSupplier.java | 1 + .../stats/suppliers/StoreStatsSupplier.java | 1 + .../es/explore/ExplorerQueryBuilderTests.java | 15 +- .../o19s/es/explore/ExplorerQueryTests.java | 39 ++-- .../logging/LoggingFetchSubPhaseTests.java | 15 +- .../es/ltr/query/LtrQueryBuilderTests.java | 17 +- .../com/o19s/es/ltr/query/LtrQueryTests.java | 17 +- .../ltr/query/StoredLtrQueryBuilderTests.java | 17 +- .../query/ValidatingLtrQueryBuilderTests.java | 14 +- .../test/fstore/90_get_stats.yml | 172 ----------------- 23 files changed, 345 insertions(+), 448 deletions(-) delete mode 100644 src/javaRestTest/java/com/o19s/es/ltr/action/LTRStatsActionIT.java delete mode 100644 src/test/resources/rest-api-spec/test/fstore/90_get_stats.yml diff --git a/src/javaRestTest/java/com/o19s/es/ltr/action/LTRStatsActionIT.java b/src/javaRestTest/java/com/o19s/es/ltr/action/LTRStatsActionIT.java deleted file mode 100644 index d4ad731..0000000 --- a/src/javaRestTest/java/com/o19s/es/ltr/action/LTRStatsActionIT.java +++ /dev/null @@ -1,178 +0,0 @@ -/* - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ -package com.o19s.es.ltr.action; - -import com.o19s.es.ltr.LtrTestUtils; -import com.o19s.es.ltr.action.LTRStatsAction.LTRStatsNodesResponse; -import com.o19s.es.ltr.action.LTRStatsAction.LTRStatsRequestBuilder; -import com.o19s.es.ltr.feature.store.StoredFeature; -import com.o19s.es.ltr.feature.store.StoredFeatureSet; -import com.o19s.es.ltr.feature.store.StoredLtrModel; -import com.o19s.es.ltr.feature.store.index.IndexFeatureStore; -import com.o19s.es.ltr.stats.StatName; -import com.o19s.es.ltr.stats.suppliers.CacheStatsOnNodeSupplier; -import com.o19s.es.ltr.stats.suppliers.StoreStatsSupplier; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ExecutionException; - -import static com.o19s.es.ltr.feature.store.index.IndexFeatureStore.DEFAULT_STORE; -import static com.o19s.es.ltr.feature.store.index.IndexFeatureStore.indexName; - -public class LTRStatsActionIT extends BaseIntegrationTest { - private static final String DEFAULT_STORE_NAME = IndexFeatureStore.storeName(DEFAULT_STORE); - - @SuppressWarnings("unchecked") - public void testStatsNoStore() throws Exception { - deleteDefaultStore(); - LTRStatsNodesResponse response = executeRequest(); - assertFalse(response.hasFailures()); - - Map clusterStats = response.getClusterStats(); - assertEquals("green", clusterStats.get(StatName.PLUGIN_STATUS.getName())); - - Map stores = (Map) clusterStats.get(StatName.STORES.getName()); - assertTrue(stores.isEmpty()); - } - - @SuppressWarnings("unchecked") - public void testAllStatsDefaultEmptyStore() throws ExecutionException, InterruptedException { - LTRStatsNodesResponse response = executeRequest(); - assertFalse(response.hasFailures()); - - Map clusterStats = response.getClusterStats(); - assertEquals("green", clusterStats.get(StatName.PLUGIN_STATUS.getName())); - - Map stores = (Map) clusterStats.get(StatName.STORES.getName()); - assertEquals(1, stores.size()); - assertTrue(stores.containsKey(DEFAULT_STORE_NAME)); - Map storeStat = (Map) stores.get(DEFAULT_STORE_NAME); - assertEquals(0L, storeStat.get(StoreStatsSupplier.Stat.STORE_FEATURE_COUNT.getName())); - assertEquals(0L, storeStat.get(StoreStatsSupplier.Stat.STORE_FEATURE_SET_COUNT.getName())); - assertEquals(0L, storeStat.get(StoreStatsSupplier.Stat.STORE_MODEL_COUNT.getName())); - - Map nodeStats = response.getNodes().get(0).getStatsMap(); - assertFalse(nodeStats.isEmpty()); - assertTrue(nodeStats.containsKey(StatName.CACHE.getName())); - - Map cacheStats = (Map) nodeStats.get(StatName.CACHE.getName()); - assertEquals(3, cacheStats.size()); - assertTrue(cacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_FEATURE.getName())); - assertTrue(cacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_FEATURE_SET.getName())); - assertTrue(cacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_MODEL.getName())); - - Map featureCacheStats = - (Map) cacheStats.get(CacheStatsOnNodeSupplier.Stat.CACHE_FEATURE.getName()); - assertEquals(5, featureCacheStats.size()); - assertTrue(featureCacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_HIT_COUNT.getName())); - assertTrue(featureCacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_MISS_COUNT.getName())); - assertTrue(featureCacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_EVICTION_COUNT.getName())); - assertTrue(featureCacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_ENTRY_COUNT.getName())); - assertTrue(featureCacheStats.containsKey(CacheStatsOnNodeSupplier.Stat.CACHE_MEMORY_USAGE_IN_BYTES.getName())); - } - - - @SuppressWarnings("unchecked") - public void testMultipleFeatureStores() throws Exception { - createStore(indexName("test1")); - - LTRStatsNodesResponse response = executeRequest(); - assertFalse(response.hasFailures()); - - Map clusterStats = response.getClusterStats(); - assertEquals("green", clusterStats.get(StatName.PLUGIN_STATUS.getName())); - - Map stores = (Map) clusterStats.get(StatName.STORES.getName()); - assertEquals(2, stores.size()); - assertTrue(stores.containsKey(DEFAULT_STORE_NAME)); - assertTrue(stores.containsKey(IndexFeatureStore.storeName(indexName("test1")))); - } - - @SuppressWarnings("unchecked") - public void testStoreStats() throws Exception { - String customStoreName = "test"; - String customStore = indexName("test"); - createStore(customStore); - - Map> infos = new HashMap<>(); - infos.put(DEFAULT_STORE_NAME, addElements(DEFAULT_STORE)); - infos.put(customStoreName, addElements(customStore)); - - LTRStatsNodesResponse response = executeRequest(); - assertFalse(response.hasFailures()); - - Map clusterStats = response.getClusterStats(); - Map stores = (Map) clusterStats.get(StatName.STORES.getName()); - - assertStoreStats((Map) stores.get(DEFAULT_STORE_NAME), infos.get(DEFAULT_STORE_NAME)); - assertStoreStats((Map) stores.get(customStoreName), infos.get(customStoreName)); - } - - private void assertStoreStats(Map storeStat, Map expected) { - assertEquals(expected.get(StoredFeatureSet.TYPE), - storeStat.get(StoreStatsSupplier.Stat.STORE_FEATURE_SET_COUNT.getName())); - - assertEquals(expected.get(StoredFeature.TYPE), - storeStat.get(StoreStatsSupplier.Stat.STORE_FEATURE_COUNT.getName())); - - assertEquals(expected.get(StoredLtrModel.TYPE), - storeStat.get(StoreStatsSupplier.Stat.STORE_MODEL_COUNT.getName())); - } - - private Map addElements(String store) throws Exception { - Map counts = new HashMap<>(); - int nFeats = randomInt(20) + 1; - int nSets = randomInt(20) + 1; - int nModels = randomInt(20) + 1; - counts.put(StoredFeature.TYPE, (long) nFeats); - counts.put(StoredFeatureSet.TYPE, (long) nSets); - counts.put(StoredLtrModel.TYPE, (long) nModels); - addElements(store, nFeats, nSets, nModels); - return counts; - } - - private void addElements(String store, int nFeatures, int nSets, int nModels) throws Exception { - for (int i = 0; i < nFeatures; i++) { - StoredFeature feat = LtrTestUtils.randomFeature("feature" + i); - addElement(feat, store); - } - - List sets = new ArrayList<>(nSets); - for (int i = 0; i < nSets; i++) { - StoredFeatureSet set = LtrTestUtils.randomFeatureSet("set" + i); - addElement(set, store); - sets.add(set); - } - - for (int i = 0; i < nModels; i++) { - addElement(LtrTestUtils.randomLinearModel("model" + i, sets.get(random().nextInt(sets.size()))), store); - } - } - - private LTRStatsNodesResponse executeRequest() throws ExecutionException, InterruptedException { - LTRStatsRequestBuilder builder = new LTRStatsRequestBuilder(client()); - Set statsToBeRetrieved = new HashSet<>(Arrays.asList( - StatName.PLUGIN_STATUS.getName(), StatName.CACHE.getName(), StatName.STORES.getName())); - builder.request().setStatsToBeRetrieved(statsToBeRetrieved); - return builder.execute().get(); - } -} diff --git a/src/main/java/com/o19s/es/explore/ExplorerQuery.java b/src/main/java/com/o19s/es/explore/ExplorerQuery.java index 5aa8c6f..869baed 100644 --- a/src/main/java/com/o19s/es/explore/ExplorerQuery.java +++ b/src/main/java/com/o19s/es/explore/ExplorerQuery.java @@ -16,12 +16,15 @@ package com.o19s.es.explore; +import org.apache.lucene.search.QueryVisitor; +import org.opensearch.ltr.settings.LTRSettings; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.Term; import org.apache.lucene.index.TermStates; import org.apache.lucene.search.Query; -import org.apache.lucene.search.QueryVisitor; import org.apache.lucene.search.ScoreMode; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Weight; @@ -35,7 +38,6 @@ import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.similarities.ClassicSimilarity; -import org.opensearch.ltr.settings.LTRSettings; import java.io.IOException; import java.util.HashSet; @@ -45,10 +47,12 @@ public class ExplorerQuery extends Query { private final Query query; private final String type; + private final LTRStats ltrStats; - public ExplorerQuery(Query query, String type) { + public ExplorerQuery(Query query, String type, LTRStats ltrStats) { this.query = query; this.type = type; + this.ltrStats = ltrStats; } private boolean isCollectionScoped() { @@ -62,6 +66,7 @@ private boolean isCollectionScoped() { public String getType() { return this.type; } + @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") @Override public boolean equals(Object other) { return sameClassAs(other) && @@ -78,7 +83,7 @@ public Query rewrite(IndexReader reader) throws IOException { Query rewritten = query.rewrite(reader); if (rewritten != query) { - return new ExplorerQuery(rewritten, type); + return new ExplorerQuery(rewritten, type, ltrStats); } return this; @@ -90,12 +95,20 @@ public int hashCode() { } @Override - public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) - throws IOException { + public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!LTRSettings.isLTRPluginEnabled()) { throw new IllegalStateException("LTR plugin is disabled. To enable, update ltr.plugin.enabled to true"); } + try { + return _createWeight(searcher, scoreMode, boost); + } catch (Exception e) { + ltrStats.getStats().get(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); + throw e; + } + } + + public Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!scoreMode.needsScores()) { return searcher.createWeight(query, scoreMode, boost); } diff --git a/src/main/java/com/o19s/es/explore/ExplorerQueryBuilder.java b/src/main/java/com/o19s/es/explore/ExplorerQueryBuilder.java index eeaf329..d805ce9 100644 --- a/src/main/java/com/o19s/es/explore/ExplorerQueryBuilder.java +++ b/src/main/java/com/o19s/es/explore/ExplorerQueryBuilder.java @@ -15,6 +15,8 @@ package com.o19s.es.explore; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import org.apache.lucene.search.Query; import org.opensearch.core.ParseField; import org.opensearch.core.common.ParsingException; @@ -53,18 +55,20 @@ public class ExplorerQueryBuilder extends AbstractQueryBuilder> getQueries() { return asList( - new QuerySpec<>(ExplorerQueryBuilder.NAME, ExplorerQueryBuilder::new, ExplorerQueryBuilder::fromXContent), - new QuerySpec<>(LtrQueryBuilder.NAME, LtrQueryBuilder::new, LtrQueryBuilder::fromXContent), - new QuerySpec<>(StoredLtrQueryBuilder.NAME, - (input) -> new StoredLtrQueryBuilder(getFeatureStoreLoader(), input), - (ctx) -> StoredLtrQueryBuilder.fromXContent(getFeatureStoreLoader(), ctx)), + new QuerySpec<>( + ExplorerQueryBuilder.NAME, + (input) -> new ExplorerQueryBuilder(input, ltrStats), + (ctx) -> ExplorerQueryBuilder.fromXContent(ctx, ltrStats) + ), + new QuerySpec<>( + LtrQueryBuilder.NAME, + (input) -> new LtrQueryBuilder(input, ltrStats), + (ctx) -> LtrQueryBuilder.fromXContent(ctx, ltrStats) + ), + new QuerySpec<>( + StoredLtrQueryBuilder.NAME, + (input) -> new StoredLtrQueryBuilder(getFeatureStoreLoader(), input, ltrStats), + (ctx) -> StoredLtrQueryBuilder.fromXContent(getFeatureStoreLoader(), ctx, ltrStats) + ), new QuerySpec<>(TermStatQueryBuilder.NAME, TermStatQueryBuilder::new, TermStatQueryBuilder::fromXContent), - new QuerySpec<>(ValidatingLtrQueryBuilder.NAME, - (input) -> new ValidatingLtrQueryBuilder(input, parserFactory), - (ctx) -> ValidatingLtrQueryBuilder.fromXContent(ctx, parserFactory))); + new QuerySpec<>( + ValidatingLtrQueryBuilder.NAME, + (input) -> new ValidatingLtrQueryBuilder(input, parserFactory, ltrStats), + (ctx) -> ValidatingLtrQueryBuilder.fromXContent(ctx, parserFactory, ltrStats) + ) + ); } @Override @@ -186,7 +199,6 @@ public List getRestHandlers(Settings settings, RestController restC list.add(new RestFeatureStoreCaches()); list.add(new RestCreateModelFromSet()); list.add(new RestAddFeatureToSet()); - list.add(new RestLTRStats()); return unmodifiableList(list); } @@ -198,8 +210,7 @@ public List getRestHandlers(Settings settings, RestController restC new ActionHandler<>(ClearCachesAction.INSTANCE, TransportClearCachesAction.class), new ActionHandler<>(AddFeaturesToSetAction.INSTANCE, TransportAddFeatureToSetAction.class), new ActionHandler<>(CreateModelFromSetAction.INSTANCE, TransportCreateModelFromSetAction.class), - new ActionHandler<>(ListStoresAction.INSTANCE, TransportListStoresAction.class), - new ActionHandler<>(LTRStatsAction.INSTANCE, TransportLTRStatsAction.class))); + new ActionHandler<>(ListStoresAction.INSTANCE, TransportListStoresAction.class))); } @Override @@ -234,11 +245,15 @@ public List getNamedXContent() { @Override public List> getSettings() { - return unmodifiableList(asList( + + List> list1 = LTRSettings.getInstance().getSettings(); + List> list2 = asList( IndexFeatureStore.STORE_VERSION_PROP, Caches.LTR_CACHE_MEM_SETTING, Caches.LTR_CACHE_EXPIRE_AFTER_READ, - Caches.LTR_CACHE_EXPIRE_AFTER_WRITE)); + Caches.LTR_CACHE_EXPIRE_AFTER_WRITE); + + return unmodifiableList(Stream.concat(list1.stream(), list2.stream()).collect(Collectors.toList())); } @Override @@ -263,18 +278,18 @@ public Collection createComponents(Client client, final JvmService jvmService = new JvmService(environment.settings()); final LTRCircuitBreakerService ltrCircuitBreakerService = new LTRCircuitBreakerService(jvmService).init(); - return asList(caches, parserFactory, ltrCircuitBreakerService, getStats(client, clusterService, indexNameExpressionResolver)); + return asList(caches, parserFactory, ltrCircuitBreakerService, ltrStats); } - private LTRStats getStats(Client client, ClusterService clusterService, IndexNameExpressionResolver indexNameExpressionResolver) { - Map stats = new HashMap<>(); - stats.put(StatName.CACHE.getName(), - new LTRStat(false, new CacheStatsOnNodeSupplier(caches))); - stats.put(StatName.STORES.getName(), - new LTRStat(true, new StoreStatsSupplier(client, clusterService, indexNameExpressionResolver))); - stats.put(StatName.PLUGIN_STATUS.getName(), - new LTRStat(true, new PluginHealthStatusSupplier(clusterService, indexNameExpressionResolver))); - return new LTRStats(unmodifiableMap(stats)); + private LTRStats getInitialStats() { + Map> stats = new HashMap<>(); + stats.put(StatName.LTR_CACHE_STATS.getName(), + new LTRStat<>(false, new CacheStatsOnNodeSupplier(caches))); + stats.put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + stats.put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + return new LTRStats((stats)); } protected FeatureStoreLoader getFeatureStoreLoader() { diff --git a/src/main/java/com/o19s/es/ltr/action/TransportFeatureStoreAction.java b/src/main/java/com/o19s/es/ltr/action/TransportFeatureStoreAction.java index f762ca0..d67852c 100644 --- a/src/main/java/com/o19s/es/ltr/action/TransportFeatureStoreAction.java +++ b/src/main/java/com/o19s/es/ltr/action/TransportFeatureStoreAction.java @@ -18,6 +18,7 @@ import org.opensearch.ltr.breaker.LTRCircuitBreakerService; import org.opensearch.ltr.exception.LimitExceededException; +import org.opensearch.ltr.stats.LTRStats; import com.o19s.es.ltr.action.ClearCachesAction.ClearCachesNodesRequest; import com.o19s.es.ltr.action.FeatureStoreAction.FeatureStoreRequest; import com.o19s.es.ltr.action.FeatureStoreAction.FeatureStoreResponse; @@ -58,6 +59,7 @@ public class TransportFeatureStoreAction extends HandledTransportAction store(request, task, listener)); + () -> store(request, task, listener), ltrStats); } else { store(request, task, listener); } @@ -154,14 +158,16 @@ private void precheck(FeatureStoreRequest request) { * @param task the parent task * @param listener the action listener to write to * @param onSuccess action ro run when the validation is successfull + * @param ltrStats LTR stats */ private void validate(FeatureValidation validation, StorableElement element, Task task, ActionListener listener, - Runnable onSuccess) { + Runnable onSuccess, + LTRStats ltrStats) { ValidatingLtrQueryBuilder ltrBuilder = new ValidatingLtrQueryBuilder(element, - validation, factory); + validation, factory, ltrStats); SearchRequestBuilder builder = new SearchRequestBuilder(client, SearchAction.INSTANCE); builder.setIndices(validation.getIndex()); builder.setQuery(ltrBuilder); diff --git a/src/main/java/com/o19s/es/ltr/query/LtrQueryBuilder.java b/src/main/java/com/o19s/es/ltr/query/LtrQueryBuilder.java index d0ab949..eb0c3d4 100644 --- a/src/main/java/com/o19s/es/ltr/query/LtrQueryBuilder.java +++ b/src/main/java/com/o19s/es/ltr/query/LtrQueryBuilder.java @@ -17,6 +17,8 @@ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import com.o19s.es.ltr.feature.PrebuiltFeature; import com.o19s.es.ltr.feature.PrebuiltFeatureSet; import com.o19s.es.ltr.feature.PrebuiltLtrModel; @@ -64,19 +66,22 @@ public class LtrQueryBuilder extends AbstractQueryBuilder { private Script _rankLibScript; private List _features; + private LTRStats _ltrStats; public LtrQueryBuilder() { } - public LtrQueryBuilder(Script _rankLibScript, List features) { + public LtrQueryBuilder(Script _rankLibScript, List features, LTRStats ltrStats) { this._rankLibScript = _rankLibScript; this._features = features; + this._ltrStats = ltrStats; } - public LtrQueryBuilder(StreamInput in) throws IOException { + public LtrQueryBuilder(StreamInput in, LTRStats ltrStats) throws IOException { super(in); _features = AbstractQueryBuilderUtils.readQueries(in); _rankLibScript = new Script(in); + _ltrStats = ltrStats; } private static void doXArrayContent(String field, List clauses, XContentBuilder builder, Params params) @@ -91,7 +96,7 @@ private static void doXArrayContent(String field, List clauses, XC builder.endArray(); } - public static LtrQueryBuilder fromXContent(XContentParser parser) throws IOException { + public static LtrQueryBuilder fromXContent(XContentParser parser, LTRStats ltrStats) throws IOException { final LtrQueryBuilder builder; try { builder = PARSER.apply(parser, null); @@ -102,6 +107,7 @@ public static LtrQueryBuilder fromXContent(XContentParser parser) throws IOExcep throw new ParsingException(parser.getTokenLocation(), "[ltr] query requires a model, none specified"); } + builder.ltrStats(ltrStats); return builder; } @@ -123,6 +129,16 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep @Override protected Query doToQuery(QueryShardContext context) throws IOException { + _ltrStats.getStat(StatName.LTR_REQUEST_TOTAL_COUNT.getName()).increment(); + try { + return _doToQuery(context); + } catch (Exception e) { + _ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); + throw e; + } + } + + private Query _doToQuery(QueryShardContext context) throws IOException { List features = new ArrayList<>(_features.size()); for (QueryBuilder builder : _features) { features.add(new PrebuiltFeature(builder.queryName(), builder.toQuery(context))); @@ -137,7 +153,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { PrebuiltFeatureSet featureSet = new PrebuiltFeatureSet(queryName(), features); PrebuiltLtrModel model = new PrebuiltLtrModel(ranker.name(), ranker, featureSet); - return RankerQuery.build(model); + return RankerQuery.build(model, _ltrStats); } @Override @@ -164,7 +180,7 @@ public QueryBuilder doRewrite(QueryRewriteContext ctx) throws IOException { } if (changed) { assert newFeatures.size() == _features.size(); - return new LtrQueryBuilder(_rankLibScript, newFeatures); + return new LtrQueryBuilder(_rankLibScript, newFeatures, _ltrStats); } return this; } @@ -194,6 +210,12 @@ public final LtrQueryBuilder rankerScript(Script rankLibModel) { return this; } + public final LtrQueryBuilder ltrStats(LTRStats ltrStats) { + _ltrStats = ltrStats; + return this; + } + + public List features() { return _features; } diff --git a/src/main/java/com/o19s/es/ltr/query/RankerQuery.java b/src/main/java/com/o19s/es/ltr/query/RankerQuery.java index 40f2170..7cf2fb1 100644 --- a/src/main/java/com/o19s/es/ltr/query/RankerQuery.java +++ b/src/main/java/com/o19s/es/ltr/query/RankerQuery.java @@ -17,6 +17,8 @@ package com.o19s.es.ltr.query; import org.opensearch.ltr.settings.LTRSettings; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import com.o19s.es.ltr.LtrQueryContext; import com.o19s.es.ltr.feature.Feature; import com.o19s.es.ltr.feature.FeatureSet; @@ -80,17 +82,23 @@ public class RankerQuery extends Query { */ private static final ThreadLocal CURRENT_VECTOR = new ThreadLocal<>(); + private final LTRStats ltrStats; private final List queries; private final FeatureSet features; private final LtrRanker ranker; private final Map featureScoreCache; - private RankerQuery(List queries, FeatureSet features, LtrRanker ranker, - Map featureScoreCache) { + private RankerQuery( + List queries, + FeatureSet features, + LtrRanker ranker, + Map featureScoreCache, + LTRStats ltrStats) { this.queries = Objects.requireNonNull(queries); this.features = Objects.requireNonNull(features); this.ranker = Objects.requireNonNull(ranker); this.featureScoreCache = featureScoreCache; + this.ltrStats = ltrStats; } /** @@ -100,9 +108,15 @@ private RankerQuery(List queries, FeatureSet features, LtrRanker ranker, * @param model a prebuilt model * @return the lucene query */ - public static RankerQuery build(PrebuiltLtrModel model) { - return build(model.ranker(), model.featureSet(), - new LtrQueryContext(null, Collections.emptySet()), Collections.emptyMap(), false); + public static RankerQuery build(PrebuiltLtrModel model, LTRStats ltrStats) { + return build( + model.ranker(), + model.featureSet(), + new LtrQueryContext(null, Collections.emptySet()), + Collections.emptyMap(), + false, + ltrStats + ); } /** @@ -113,31 +127,46 @@ public static RankerQuery build(PrebuiltLtrModel model) { * @param params the query params * @return the lucene query */ - public static RankerQuery build(LtrModel model, LtrQueryContext context, Map params, - Boolean featureScoreCacheFlag) { - return build(model.ranker(), model.featureSet(), context, params, featureScoreCacheFlag); + public static RankerQuery build( + LtrModel model, + LtrQueryContext context, + Map params, + Boolean featureScoreCacheFlag, + LTRStats ltrStats) { + return build( + model.ranker(), + model.featureSet(), + context, + params, + featureScoreCacheFlag, + ltrStats); } - private static RankerQuery build(LtrRanker ranker, FeatureSet features, - LtrQueryContext context, Map params, Boolean featureScoreCacheFlag) { + private static RankerQuery build( + LtrRanker ranker, + FeatureSet features, + LtrQueryContext context, + Map params, + Boolean featureScoreCacheFlag, + LTRStats ltrStats) { List queries = features.toQueries(context, params); Map featureScoreCache = null; if (null != featureScoreCacheFlag && featureScoreCacheFlag) { featureScoreCache = new HashMap<>(); } - return new RankerQuery(queries, features, ranker, featureScoreCache); + return new RankerQuery(queries, features, ranker, featureScoreCache, ltrStats); } public static RankerQuery buildLogQuery(LogLtrRanker.LogConsumer consumer, FeatureSet features, - LtrQueryContext context, Map params) { + LtrQueryContext context, Map params, LTRStats ltrStats) { List queries = features.toQueries(context, params); return new RankerQuery(queries, features, - new LogLtrRanker(consumer, features.size()), null); + new LogLtrRanker(consumer, features.size()), null, ltrStats); } public RankerQuery toLoggerQuery(LogLtrRanker.LogConsumer consumer) { NullRanker newRanker = new NullRanker(features.size()); - return new RankerQuery(queries, features, new LogLtrRanker(newRanker, consumer), featureScoreCache); + return new RankerQuery(queries, features, new LogLtrRanker(newRanker, consumer), featureScoreCache, ltrStats); } @Override @@ -149,7 +178,7 @@ public Query rewrite(IndexReader reader) throws IOException { rewritten |= rewrittenQuery != query; rewrittenQueries.add(rewrittenQuery); } - return rewritten ? new RankerQuery(rewrittenQueries, features, ranker, featureScoreCache) : this; + return rewritten ? new RankerQuery(rewrittenQueries, features, ranker, featureScoreCache, ltrStats) : this; } @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") @@ -206,6 +235,15 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo throw new IllegalStateException("LTR plugin is disabled. To enable, update ltr.plugin.enabled to true"); } + try { + return _createWeight(searcher, scoreMode, boost); + } catch (Exception e) { + ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); + throw e; + } + } + + private Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!scoreMode.needsScores()) { // If scores are not needed simply return a constant score on all docs return new ConstantScoreWeight(this, boost) { diff --git a/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java b/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java index e32cde5..2160da7 100644 --- a/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java +++ b/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java @@ -16,6 +16,8 @@ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import com.o19s.es.ltr.LtrQueryContext; import com.o19s.es.ltr.feature.FeatureSet; import com.o19s.es.ltr.feature.store.CompiledLtrModel; @@ -76,13 +78,14 @@ public class StoredLtrQueryBuilder extends AbstractQueryBuilder params; private List activeFeatures; + private LTRStats ltrStats; public StoredLtrQueryBuilder(FeatureStoreLoader storeLoader) { this.storeLoader = storeLoader; } - public StoredLtrQueryBuilder(FeatureStoreLoader storeLoader, StreamInput input) throws IOException { + public StoredLtrQueryBuilder(FeatureStoreLoader storeLoader, StreamInput input, LTRStats ltrStats) throws IOException { super(input); this.storeLoader = Objects.requireNonNull(storeLoader); modelName = input.readOptionalString(); @@ -92,10 +95,12 @@ public StoredLtrQueryBuilder(FeatureStoreLoader storeLoader, StreamInput input) String[] activeFeat = input.readOptionalStringArray(); activeFeatures = activeFeat == null ? null : Arrays.asList(activeFeat); storeName = input.readOptionalString(); + this.ltrStats = ltrStats; } public static StoredLtrQueryBuilder fromXContent(FeatureStoreLoader storeLoader, - XContentParser parser) throws IOException { + XContentParser parser, + LTRStats ltrStats) throws IOException { storeLoader = Objects.requireNonNull(storeLoader); final StoredLtrQueryBuilder builder = new StoredLtrQueryBuilder(storeLoader); try { @@ -109,6 +114,7 @@ public static StoredLtrQueryBuilder fromXContent(FeatureStoreLoader storeLoader, if (builder.params() == null) { throw new ParsingException(parser.getTokenLocation(), "Field [" + PARAMS + "] is mandatory."); } + builder.ltrStats(ltrStats); return builder; } @@ -157,6 +163,16 @@ private static void validateActiveFeatures(FeatureSet features, LtrQueryContext @Override protected RankerQuery doToQuery(QueryShardContext context) throws IOException { + this.ltrStats.getStat(StatName.LTR_REQUEST_TOTAL_COUNT.getName()).increment(); + try { + return _doToQuery(context); + } catch (Exception e) { + ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); + throw e; + } + } + + private RankerQuery _doToQuery(QueryShardContext context) throws IOException { String indexName = storeName != null ? IndexFeatureStore.indexName(storeName) : IndexFeatureStore.DEFAULT_STORE; FeatureStore store = storeLoader.load(indexName, context::getClient); LtrQueryContext ltrQueryContext = new LtrQueryContext(context, @@ -164,7 +180,7 @@ protected RankerQuery doToQuery(QueryShardContext context) throws IOException { if (modelName != null) { CompiledLtrModel model = store.loadModel(modelName); validateActiveFeatures(model.featureSet(), ltrQueryContext); - return RankerQuery.build(model, ltrQueryContext, params, featureScoreCacheFlag); + return RankerQuery.build(model, ltrQueryContext, params, featureScoreCacheFlag, ltrStats); } else { assert featureSetName != null; FeatureSet set = store.loadSet(featureSetName); @@ -173,7 +189,7 @@ protected RankerQuery doToQuery(QueryShardContext context) throws IOException { LinearRanker ranker = new LinearRanker(weights); CompiledLtrModel model = new CompiledLtrModel("linear", set, ranker); validateActiveFeatures(model.featureSet(), ltrQueryContext); - return RankerQuery.build(model, ltrQueryContext, params, featureScoreCacheFlag); + return RankerQuery.build(model, ltrQueryContext, params, featureScoreCacheFlag, ltrStats); } } @@ -220,6 +236,11 @@ public StoredLtrQueryBuilder featureSetName(String featureSetName) { return this; } + public StoredLtrQueryBuilder ltrStats(LTRStats ltrStats) { + this.ltrStats = ltrStats; + return this; + } + public String storeName() { return storeName; } diff --git a/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java b/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java index 920d1f8..6948c78 100644 --- a/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java +++ b/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java @@ -16,6 +16,8 @@ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; import com.o19s.es.ltr.LtrQueryContext; import com.o19s.es.ltr.feature.Feature; import com.o19s.es.ltr.feature.FeatureSet; @@ -88,17 +90,22 @@ public class ValidatingLtrQueryBuilder extends AbstractQueryBuilder supplier; diff --git a/src/main/java/com/o19s/es/ltr/stats/LTRStats.java b/src/main/java/com/o19s/es/ltr/stats/LTRStats.java index 7fc8273..e2ca08c 100644 --- a/src/main/java/com/o19s/es/ltr/stats/LTRStats.java +++ b/src/main/java/com/o19s/es/ltr/stats/LTRStats.java @@ -22,6 +22,7 @@ /** * This class is the main entry-point for access to the stats that the LTR plugin keeps track of. */ +@Deprecated public class LTRStats { private final Map stats; diff --git a/src/main/java/com/o19s/es/ltr/stats/StatName.java b/src/main/java/com/o19s/es/ltr/stats/StatName.java index 1a45473..0b5fa3f 100644 --- a/src/main/java/com/o19s/es/ltr/stats/StatName.java +++ b/src/main/java/com/o19s/es/ltr/stats/StatName.java @@ -19,6 +19,7 @@ import java.util.HashSet; import java.util.Set; +@Deprecated public enum StatName { PLUGIN_STATUS("status"), STORES("stores"), diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java index 85a5bb6..926d16d 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java @@ -26,6 +26,7 @@ /** * Aggregate stats on the cache used by the plugin per node. */ +@Deprecated public class CacheStatsOnNodeSupplier implements Supplier>> { private final Caches caches; diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java index ebf588a..af9f184 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java @@ -29,6 +29,7 @@ /** * Supplier for an overall plugin health status. */ +@Deprecated public class PluginHealthStatusSupplier implements Supplier { private static final String STATUS_GREEN = "green"; private static final String STATUS_YELLOW = "yellow"; diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java index efac97d..72dd842 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java @@ -49,6 +49,7 @@ * information such as the index health and count of feature sets, features and * models in the store. */ +@Deprecated public class StoreStatsSupplier implements Supplier>> { private static final Logger LOG = LogManager.getLogger(StoreStatsSupplier.class); private static final String AGG_FIELD = "type"; diff --git a/src/test/java/com/o19s/es/explore/ExplorerQueryBuilderTests.java b/src/test/java/com/o19s/es/explore/ExplorerQueryBuilderTests.java index 222ca23..03ecdf1 100644 --- a/src/test/java/com/o19s/es/explore/ExplorerQueryBuilderTests.java +++ b/src/test/java/com/o19s/es/explore/ExplorerQueryBuilderTests.java @@ -15,6 +15,10 @@ */ package com.o19s.es.explore; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.o19s.es.ltr.LtrQueryParserPlugin; import org.apache.lucene.search.Query; import org.opensearch.core.common.ParsingException; @@ -28,7 +32,8 @@ import java.io.IOException; import java.util.Collection; - +import java.util.HashMap; +import static java.util.Collections.unmodifiableMap; import static java.util.Arrays.asList; import static org.hamcrest.CoreMatchers.instanceOf; @@ -37,12 +42,18 @@ public class ExplorerQueryBuilderTests extends AbstractQueryTestCase> getPlugins() { return asList(LtrQueryParserPlugin.class, TestGeoShapeFieldMapperPlugin.class); } - + private LTRStats ltrStats = new LTRStats(unmodifiableMap(new HashMap>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); @Override protected ExplorerQueryBuilder doCreateTestQueryBuilder() { ExplorerQueryBuilder builder = new ExplorerQueryBuilder(); builder.query(new TermQueryBuilder("foo", "bar")); builder.statsType("sum_raw_ttf"); + builder.ltrStats(ltrStats); return builder; } diff --git a/src/test/java/com/o19s/es/explore/ExplorerQueryTests.java b/src/test/java/com/o19s/es/explore/ExplorerQueryTests.java index ede0ab7..fad5ad7 100644 --- a/src/test/java/com/o19s/es/explore/ExplorerQueryTests.java +++ b/src/test/java/com/o19s/es/explore/ExplorerQueryTests.java @@ -15,6 +15,10 @@ */ package com.o19s.es.explore; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.document.StoredField; @@ -37,12 +41,17 @@ import org.junit.After; import org.junit.Before; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.unmodifiableMap; import static org.hamcrest.Matchers.equalTo; public class ExplorerQueryTests extends LuceneTestCase { private Directory dir; private IndexReader reader; private IndexSearcher searcher; + private LTRStats ltrStats; // Some simple documents to index private final String[] docs = new String[] { @@ -69,6 +78,12 @@ public void setupIndex() throws Exception { reader = DirectoryReader.open(dir); searcher = new IndexSearcher(reader); + Map> stats = new HashMap<>(); + stats.put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + stats.put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + ltrStats = new LTRStats(unmodifiableMap(stats)); } @After @@ -84,7 +99,7 @@ public void testQuery() throws Exception { Query q = new TermQuery(new Term("text", "cow")); String statsType = "sum_raw_tf"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 2 docs assertThat(searcher.count(eq), equalTo(2)); @@ -100,7 +115,7 @@ public void testQueryWithEmptyResults() throws Exception { String statsType = "sum_raw_tf"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 0 docs assertThat(searcher.count(eq), equalTo(0)); @@ -114,7 +129,7 @@ public void testQueryWithTermPositionAverage() throws Exception { Query q = new TermQuery(new Term("text", "dance")); String statsType = "avg_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 1 docs assertThat(searcher.count(eq), equalTo(1)); @@ -129,7 +144,7 @@ public void testQueryWithTermPositionMax() throws Exception { Query q = new TermQuery(new Term("text", "dance")); String statsType = "max_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 1 docs assertThat(searcher.count(eq), equalTo(1)); @@ -144,7 +159,7 @@ public void testQueryWithTermPositionMin() throws Exception { Query q = new TermQuery(new Term("text", "dance")); String statsType = "min_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 1 docs assertThat(searcher.count(eq), equalTo(1)); @@ -168,7 +183,7 @@ public void testQueryWithTermPositionMinWithTwoTerms() throws Exception { Query q = builder.build(); String statsType = "min_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Verify score is 5 (5 unique terms) TopDocs docs = searcher.search(eq, 4); @@ -189,7 +204,7 @@ public void testQueryWithTermPositionMaxWithTwoTerms() throws Exception { Query q = builder.build(); String statsType = "max_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Verify score is 5 (5 unique terms) TopDocs docs = searcher.search(eq, 4); @@ -210,7 +225,7 @@ public void testQueryWithTermPositionAvgWithTwoTerms() throws Exception { Query q = builder.build(); String statsType = "avg_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Verify score is 5 (5 unique terms) TopDocs docs = searcher.search(eq, 4); @@ -222,7 +237,7 @@ public void testQueryWithTermPositionAvgWithNoTerm() throws Exception { Query q = new TermQuery(new Term("text", "xxxxxxxxxxxxxxxxxx")); String statsType = "avg_raw_tp"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Basic query check, should match 1 docs assertThat(searcher.count(eq), equalTo(0)); @@ -246,7 +261,7 @@ public void testBooleanQuery() throws Exception { Query q = builder.build(); String statsType = "sum_raw_tf"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Verify tf score TopDocs docs = searcher.search(eq, 4); @@ -271,7 +286,7 @@ public void testUniqueTerms() throws Exception { Query q = builder.build(); String statsType = "unique_terms_count"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); // Verify score is 5 (5 unique terms) TopDocs docs = searcher.search(eq, 4); @@ -283,7 +298,7 @@ public void testInvalidStat() throws Exception { Query q = new TermQuery(new Term("text", "cow")); String statsType = "sum_invalid_stat"; - ExplorerQuery eq = new ExplorerQuery(q, statsType); + ExplorerQuery eq = new ExplorerQuery(q, statsType, ltrStats); expectThrows(RuntimeException.class, () -> searcher.search(eq, 4)); } diff --git a/src/test/java/com/o19s/es/ltr/logging/LoggingFetchSubPhaseTests.java b/src/test/java/com/o19s/es/ltr/logging/LoggingFetchSubPhaseTests.java index c4bd8d7..b25f8ed 100644 --- a/src/test/java/com/o19s/es/ltr/logging/LoggingFetchSubPhaseTests.java +++ b/src/test/java/com/o19s/es/ltr/logging/LoggingFetchSubPhaseTests.java @@ -16,6 +16,10 @@ package com.o19s.es.ltr.logging; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.o19s.es.ltr.feature.PrebuiltFeature; import com.o19s.es.ltr.feature.PrebuiltFeatureSet; import com.o19s.es.ltr.feature.PrebuiltLtrModel; @@ -47,6 +51,7 @@ import org.opensearch.common.lucene.search.function.CombineFunction; import org.opensearch.common.lucene.search.function.FieldValueFactorFunction; import org.opensearch.common.lucene.search.function.FunctionScoreQuery; +import org.opensearch.core.common.text.Text; import org.opensearch.index.fielddata.plain.SortedNumericIndexFieldData; import org.opensearch.search.SearchHit; import org.opensearch.search.fetch.FetchSubPhase; @@ -63,6 +68,7 @@ import java.util.Map; import java.util.UUID; +import static java.util.Collections.unmodifiableMap; import static org.opensearch.common.lucene.search.function.FieldValueFactorFunction.Modifier.LN2P; import static org.opensearch.index.fielddata.IndexNumericFieldData.NumericType.FLOAT; @@ -71,7 +77,12 @@ public class LoggingFetchSubPhaseTests extends LuceneTestCase { private static Directory directory; private static IndexSearcher searcher; private static Map docs; - + private LTRStats ltrStats = new LTRStats(unmodifiableMap(new HashMap>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); @BeforeClass public static void init() throws Exception { @@ -209,7 +220,7 @@ public RankerQuery buildQuery(String text) { features.add(new PrebuiltFeature("score_feat", buildFunctionScore())); PrebuiltFeatureSet set = new PrebuiltFeatureSet("my_set", features); LtrRanker ranker = LinearRankerTests.generateRandomRanker(set.size()); - return RankerQuery.build(new PrebuiltLtrModel("my_model", ranker, set)); + return RankerQuery.build(new PrebuiltLtrModel("my_model", ranker, set), ltrStats); } diff --git a/src/test/java/com/o19s/es/ltr/query/LtrQueryBuilderTests.java b/src/test/java/com/o19s/es/ltr/query/LtrQueryBuilderTests.java index 6e691fb..93cd7df 100644 --- a/src/test/java/com/o19s/es/ltr/query/LtrQueryBuilderTests.java +++ b/src/test/java/com/o19s/es/ltr/query/LtrQueryBuilderTests.java @@ -16,6 +16,10 @@ */ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.o19s.es.ltr.LtrQueryParserPlugin; import org.apache.lucene.search.Query; import org.opensearch.index.query.MatchAllQueryBuilder; @@ -34,8 +38,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import static java.util.Collections.unmodifiableMap; import static org.hamcrest.CoreMatchers.instanceOf; /** @@ -47,6 +53,12 @@ public class LtrQueryBuilderTests extends AbstractQueryTestCase protected Collection> getPlugins() { return Arrays.asList(LtrQueryParserPlugin.class, TestGeoShapeFieldMapperPlugin.class); } + private LTRStats ltrStats = new LTRStats(unmodifiableMap(new HashMap>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); private static final String simpleModel = "## LambdaMART\\n" + "## name:foo\\n" + @@ -125,6 +137,7 @@ public void testNamedFeatures() throws IOException { " } " + "}"; LtrQueryBuilder queryBuilder = (LtrQueryBuilder)parseQuery(ltrQuery); + queryBuilder.ltrStats(ltrStats); QueryShardContext context = createShardContext(); RankerQuery query = (RankerQuery)queryBuilder.toQuery(context); assertEquals(query.getFeature(0).name(), "bar_query"); @@ -152,6 +165,7 @@ public void testUnnamedFeatures() throws IOException { " } " + "}"; LtrQueryBuilder queryBuilder = (LtrQueryBuilder)parseQuery(ltrQuery); + queryBuilder.ltrStats(ltrStats); QueryShardContext context = createShardContext(); RankerQuery query = (RankerQuery)queryBuilder.toQuery(context); assertNull(query.getFeature(0).name()); @@ -186,6 +200,7 @@ protected LtrQueryBuilder doCreateTestQueryBuilder() { simpleModel.replace("\\\"", "\"") .replace("\\n", "\n"), Collections.emptyMap())); + builder.ltrStats(ltrStats); return builder; } @@ -214,7 +229,7 @@ public void testMustRewrite() throws IOException { features.add(new MatchQueryBuilder("test", "foo2")); } - LtrQueryBuilder builder = new LtrQueryBuilder(script, features); + LtrQueryBuilder builder = new LtrQueryBuilder(script, features, ltrStats); QueryBuilder rewritten = builder.rewrite(createShardContext()); if (!mustRewrite && features.isEmpty()) { // if it's empty we rewrite to match all diff --git a/src/test/java/com/o19s/es/ltr/query/LtrQueryTests.java b/src/test/java/com/o19s/es/ltr/query/LtrQueryTests.java index f970db4..a9fe0d9 100644 --- a/src/test/java/com/o19s/es/ltr/query/LtrQueryTests.java +++ b/src/test/java/com/o19s/es/ltr/query/LtrQueryTests.java @@ -24,6 +24,10 @@ import ciir.umass.edu.learning.RankerTrainer; import ciir.umass.edu.metric.NDCGScorer; import ciir.umass.edu.utilities.MyThreadPool; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.o19s.es.ltr.feature.FeatureSet; import com.o19s.es.ltr.feature.PrebuiltFeature; import com.o19s.es.ltr.feature.PrebuiltFeatureSet; @@ -93,7 +97,7 @@ import java.util.SortedMap; import java.util.TreeMap; import java.util.stream.Collectors; - +import static java.util.Collections.unmodifiableMap; @LuceneTestCase.SuppressSysoutChecks(bugUrl = "RankURL does this when training models... ") public class LtrQueryTests extends LuceneTestCase { @@ -111,6 +115,13 @@ private int[] range(int start, int stop) return result; } + private LTRStats ltrStats = new LTRStats(unmodifiableMap(new HashMap>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); + private Field newField(String name, String value, Store stored) { FieldType tagsFieldType = new FieldType(); tagsFieldType.setStored(stored == Store.YES); @@ -189,7 +200,7 @@ public void reset() { } } }; - RankerQuery query = RankerQuery.buildLogQuery(logger, set, null, Collections.emptyMap()); + RankerQuery query = RankerQuery.buildLogQuery(logger, set, null, Collections.emptyMap(), ltrStats); searcherUnderTest.search(query, new SimpleCollector() { @@ -364,7 +375,7 @@ private RankerQuery toRankerQuery(List features, Ranker ranker, ltrRanker = new FeatureNormalizingRanker(ltrRanker, ftrNorms); } PrebuiltLtrModel model = new PrebuiltLtrModel(ltrRanker.name(), ltrRanker, new PrebuiltFeatureSet(null, features)); - return RankerQuery.build(model); + return RankerQuery.build(model, ltrStats); } public void testTrainModel() throws IOException { diff --git a/src/test/java/com/o19s/es/ltr/query/StoredLtrQueryBuilderTests.java b/src/test/java/com/o19s/es/ltr/query/StoredLtrQueryBuilderTests.java index c40d399..9eb029a 100644 --- a/src/test/java/com/o19s/es/ltr/query/StoredLtrQueryBuilderTests.java +++ b/src/test/java/com/o19s/es/ltr/query/StoredLtrQueryBuilderTests.java @@ -16,6 +16,10 @@ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.o19s.es.ltr.LtrQueryParserPlugin; import com.o19s.es.ltr.LtrTestUtils; import com.o19s.es.ltr.feature.store.CompiledLtrModel; @@ -60,11 +64,18 @@ import java.util.Set; import java.util.stream.Collectors; +import static java.util.Collections.unmodifiableMap; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; public class StoredLtrQueryBuilderTests extends AbstractQueryTestCase { private static final MemStore store = new MemStore(); + private LTRStats ltrStats = new LTRStats(unmodifiableMap(new HashMap>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); // TODO: Remove the TestGeoShapeFieldMapperPlugin once upstream has completed the migration. protected Collection> getPlugins() { @@ -125,11 +136,13 @@ protected StoredLtrQueryBuilder doCreateTestQueryBuilder() { Map params = new HashMap<>(); params.put("query_string", "a wonderful query"); builder.params(params); + builder.ltrStats(ltrStats); return builder; } public void testMissingParams() { StoredLtrQueryBuilder builder = new StoredLtrQueryBuilder(LtrTestUtils.wrapMemStore(StoredLtrQueryBuilderTests.store)); + builder.ltrStats(ltrStats); builder.modelName("model1"); assertThat(expectThrows(IllegalArgumentException.class, () -> builder.toQuery(createShardContext())).getMessage(), @@ -147,6 +160,7 @@ public void testInvalidActiveFeatures() { StoredLtrQueryBuilder builder = new StoredLtrQueryBuilder(LtrTestUtils.wrapMemStore(StoredLtrQueryBuilderTests.store)); builder.modelName("model1"); builder.activeFeatures(Collections.singletonList("non_existent_feature")); + builder.ltrStats(ltrStats); assertThat(expectThrows(IllegalArgumentException.class, () -> builder.toQuery(createShardContext())).getMessage(), equalTo("Feature: [non_existent_feature] provided in active_features does not exist")); } @@ -161,7 +175,7 @@ public void testSerDe() throws IOException { BytesRef ref = out.bytes().toBytesRef(); StreamInput input = ByteBufferStreamInput.wrap(ref.bytes, ref.offset, ref.length); StoredLtrQueryBuilder builderFromInputStream = new StoredLtrQueryBuilder( - LtrTestUtils.wrapMemStore(StoredLtrQueryBuilderTests.store), input); + LtrTestUtils.wrapMemStore(StoredLtrQueryBuilderTests.store), input, ltrStats); List expected = Collections.singletonList("match1"); assertEquals(expected, builderFromInputStream.activeFeatures()); } @@ -183,6 +197,7 @@ private void assertQueryClass(Class clazz, boolean setActiveFeature) throws I if (setActiveFeature) { builder.activeFeatures(Arrays.asList("match1", "match2")); } + builder.ltrStats(ltrStats); RankerQuery rankerQuery = builder.doToQuery(createShardContext()); List queries = rankerQuery.stream().collect(Collectors.toList()); diff --git a/src/test/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilderTests.java b/src/test/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilderTests.java index c5d3e63..ab1a5fb 100644 --- a/src/test/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilderTests.java +++ b/src/test/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilderTests.java @@ -16,6 +16,10 @@ package com.o19s.es.ltr.query; +import org.opensearch.ltr.stats.LTRStat; +import org.opensearch.ltr.stats.LTRStats; +import org.opensearch.ltr.stats.StatName; +import org.opensearch.ltr.stats.suppliers.CounterSupplier; import com.carrotsearch.randomizedtesting.RandomizedRunner; import com.o19s.es.ltr.LtrQueryParserPlugin; import com.o19s.es.ltr.feature.FeatureValidation; @@ -48,6 +52,7 @@ import java.util.stream.IntStream; import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableMap; import static java.util.stream.Collectors.joining; import static org.hamcrest.CoreMatchers.instanceOf; @@ -57,6 +62,13 @@ public class ValidatingLtrQueryBuilderTests extends AbstractQueryTestCase>() {{ + put(StatName.LTR_REQUEST_TOTAL_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + put(StatName.LTR_REQUEST_ERROR_COUNT.getName(), + new LTRStat<>(false, new CounterSupplier())); + }})); + // TODO: Remove the TestGeoShapeFieldMapperPlugin once upstream has completed the migration. protected Collection> getPlugins() { return asList(LtrQueryParserPlugin.class, TestGeoShapeFieldMapperPlugin.class); @@ -106,7 +118,7 @@ protected ValidatingLtrQueryBuilder doCreateTestQueryBuilder() { Map params = new HashMap<>(); params.put("query_string", "hello world"); FeatureValidation val = new FeatureValidation("test_index", params); - return new ValidatingLtrQueryBuilder(element, val, factory); + return new ValidatingLtrQueryBuilder(element, val, factory, ltrStats); } @Override diff --git a/src/test/resources/rest-api-spec/test/fstore/90_get_stats.yml b/src/test/resources/rest-api-spec/test/fstore/90_get_stats.yml deleted file mode 100644 index 3b8347e..0000000 --- a/src/test/resources/rest-api-spec/test/fstore/90_get_stats.yml +++ /dev/null @@ -1,172 +0,0 @@ ---- -setup: - - do: - indices.create: - index: test - - - do: - index: - index: test - id: 1 - body: { "field1": "v1", "field2": "v2", "field3": "some text", "user_rating": 5.2 } - - - do: - index: - index: test - id: 2 - body: { "field1": "v1 aoeu", "field2": " ua u v2", "field3": "foo bar text", "user_rating": 0.0 } - - - do: - ltr.create_store: {} - - do: - ltr.create_feature: - name: feature1 - body: - feature: - params: - - query_string - template: - match: - field1: "{{query_string}}" - - do: - ltr.create_feature: - name: feature2 - body: - feature: - params: - - query_string - template: - match: - field2: "{{query_string}}" - - do: - ltr.create_featureset: - name: my_featureset - body: - featureset: - name: my_featureset - - do: - ltr.add_features_to_set: - name: my_featureset - query: feature1 - - do: - ltr.add_features_to_set: - name: my_featureset - query: feature2 - - do: - ltr.add_features_to_set: - name: my_featureset - body: - features: - - name: user_rating - params: query_string - template: {"function_score": { "functions": [ {"field_value_factor": { "field": "user_rating" } }], "query": {"match_all": {}}}} - - - do: - ltr.add_features_to_set: - name: my_featureset - body: - features: - - name: no_param_feature - params: [] - template: {"function_score": { "functions": [ {"field_value_factor": { "field": "user_rating" } }], "query": {"match_all": {}}}} - - - do: - indices.refresh: {} - - - do: - ltr.create_model_from_set: - name: my_featureset - body: - model: - name: single_feature_ranklib_model - model: - type: model/ranklib - definition: | - ## LambdaMART - ## No. of trees = 1 - ## No. of leaves = 1 - ## No. of threshold candidates = 256 - ## Learning rate = 0.1 - ## Stop early = 100 - - - - - 1 - 1.0 - - 2.0 - - - 4.0 - - - - - - - - do: - ltr.create_model_from_set: - name: my_featureset - body: - model: - name: single_feature_linear_model - model: - type: model/linear - definition: - feature1: 1.3 - - # Model that uses three features. - - do: - ltr.create_model_from_set: - name: my_featureset - body: - model: - name: three_feature_linear_model - model: - type: model/linear - definition: - feature1: 1.3 - feature2: 2.3 - no_param_feature: 3.0 - - - do: - indices.refresh: {} - ---- -"Get all stats": - - do: - ltr.get_stats: {} - - set: - nodes._arbitrary_key_: node_id - - match: { status: "green" } - - length: { stores: 1 } - - match: { stores._default_.status: "green" } - - match: { stores._default_.featureset_count: 1 } - - match: { stores._default_.feature_count: 2 } - - match: { stores._default_.model_count: 3 } - - is_true: nodes.$node_id.cache - - is_true: nodes.$node_id.cache.feature - - is_true: nodes.$node_id.cache.featureset - - is_true: nodes.$node_id.cache.model - - gte: {nodes.$node_id.cache.feature.hit_count: 0 } - - gte: {nodes.$node_id.cache.feature.miss_count: 0 } - - gte: {nodes.$node_id.cache.feature.eviction_count: 0 } - - gte: {nodes.$node_id.cache.feature.entry_count: 0 } - - gte: {nodes.$node_id.cache.feature.memory_usage_in_bytes: 0 } - - gte: {nodes.$node_id.cache.featureset.hit_count: 0 } - - gte: {nodes.$node_id.cache.featureset.miss_count: 0 } - - gte: {nodes.$node_id.cache.featureset.eviction_count: 0 } - - gte: {nodes.$node_id.cache.featureset.entry_count: 0 } - - gte: {nodes.$node_id.cache.featureset.memory_usage_in_bytes: 0 } - - gte: {nodes.$node_id.cache.model.hit_count: 0 } - - gte: {nodes.$node_id.cache.model.miss_count: 0 } - - gte: {nodes.$node_id.cache.model.eviction_count: 0 } - - gte: {nodes.$node_id.cache.model.entry_count: 0 } - - gte: {nodes.$node_id.cache.model.memory_usage_in_bytes: 0 } ---- -"Get an individual stat - plugin status": - - do: - ltr.get_stats: - stat: "status" - - match: { status: "green" } From ecaed862bc1fcc7139920b28879d76f9ff5bb4e7 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Wed, 4 Dec 2024 00:08:48 +0100 Subject: [PATCH 3/8] fix: refactored method to be private Signed-off-by: Johannes Peter --- src/main/java/com/o19s/es/explore/ExplorerQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/o19s/es/explore/ExplorerQuery.java b/src/main/java/com/o19s/es/explore/ExplorerQuery.java index 869baed..9389428 100644 --- a/src/main/java/com/o19s/es/explore/ExplorerQuery.java +++ b/src/main/java/com/o19s/es/explore/ExplorerQuery.java @@ -108,7 +108,7 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo } } - public Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { + private Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!scoreMode.needsScores()) { return searcher.createWeight(query, scoreMode, boost); } From e197fca437dbc01d626596a254921ea29f0aa2c0 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Wed, 4 Dec 2024 08:17:17 +0100 Subject: [PATCH 4/8] fix: refactored methods with leading _ to *Internal Signed-off-by: Johannes Peter --- src/main/java/com/o19s/es/explore/ExplorerQuery.java | 4 ++-- src/main/java/com/o19s/es/ltr/query/RankerQuery.java | 4 ++-- .../java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java | 4 ++-- .../java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/o19s/es/explore/ExplorerQuery.java b/src/main/java/com/o19s/es/explore/ExplorerQuery.java index 9389428..d22d6d7 100644 --- a/src/main/java/com/o19s/es/explore/ExplorerQuery.java +++ b/src/main/java/com/o19s/es/explore/ExplorerQuery.java @@ -101,14 +101,14 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo } try { - return _createWeight(searcher, scoreMode, boost); + return createWeightInternal(searcher, scoreMode, boost); } catch (Exception e) { ltrStats.getStats().get(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); throw e; } } - private Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { + private Weight createWeightInternal(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!scoreMode.needsScores()) { return searcher.createWeight(query, scoreMode, boost); } diff --git a/src/main/java/com/o19s/es/ltr/query/RankerQuery.java b/src/main/java/com/o19s/es/ltr/query/RankerQuery.java index 7cf2fb1..cea4d95 100644 --- a/src/main/java/com/o19s/es/ltr/query/RankerQuery.java +++ b/src/main/java/com/o19s/es/ltr/query/RankerQuery.java @@ -236,14 +236,14 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo } try { - return _createWeight(searcher, scoreMode, boost); + return createWeightInternal(searcher, scoreMode, boost); } catch (Exception e) { ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); throw e; } } - private Weight _createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { + private Weight createWeightInternal(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { if (!scoreMode.needsScores()) { // If scores are not needed simply return a constant score on all docs return new ConstantScoreWeight(this, boost) { diff --git a/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java b/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java index 2160da7..50f3651 100644 --- a/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java +++ b/src/main/java/com/o19s/es/ltr/query/StoredLtrQueryBuilder.java @@ -165,14 +165,14 @@ private static void validateActiveFeatures(FeatureSet features, LtrQueryContext protected RankerQuery doToQuery(QueryShardContext context) throws IOException { this.ltrStats.getStat(StatName.LTR_REQUEST_TOTAL_COUNT.getName()).increment(); try { - return _doToQuery(context); + return doToQueryInternal(context); } catch (Exception e) { ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); throw e; } } - private RankerQuery _doToQuery(QueryShardContext context) throws IOException { + private RankerQuery doToQueryInternal(QueryShardContext context) throws IOException { String indexName = storeName != null ? IndexFeatureStore.indexName(storeName) : IndexFeatureStore.DEFAULT_STORE; FeatureStore store = storeLoader.load(indexName, context::getClient); LtrQueryContext ltrQueryContext = new LtrQueryContext(context, diff --git a/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java b/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java index 6948c78..534a48e 100644 --- a/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java +++ b/src/main/java/com/o19s/es/ltr/query/ValidatingLtrQueryBuilder.java @@ -170,14 +170,14 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep protected Query doToQuery(QueryShardContext queryShardContext) throws IOException { ltrStats.getStat(StatName.LTR_REQUEST_TOTAL_COUNT.getName()).increment(); try { - return _doToQuery(queryShardContext); + return doToQueryInternal(queryShardContext); } catch (Exception e) { ltrStats.getStat(StatName.LTR_REQUEST_ERROR_COUNT.getName()).increment(); throw e; } } - private Query _doToQuery(QueryShardContext queryShardContext) throws IOException { + private Query doToQueryInternal(QueryShardContext queryShardContext) throws IOException { //TODO: should we be passing activeFeatures here? LtrQueryContext context = new LtrQueryContext(queryShardContext); if (StoredFeature.TYPE.equals(element.type())) { From e22ab980bb6134c1964dd492cc87321df8de2a57 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Wed, 4 Dec 2024 10:47:55 +0100 Subject: [PATCH 5/8] fix: added documentation to deprecated classes Signed-off-by: Johannes Peter --- src/main/java/com/o19s/es/ltr/stats/LTRStat.java | 5 ++++- src/main/java/com/o19s/es/ltr/stats/LTRStats.java | 5 ++++- src/main/java/com/o19s/es/ltr/stats/StatName.java | 6 +++++- .../es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java | 5 ++++- .../es/ltr/stats/suppliers/PluginHealthStatusSupplier.java | 1 - .../com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java | 1 - 6 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/o19s/es/ltr/stats/LTRStat.java b/src/main/java/com/o19s/es/ltr/stats/LTRStat.java index 71ea1fa..7eefb09 100644 --- a/src/main/java/com/o19s/es/ltr/stats/LTRStat.java +++ b/src/main/java/com/o19s/es/ltr/stats/LTRStat.java @@ -21,8 +21,11 @@ * A container for a stat provided by the plugin. Each instance is associated with * an underlying supplier. The stat instance also stores a flag to indicate whether * this is a cluster level or a node level stat. + * + * @deprecated This class is outdated since 3.0.0-3.0.0 and will be removed in the future. + * Please use the new stats framework in the {@link org.opensearch.ltr.stats} package. */ -@Deprecated +@Deprecated(since = "3.0.0-3.0.0", forRemoval = true) public class LTRStat { private final boolean clusterLevel; private final Supplier supplier; diff --git a/src/main/java/com/o19s/es/ltr/stats/LTRStats.java b/src/main/java/com/o19s/es/ltr/stats/LTRStats.java index e2ca08c..3e0ce2f 100644 --- a/src/main/java/com/o19s/es/ltr/stats/LTRStats.java +++ b/src/main/java/com/o19s/es/ltr/stats/LTRStats.java @@ -21,8 +21,11 @@ /** * This class is the main entry-point for access to the stats that the LTR plugin keeps track of. + * + * @deprecated This class is outdated since 3.0.0-3.0.0 and will be removed in the future. + * Please use the new stats framework in the {@link org.opensearch.ltr.stats} package. */ -@Deprecated +@Deprecated(since = "3.0.0-3.0.0", forRemoval = true) public class LTRStats { private final Map stats; diff --git a/src/main/java/com/o19s/es/ltr/stats/StatName.java b/src/main/java/com/o19s/es/ltr/stats/StatName.java index 0b5fa3f..36c0bec 100644 --- a/src/main/java/com/o19s/es/ltr/stats/StatName.java +++ b/src/main/java/com/o19s/es/ltr/stats/StatName.java @@ -19,7 +19,11 @@ import java.util.HashSet; import java.util.Set; -@Deprecated +/** + * @deprecated This class is outdated since 3.0.0-3.0.0 and will be removed in the future. + * Please use the new stats framework in the {@link org.opensearch.ltr.stats} package. + */ +@Deprecated(since = "3.0.0-3.0.0", forRemoval = true) public enum StatName { PLUGIN_STATUS("status"), STORES("stores"), diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java index 926d16d..cc62ebd 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java @@ -25,8 +25,11 @@ /** * Aggregate stats on the cache used by the plugin per node. + * + * @deprecated This class is outdated since 3.0.0-3.0.0 and will be removed in the future. + * Please use the new stats framework in the {@link org.opensearch.ltr.stats} package. */ -@Deprecated +@Deprecated(since = "3.0.0-3.0.0", forRemoval = true) public class CacheStatsOnNodeSupplier implements Supplier>> { private final Caches caches; diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java index af9f184..ebf588a 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/PluginHealthStatusSupplier.java @@ -29,7 +29,6 @@ /** * Supplier for an overall plugin health status. */ -@Deprecated public class PluginHealthStatusSupplier implements Supplier { private static final String STATUS_GREEN = "green"; private static final String STATUS_YELLOW = "yellow"; diff --git a/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java b/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java index 72dd842..efac97d 100644 --- a/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java +++ b/src/main/java/com/o19s/es/ltr/stats/suppliers/StoreStatsSupplier.java @@ -49,7 +49,6 @@ * information such as the index health and count of feature sets, features and * models in the store. */ -@Deprecated public class StoreStatsSupplier implements Supplier>> { private static final Logger LOG = LogManager.getLogger(StoreStatsSupplier.class); private static final String AGG_FIELD = "type"; From f54c5d8add47df39cd4393e5d11a024a6490b466 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Wed, 4 Dec 2024 10:57:37 +0100 Subject: [PATCH 6/8] fix: minor fixes, converted boolean var to primitive, finalized variables Signed-off-by: Johannes Peter --- src/main/java/org/opensearch/ltr/stats/LTRStat.java | 4 ++-- src/main/java/org/opensearch/ltr/stats/LTRStats.java | 2 +- src/main/java/org/opensearch/ltr/stats/StatName.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/ltr/stats/LTRStat.java b/src/main/java/org/opensearch/ltr/stats/LTRStat.java index 56c57d7..49e8577 100644 --- a/src/main/java/org/opensearch/ltr/stats/LTRStat.java +++ b/src/main/java/org/opensearch/ltr/stats/LTRStat.java @@ -23,8 +23,8 @@ * Class represents a stat the plugin keeps track of */ public class LTRStat { - private Boolean clusterLevel; - private Supplier supplier; + private final boolean clusterLevel; + private final Supplier supplier; /** * Constructor diff --git a/src/main/java/org/opensearch/ltr/stats/LTRStats.java b/src/main/java/org/opensearch/ltr/stats/LTRStats.java index 0e21d80..d97fe55 100644 --- a/src/main/java/org/opensearch/ltr/stats/LTRStats.java +++ b/src/main/java/org/opensearch/ltr/stats/LTRStats.java @@ -22,7 +22,7 @@ * This class is the main entry-point for access to the stats that the LTR plugin keeps track of. */ public class LTRStats { - private Map> stats; + private final Map> stats; /** * Constructor diff --git a/src/main/java/org/opensearch/ltr/stats/StatName.java b/src/main/java/org/opensearch/ltr/stats/StatName.java index d04bbd0..b5a51e9 100644 --- a/src/main/java/org/opensearch/ltr/stats/StatName.java +++ b/src/main/java/org/opensearch/ltr/stats/StatName.java @@ -26,7 +26,7 @@ public enum StatName { LTR_REQUEST_ERROR_COUNT("request_error_count"), LTR_CACHE_STATS("cache"); - private String name; + private final String name; StatName(String name) { this.name = name; From d98f536db8431532f87a4a3a0486a813ba234e5e Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Wed, 4 Dec 2024 11:00:50 +0100 Subject: [PATCH 7/8] fix: added null checks to LTRStats Signed-off-by: Johannes Peter --- src/main/java/org/opensearch/ltr/stats/LTRStats.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/ltr/stats/LTRStats.java b/src/main/java/org/opensearch/ltr/stats/LTRStats.java index d97fe55..2d44492 100644 --- a/src/main/java/org/opensearch/ltr/stats/LTRStats.java +++ b/src/main/java/org/opensearch/ltr/stats/LTRStats.java @@ -49,7 +49,11 @@ public Map> getStats() { * @return LTRStat * @throws IllegalArgumentException thrown on illegal statName */ - public LTRStat getStat(String key) throws IllegalArgumentException { + public LTRStat getStat(final String key) throws IllegalArgumentException { + if (key == null) { + throw new IllegalArgumentException("Stat name cannot be null"); + } + if (!stats.containsKey(key)) { throw new IllegalArgumentException("Stat=\"" + key + "\" does not exist"); } @@ -62,6 +66,10 @@ public LTRStat getStat(String key) throws IllegalArgumentException { * @param stat Stat */ public void addStats(String key, LTRStat stat) { + if (key == null) { + throw new IllegalArgumentException("Stat name cannot be null"); + } + this.stats.put(key, stat); } From 8dc69d0f05dc3a2201b3724675888a4c8db426a2 Mon Sep 17 00:00:00 2001 From: Johannes Peter Date: Thu, 5 Dec 2024 10:40:39 +0100 Subject: [PATCH 8/8] fix: pulled hard-coded strings out of method and refactored to proper class variables for class CacheStatsOnNodeSupplier Signed-off-by: Johannes Peter --- .../suppliers/CacheStatsOnNodeSupplier.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java b/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java index 4d54f0a..7c33418 100644 --- a/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java +++ b/src/main/java/org/opensearch/ltr/stats/suppliers/CacheStatsOnNodeSupplier.java @@ -27,13 +27,18 @@ * Aggregate stats on the cache used by the plugin per node. */ public class CacheStatsOnNodeSupplier implements Supplier>> { - private static final String LTR_CACHE_HIT_COUNT = "hit_count"; - private static final String LTR_CACHE_MISS_COUNT = "miss_count"; - private static final String LTR_CACHE_EVICTION_COUNT = "eviction_count"; - private static final String LTR_CACHE_ENTRY_COUNT = "entry_count"; - private static final String LTR_CACHE_MEMORY_USAGE_IN_BYTES = "memory_usage_in_bytes"; - private Caches caches; + private static final String LTR_CACHE_OBJECT_FEATURE = "feature"; + private static final String LTR_CACHE_OBJECT_FEATURESET = "featureset"; + private static final String LTR_CACHE_OBJECT_MODEL = "model"; + + private static final String LTR_CACHE_METRIC_HIT_COUNT = "hit_count"; + private static final String LTR_CACHE_METRIC_MISS_COUNT = "miss_count"; + private static final String LTR_CACHE_METRIC_EVICTION_COUNT = "eviction_count"; + private static final String LTR_CACHE_METRIC_ENTRY_COUNT = "entry_count"; + private static final String LTR_CACHE_METRIC_MEMORY_USAGE_IN_BYTES = "memory_usage_in_bytes"; + + private final Caches caches; public CacheStatsOnNodeSupplier(Caches caches) { this.caches = caches; @@ -42,19 +47,19 @@ public CacheStatsOnNodeSupplier(Caches caches) { @Override public Map> get() { Map> values = new HashMap<>(); - values.put("feature", getCacheStats(caches.featureCache())); - values.put("featureset", getCacheStats(caches.featureSetCache())); - values.put("model", getCacheStats(caches.modelCache())); + values.put(LTR_CACHE_OBJECT_FEATURE, getCacheStats(caches.featureCache())); + values.put(LTR_CACHE_OBJECT_FEATURESET, getCacheStats(caches.featureSetCache())); + values.put(LTR_CACHE_OBJECT_MODEL, getCacheStats(caches.modelCache())); return Collections.unmodifiableMap(values); } private Map getCacheStats(Cache cache) { Map stat = new HashMap<>(); - stat.put(LTR_CACHE_HIT_COUNT, cache.stats().getHits()); - stat.put(LTR_CACHE_MISS_COUNT, cache.stats().getMisses()); - stat.put(LTR_CACHE_EVICTION_COUNT, cache.stats().getEvictions()); - stat.put(LTR_CACHE_ENTRY_COUNT, cache.count()); - stat.put(LTR_CACHE_MEMORY_USAGE_IN_BYTES, cache.weight()); + stat.put(LTR_CACHE_METRIC_HIT_COUNT, cache.stats().getHits()); + stat.put(LTR_CACHE_METRIC_MISS_COUNT, cache.stats().getMisses()); + stat.put(LTR_CACHE_METRIC_EVICTION_COUNT, cache.stats().getEvictions()); + stat.put(LTR_CACHE_METRIC_ENTRY_COUNT, cache.count()); + stat.put(LTR_CACHE_METRIC_MEMORY_USAGE_IN_BYTES, cache.weight()); return Collections.unmodifiableMap(stat); } }