Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit d540f28

Browse files
committedMar 4, 2025·
Clean up files
Signed-off-by: Andy Qin <qinandy@amazon.com>
1 parent 56c2d19 commit d540f28

27 files changed

+87
-68
lines changed
 

‎DEVELOPER_GUIDE.md

+12-12
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,9 @@ through the same build issue.
351351

352352
### Class and package names
353353

354-
Class names should use `CamelCase`.
354+
Class names should use `CamelCase`.
355355

356-
Try to put new classes into existing packages if package name abstracts the purpose of the class.
356+
Try to put new classes into existing packages if package name abstracts the purpose of the class.
357357

358358
Example of good class file name and package utilization:
359359

@@ -371,7 +371,7 @@ methods rather than a long single one and does everything.
371371
### Documentation
372372

373373
Document you code. That includes purpose of new classes, every public method and code sections that have critical or non-trivial
374-
logic (check this example https://github.com/opensearch-project/neural-search/blob/main/src/main/java/org/opensearch/neuralsearch/query/NeuralQueryBuilder.java#L238).
374+
logic (check this example https://github.com/opensearch-project/neural-search/blob/main/src/main/java/org/opensearch/neuralsearch/query/NeuralQueryBuilder.java#L238).
375375

376376
When you submit a feature PR, please submit a new
377377
[documentation issue](https://github.com/opensearch-project/documentation-website/issues/new/choose). This is a path for the documentation to be published as part of https://opensearch.org/docs/latest/ documentation site.
@@ -384,17 +384,17 @@ For the most part, we're using common conventions for Java projects. Here are a
384384

385385
1. Use descriptive names for classes, methods, fields, and variables.
386386
2. Avoid abbreviations unless they are widely accepted
387-
3. Use `final` on all method arguments unless it's absolutely necessary
387+
3. Use `final` on all method arguments unless it's absolutely necessary
388388
4. Wildcard imports are not allowed.
389389
5. Static imports are preferred over qualified imports when using static methods
390390
6. Prefer creating non-static public methods whenever possible. Avoid static methods in general, as they can often serve as shortcuts.
391391
Static methods are acceptable if they are private and do not access class state.
392-
7. Use functional programming style inside methods unless it's a performance critical section.
392+
7. Use functional programming style inside methods unless it's a performance critical section.
393393
8. For parameters of lambda expression please use meaningful names instead of shorten cryptic ones.
394394
9. Use Optional for return values if the value may not be present. This should be preferred to returning null.
395395
10. Do not create checked exceptions, and do not throw checked exceptions from public methods whenever possible. In general, if you call a method with a checked exception, you should wrap that exception into an unchecked exception.
396396
11. Throwing checked exceptions from private methods is acceptable.
397-
12. Use String.format when a string includes parameters, and prefer this over direct string concatenation. Always specify a Locale with String.format;
397+
12. Use String.format when a string includes parameters, and prefer this over direct string concatenation. Always specify a Locale with String.format;
398398
as a rule of thumb, use Locale.ROOT.
399399
13. Prefer Lombok annotations to the manually written boilerplate code
400400
14. When throwing an exception, avoid including user-provided content in the exception message. For secure coding practices,
@@ -440,17 +440,17 @@ Fix any new warnings before submitting your PR to ensure proper code documentati
440440

441441
### Tests
442442

443-
Write unit and integration tests for your new functionality.
443+
Write unit and integration tests for your new functionality.
444444

445445
Unit tests are preferred as they are cheap and fast, try to use them to cover all possible
446-
combinations of parameters. Utilize mocks to mimic dependencies.
446+
combinations of parameters. Utilize mocks to mimic dependencies.
447447

448-
Integration tests should be used sparingly, focusing primarily on the main (happy path) scenario or cases where extensive
449-
mocking is impractical. Include one or two unhappy paths to confirm that correct response codes are returned to the user.
450-
Whenever possible, favor scenarios that do not require model deployment. If model deployment is necessary, use an existing
448+
Integration tests should be used sparingly, focusing primarily on the main (happy path) scenario or cases where extensive
449+
mocking is impractical. Include one or two unhappy paths to confirm that correct response codes are returned to the user.
450+
Whenever possible, favor scenarios that do not require model deployment. If model deployment is necessary, use an existing
451451
model, as tests involving new model deployments are the most resource-intensive.
452452

453-
If your changes could affect backward compatibility, please include relevant backward compatibility tests along with your
453+
If your changes could affect backward compatibility, please include relevant backward compatibility tests along with your
454454
PR. For guidance on adding these tests, refer to the [Backwards Compatibility Testing](#backwards-compatibility-testing) section in this guide.
455455

456456
### Outdated or irrelevant code

‎src/main/java/org/opensearch/neuralsearch/rest/RestNeuralStatsHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ public class RestNeuralStatsHandler extends BaseRestHandler {
4141

4242
private static final Set<String> EVENT_STAT_NAMES = EnumSet.allOf(EventStatName.class)
4343
.stream()
44-
.map(EventStatName::getName)
44+
.map(EventStatName::getNameString)
4545
.map(String::toLowerCase)
4646
.collect(Collectors.toSet());
4747

4848
private static final Set<String> STATE_STAT_NAMES = EnumSet.allOf(StateStatName.class)
4949
.stream()
50-
.map(StateStatName::getName)
50+
.map(StateStatName::getNameString)
5151
.map(String::toLowerCase)
5252
.collect(Collectors.toSet());
5353

‎src/main/java/org/opensearch/neuralsearch/stats/common/StatName.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public interface StatName {
1313
* Gets the name of the stat. These must be unique to support user request stat filtering.
1414
* @return the name of the stat
1515
*/
16-
String getName();
16+
String getNameString();
1717

1818
/**
1919
* Gets the path of the stat in dot notation.

‎src/main/java/org/opensearch/neuralsearch/stats/common/StatType.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ public interface StatType {
1313
* Get the name of the stat type containing info about the type and how to process it
1414
* @return name of the stat type
1515
*/
16-
String getName();
16+
String getTypeString();
1717
}

‎src/main/java/org/opensearch/neuralsearch/stats/events/EventStat.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55
package org.opensearch.neuralsearch.stats.events;
66

7+
import org.opensearch.neuralsearch.stats.common.StatSnapshot;
8+
79
/**
810
* Interface for event stats. These contain logic to store and update ongoing event information.
911
*/
@@ -18,7 +20,7 @@ public interface EventStat {
1820
* Returns a snapshot of the stat. Used to cross transport layer/rest layer
1921
* @return
2022
*/
21-
TimestampedEventStatSnapshot getEventStatSnapshot();
23+
StatSnapshot<?> getStatSnapshot();
2224

2325
/**
2426
* Increments the stat

‎src/main/java/org/opensearch/neuralsearch/stats/events/EventStatName.java

+11-8
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,24 @@
1919
public enum EventStatName implements StatName {
2020
TEXT_EMBEDDING_PROCESSOR_EXECUTIONS("text_embedding_executions", "processors.ingest", EventStatType.TIMESTAMPED_COUNTER);
2121

22-
private final String name;
22+
private final String nameString;
2323
private final String path;
2424
private final EventStatType statType;
2525

26+
/**
27+
* Enum lookup table by nameString
28+
*/
2629
private static final Map<String, EventStatName> BY_NAME = Arrays.stream(values())
27-
.collect(Collectors.toMap(stat -> stat.name, stat -> stat));
30+
.collect(Collectors.toMap(stat -> stat.nameString, stat -> stat));
2831

2932
/**
3033
* Constructor
31-
* @param name the unique name of the stat.
34+
* @param nameString the unique name of the stat.
3235
* @param path the unique path of the stat
3336
* @param statType the category of stat
3437
*/
35-
EventStatName(String name, String path, EventStatType statType) {
36-
this.name = name;
38+
EventStatName(String nameString, String path, EventStatType statType) {
39+
this.nameString = nameString;
3740
this.path = path;
3841
this.statType = statType;
3942
}
@@ -57,13 +60,13 @@ public static EventStatName from(String name) {
5760
*/
5861
public String getFullPath() {
5962
if (StringUtils.isBlank(path)) {
60-
return name;
63+
return nameString;
6164
}
62-
return String.join(".", path, name);
65+
return String.join(".", path, nameString);
6366
}
6467

6568
@Override
6669
public String toString() {
67-
return getName();
70+
return getNameString();
6871
}
6972
}

‎src/main/java/org/opensearch/neuralsearch/stats/events/EventStatType.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public enum EventStatType implements StatType {
1818
* Gets the name of the stat type, the enum name in lowercase
1919
* @return the name of the stat type
2020
*/
21-
public String getName() {
21+
public String getTypeString() {
2222
return name().toLowerCase(Locale.ROOT);
2323
}
2424
}

‎src/main/java/org/opensearch/neuralsearch/stats/events/EventStatsManager.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import lombok.Getter;
88
import org.opensearch.neuralsearch.settings.NeuralSearchSettingsAccessor;
9+
import org.opensearch.neuralsearch.stats.common.StatSnapshot;
910

1011
import java.util.EnumSet;
1112
import java.util.HashMap;
@@ -79,13 +80,16 @@ public void inc(EventStatName eventStatName) {
7980
* @param statsToRetrieve Set of event stat names to retrieve data for
8081
* @return Map of event stat names to their current snapshots
8182
*/
82-
public Map<EventStatName, TimestampedEventStatSnapshot> getEventStatSnapshots(EnumSet<EventStatName> statsToRetrieve) {
83+
public Map<EventStatName, TimestampedEventStatSnapshot> getTimestampedEventStatSnapshots(EnumSet<EventStatName> statsToRetrieve) {
8384
// Filter stats based on passed in collection
8485
Map<EventStatName, TimestampedEventStatSnapshot> eventStatsDataMap = new HashMap<>();
8586
for (EventStatName statName : statsToRetrieve) {
86-
if (stats.containsKey(statName)) {
87-
// Get event data snapshot
88-
eventStatsDataMap.put(statName, stats.get(statName).getEventStatSnapshot());
87+
if (stats.containsKey(statName) && statName.getStatType() == EventStatType.TIMESTAMPED_COUNTER) {
88+
StatSnapshot<?> snapshot = stats.get(statName).getStatSnapshot();
89+
if (snapshot instanceof TimestampedEventStatSnapshot) {
90+
// Get event data snapshot
91+
eventStatsDataMap.put(statName, (TimestampedEventStatSnapshot) snapshot);
92+
}
8993
}
9094
}
9195
return eventStatsDataMap;

‎src/main/java/org/opensearch/neuralsearch/stats/events/TimestampedEventStat.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public long getMinutesSinceLastEvent() {
119119
* Gets the StatSnapshot for the event stat data
120120
* @return
121121
*/
122-
public TimestampedEventStatSnapshot getEventStatSnapshot() {
122+
public TimestampedEventStatSnapshot getStatSnapshot() {
123123
return TimestampedEventStatSnapshot.builder()
124124
.statName(statName)
125125
.value(getValue())

‎src/main/java/org/opensearch/neuralsearch/stats/events/TimestampedEventStatSnapshot.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public static TimestampedEventStatSnapshot aggregateEventStatSnapshots(Collectio
126126
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
127127
builder.startObject();
128128
builder.field(StatSnapshot.VALUE_FIELD, value);
129-
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getName());
129+
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getTypeString());
130130
builder.field(TRAILING_INTERVAL_KEY, trailingIntervalValue);
131131
builder.field(MINUTES_SINCE_LAST_EVENT_KEY, minutesSinceLastEvent);
132132
builder.endObject();

‎src/main/java/org/opensearch/neuralsearch/stats/state/CountableStateStatSnapshot.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public void incrementBy(Long delta) {
5656
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
5757
builder.startObject();
5858
builder.field(StatSnapshot.VALUE_FIELD, getValue());
59-
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getName());
59+
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getTypeString());
6060
builder.endObject();
6161
return builder;
6262
}

‎src/main/java/org/opensearch/neuralsearch/stats/state/SettableStateStatSnapshot.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public SettableStateStatSnapshot(StateStatName statName, T value) {
5454
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
5555
builder.startObject();
5656
builder.field(StatSnapshot.VALUE_FIELD, getValue());
57-
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getName());
57+
builder.field(StatSnapshot.STAT_TYPE_FIELD, statName.getStatType().getTypeString());
5858
builder.endObject();
5959
return builder;
6060
}

‎src/main/java/org/opensearch/neuralsearch/stats/state/StateStatName.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,29 @@ public enum StateStatName implements StatName {
2121
CLUSTER_VERSION("cluster_version", "", StateStatType.SETTABLE),
2222
TEXT_EMBEDDING_PROCESSORS("text_embedding_processors_in_pipelines", "processors.ingest", StateStatType.COUNTABLE);
2323

24-
private final String name;
24+
private final String nameString;
2525
private final String path;
2626
private final StateStatType statType;
2727

2828
private static final Map<String, StateStatName> BY_NAME = Arrays.stream(values())
29-
.collect(Collectors.toMap(stat -> stat.name, stat -> stat));
29+
.collect(Collectors.toMap(stat -> stat.nameString, stat -> stat));
3030

3131
/**
3232
* Constructor
33-
* @param name the unique name of the stat.
33+
* @param nameString the unique name of the stat.
3434
* @param path the unique path of the stat
3535
* @param statType the category of stat
3636
*/
37-
StateStatName(String name, String path, StateStatType statType) {
38-
this.name = name;
37+
StateStatName(String nameString, String path, StateStatType statType) {
38+
this.nameString = nameString;
3939
this.path = path;
4040
this.statType = statType;
4141
}
4242

4343
/**
4444
* Gets the StatName associated with a unique string name
4545
* @throws IllegalArgumentException if stat name does not exist
46-
* @param name the string name of the stat
46+
* @param value the string name of the stat
4747
* @return the StatName enum associated with that String name
4848
*/
4949
public static StateStatName from(String value) {
@@ -59,13 +59,13 @@ public static StateStatName from(String value) {
5959
*/
6060
public String getFullPath() {
6161
if (StringUtils.isBlank(path)) {
62-
return name;
62+
return nameString;
6363
}
64-
return String.join(".", path, name);
64+
return String.join(".", path, nameString);
6565
}
6666

6767
@Override
6868
public String toString() {
69-
return getName();
69+
return getNameString();
7070
}
7171
}

‎src/main/java/org/opensearch/neuralsearch/stats/state/StateStatType.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public enum StateStatType implements StatType {
1919
* Gets the name of the stat type, the enum name in lowercase
2020
* @return the name of the stat type
2121
*/
22-
public String getName() {
22+
public String getTypeString() {
2323
return name().toLowerCase(Locale.ROOT);
2424
}
2525
}

‎src/main/java/org/opensearch/neuralsearch/transport/NeuralStatsTransportAction.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,18 @@ protected NeuralStatsNodeResponse newNodeResponse(StreamInput in) throws IOExcep
126126
return new NeuralStatsNodeResponse(in);
127127
}
128128

129+
/**
130+
* Node operation to retrieve stats from node local event stats manager
131+
* @param request the node level request
132+
* @return the node level response containing node level event stats
133+
*/
129134
@Override
130135
protected NeuralStatsNodeResponse nodeOperation(NeuralStatsNodeRequest request) {
131136
// Reads from NeuralStats to node level stats on an individual node
132137
EnumSet<EventStatName> eventStatsToRetrieve = request.getRequest().getNeuralStatsInput().getEventStatNames();
133-
Map<EventStatName, TimestampedEventStatSnapshot> eventStatDataMap = eventStatsManager.getEventStatSnapshots(eventStatsToRetrieve);
138+
Map<EventStatName, TimestampedEventStatSnapshot> eventStatDataMap = eventStatsManager.getTimestampedEventStatSnapshots(
139+
eventStatsToRetrieve
140+
);
134141

135142
return new NeuralStatsNodeResponse(clusterService.localNode(), eventStatDataMap);
136143
}

‎src/main/java/org/opensearch/neuralsearch/util/PipelineServiceUtil.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ public class PipelineServiceUtil {
2626
private static PipelineServiceUtil INSTANCE;
2727

2828
/**
29-
* Return instance of the cluster context, must be initialized first for proper usage
30-
* @return instance of cluster context
29+
* Return instance of the pipeline context, must be initialized first for proper usage
30+
* @return instance of pipeline context
3131
*/
3232
public static synchronized PipelineServiceUtil instance() {
3333
if (INSTANCE == null) {

‎src/test/java/org/opensearch/neuralsearch/rest/RestNeuralStatsHandlerIT.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ public void test_textEmbedding() throws Exception {
100100
}
101101

102102
public void test_statsFiltering() throws Exception {
103-
Response response = executeNeuralStatRequest(new ArrayList<>(), Arrays.asList(StateStatName.TEXT_EMBEDDING_PROCESSORS.getName()));
103+
Response response = executeNeuralStatRequest(
104+
new ArrayList<>(),
105+
Arrays.asList(StateStatName.TEXT_EMBEDDING_PROCESSORS.getNameString())
106+
);
104107

105108
String responseBody = EntityUtils.toString(response.getEntity());
106109
Map<String, Object> stats = parseStatsResponse(responseBody);
@@ -143,7 +146,7 @@ public void test_includeMetadata() throws Exception {
143146
String valueWithMetadata = ((Map<String, String>) clusterVersionStatMetadata).get(StatSnapshot.VALUE_FIELD);
144147

145148
// Stat type metadata should match
146-
assertEquals(StateStatType.SETTABLE.getName(), statType);
149+
assertEquals(StateStatType.SETTABLE.getTypeString(), statType);
147150

148151
// Fetch Without metadata
149152
params.put(RestNeuralStatsHandler.INCLUDE_METADATA_PARAM, "false");

‎src/test/java/org/opensearch/neuralsearch/stats/NeuralStatsInputTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ public void test_toXContent() throws IOException {
133133
Map<String, Object> responseMap = xContentBuilderToMap(builder);
134134

135135
assertEquals(Collections.singletonList(NODE_ID_1), responseMap.get("node_ids"));
136-
assertEquals(Collections.singletonList(EVENT_STAT.getName()), responseMap.get("event_stats"));
137-
assertEquals(Collections.singletonList(STATE_STAT.getName()), responseMap.get("state_stats"));
136+
assertEquals(Collections.singletonList(EVENT_STAT.getNameString()), responseMap.get("event_stats"));
137+
assertEquals(Collections.singletonList(STATE_STAT.getNameString()), responseMap.get("state_stats"));
138138
assertEquals(true, responseMap.get("include_metadata"));
139139
assertEquals(true, responseMap.get("flat_keys"));
140140
}

‎src/test/java/org/opensearch/neuralsearch/stats/events/EventStatNameTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class EventStatNameTests extends OpenSearchTestCase {
1717
public static final EnumSet<EventStatName> EVENT_STATS = EnumSet.allOf(EventStatName.class);
1818

1919
public void test_from_valid() {
20-
String validStatName = EventStatName.TEXT_EMBEDDING_PROCESSOR_EXECUTIONS.getName();
20+
String validStatName = EventStatName.TEXT_EMBEDDING_PROCESSOR_EXECUTIONS.getNameString();
2121
EventStatName result = EventStatName.from(validStatName);
2222
assertEquals(EventStatName.TEXT_EMBEDDING_PROCESSOR_EXECUTIONS, result);
2323
}
@@ -29,7 +29,7 @@ public void test_from_invalid() {
2929
public void test_unique_names() {
3030
Set<String> names = new HashSet<>();
3131
for (EventStatName statName : EVENT_STATS) {
32-
String name = statName.getName().toLowerCase(Locale.ROOT);
32+
String name = statName.getNameString().toLowerCase(Locale.ROOT);
3333
assertFalse(String.format("Checking name uniqueness for %s", name), names.contains(name));
3434
names.add(name);
3535
}

0 commit comments

Comments
 (0)
Please sign in to comment.