Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from Joda-Time to java.time API #175

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@

package org.opensearch.plugin.insights.core.exporter;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.List;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormatter;
import org.opensearch.action.bulk.BulkRequestBuilder;
import org.opensearch.action.bulk.BulkResponse;
import org.opensearch.action.index.IndexRequest;
Expand Down Expand Up @@ -111,6 +111,6 @@ public void close() {
}

private String getDateTimeFromFormat() {
return indexPattern.print(DateTime.now(DateTimeZone.UTC));
return indexPattern.format(ZonedDateTime.now(ZoneOffset.UTC));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;

import java.io.IOException;
import java.time.format.DateTimeFormatter;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.format.DateTimeFormat;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
import org.opensearch.plugin.insights.core.metrics.OperationalMetric;
Expand Down Expand Up @@ -80,7 +80,7 @@ public void validateExporterConfig(final Settings settings) throws IllegalArgume
*/
public QueryInsightsExporter createExporter(SinkType type, String indexPattern) {
if (SinkType.LOCAL_INDEX.equals(type)) {
QueryInsightsExporter exporter = new LocalIndexExporter(client, DateTimeFormat.forPattern(indexPattern));
QueryInsightsExporter exporter = new LocalIndexExporter(client, DateTimeFormatter.ofPattern(indexPattern, Locale.ROOT));
this.exporters.add(exporter);
return exporter;
}
Expand All @@ -96,7 +96,7 @@ public QueryInsightsExporter createExporter(SinkType type, String indexPattern)
*/
public QueryInsightsExporter updateExporter(QueryInsightsExporter exporter, String indexPattern) {
if (exporter.getClass() == LocalIndexExporter.class) {
((LocalIndexExporter) exporter).setIndexPattern(DateTimeFormat.forPattern(indexPattern));
((LocalIndexExporter) exporter).setIndexPattern(DateTimeFormatter.ofPattern(indexPattern, Locale.ROOT));
}
return exporter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

package org.opensearch.plugin.insights.core.reader;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.List;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormatter;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Client;
Expand Down Expand Up @@ -91,18 +91,21 @@ public List<SearchQueryRecord> read(final String from, final String to) {
if (from == null || to == null) {
return records;
}
final DateTime start = DateTime.parse(from);
DateTime end = DateTime.parse(to);
if (end.compareTo(DateTime.now(DateTimeZone.UTC)) > 0) {
end = DateTime.now(DateTimeZone.UTC);
final ZonedDateTime start = ZonedDateTime.parse(from);
ZonedDateTime end = ZonedDateTime.parse(to);
ZonedDateTime now = ZonedDateTime.now(ZoneOffset.UTC);
if (end.isAfter(now)) {
end = now;
}
DateTime curr = start;
while (curr.compareTo(end.plusDays(1).withTimeAtStartOfDay()) < 0) {
ZonedDateTime curr = start;
while (curr.isBefore(end.plusDays(1).toLocalDate().atStartOfDay(end.getZone()))) {
String index = getDateTimeFromFormat(curr);
SearchRequest searchRequest = new SearchRequest(index);
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
MatchQueryBuilder excludeQuery = QueryBuilders.matchQuery("indices", "top_queries*");
RangeQueryBuilder rangeQuery = QueryBuilders.rangeQuery("timestamp").from(start.getMillis()).to(end.getMillis());
RangeQueryBuilder rangeQuery = QueryBuilders.rangeQuery("timestamp")
.from(start.toInstant().toEpochMilli())
.to(end.toInstant().toEpochMilli());
QueryBuilder query = QueryBuilders.boolQuery().must(rangeQuery).mustNot(excludeQuery);
searchSourceBuilder.query(query);
searchRequest.source(searchSourceBuilder);
Expand Down Expand Up @@ -132,7 +135,7 @@ public void close() {
logger.debug("Closing the LocalIndexReader..");
}

private String getDateTimeFromFormat(DateTime current) {
return indexPattern.print(current);
private String getDateTimeFromFormat(ZonedDateTime current) {
return current.format(indexPattern);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
package org.opensearch.plugin.insights.core.reader;

import java.io.IOException;
import java.time.format.DateTimeFormatter;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.format.DateTimeFormat;
import org.opensearch.client.Client;
import org.opensearch.core.xcontent.NamedXContentRegistry;

Expand Down Expand Up @@ -46,7 +47,11 @@ public QueryInsightsReaderFactory(final Client client) {
* @return QueryInsightsReader the created Reader
*/
public QueryInsightsReader createReader(String indexPattern, NamedXContentRegistry namedXContentRegistry) {
QueryInsightsReader Reader = new LocalIndexReader(client, DateTimeFormat.forPattern(indexPattern), namedXContentRegistry);
QueryInsightsReader Reader = new LocalIndexReader(
client,
DateTimeFormatter.ofPattern(indexPattern, Locale.ROOT),
namedXContentRegistry
);
this.Readers.add(Reader);
return Reader;
}
Expand All @@ -60,7 +65,7 @@ public QueryInsightsReader createReader(String indexPattern, NamedXContentRegist
*/
public QueryInsightsReader updateReader(QueryInsightsReader Reader, String indexPattern) {
if (Reader.getClass() == LocalIndexReader.class) {
((LocalIndexReader) Reader).setIndexPattern(DateTimeFormat.forPattern(indexPattern));
((LocalIndexReader) Reader).setIndexPattern(DateTimeFormatter.ofPattern(indexPattern, Locale.ROOT));
}
return Reader;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -32,7 +33,6 @@
import java.util.stream.Stream;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.DateTime;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -362,10 +362,10 @@ public List<SearchQueryRecord> getTopQueriesRecords(final boolean includeLastWin
}
List<SearchQueryRecord> filterQueries = queries;
if (from != null && to != null) {
final DateTime start = DateTime.parse(from);
final DateTime end = DateTime.parse(to);
Predicate<SearchQueryRecord> timeFilter = element -> start.getMillis() <= element.getTimestamp()
&& element.getTimestamp() <= end.getMillis();
final ZonedDateTime start = ZonedDateTime.parse(from);
final ZonedDateTime end = ZonedDateTime.parse(to);
Predicate<SearchQueryRecord> timeFilter = element -> start.toInstant().toEpochMilli() <= element.getTimestamp()
&& element.getTimestamp() <= end.toInstant().toEpochMilli();
filterQueries = queries.stream().filter(checkIfInternal.and(timeFilter)).collect(Collectors.toList());
}
return Stream.of(filterQueries)
Expand Down Expand Up @@ -394,11 +394,11 @@ public List<SearchQueryRecord> getTopQueriesRecordsFromIndex(final String from,
final List<SearchQueryRecord> queries = new ArrayList<>();
if (reader != null) {
try {
final DateTime start = DateTime.parse(from);
final DateTime end = DateTime.parse(to);
final ZonedDateTime start = ZonedDateTime.parse(from);
final ZonedDateTime end = ZonedDateTime.parse(to);
List<SearchQueryRecord> records = reader.read(from, to);
Predicate<SearchQueryRecord> timeFilter = element -> start.getMillis() <= element.getTimestamp()
&& element.getTimestamp() <= end.getMillis();
Predicate<SearchQueryRecord> timeFilter = element -> start.toInstant().toEpochMilli() <= element.getTimestamp()
&& element.getTimestamp() <= end.toInstant().toEpochMilli();
List<SearchQueryRecord> filteredRecords = records.stream()
.filter(checkIfInternal.and(timeFilter))
.collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.TOP_QUERIES_BASE_URI;
import static org.opensearch.rest.RestRequest.Method.GET;

import java.time.ZonedDateTime;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.stream.Collectors;
import org.joda.time.DateTime;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.Strings;
Expand Down Expand Up @@ -67,7 +67,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC

private static boolean isNotISODate(final String dateTime) {
try {
DateTime.parse(dateTime);
ZonedDateTime.parse(dateTime);
return false;
} catch (Exception e) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

import java.time.format.DateTimeFormatter;
import java.util.List;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import java.util.Locale;
import org.junit.Before;
import org.opensearch.action.bulk.BulkAction;
import org.opensearch.action.bulk.BulkRequestBuilder;
Expand All @@ -31,7 +31,7 @@
* Granular tests for the {@link LocalIndexExporterTests} class.
*/
public class LocalIndexExporterTests extends OpenSearchTestCase {
private final DateTimeFormatter format = DateTimeFormat.forPattern("YYYY.MM.dd");
private final DateTimeFormatter format = DateTimeFormatter.ofPattern("YYYY.MM.dd", Locale.ROOT);
private final Client client = mock(Client.class);
private LocalIndexExporter localIndexExporter;

Expand Down Expand Up @@ -91,7 +91,7 @@ public void testClose() {
}

public void testGetAndSetIndexPattern() {
DateTimeFormatter newFormatter = mock(DateTimeFormatter.class);
final DateTimeFormatter newFormatter = DateTimeFormatter.ofPattern("YYYY-MM-dd", Locale.ROOT);
localIndexExporter.setIndexPattern(newFormatter);
assert (localIndexExporter.getIndexPattern() == newFormatter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
import static org.mockito.Mockito.when;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.EXPORTER_TYPE;

import org.joda.time.format.DateTimeFormat;
import java.time.format.DateTimeFormatter;
import java.util.Locale;
import org.junit.Before;
import org.opensearch.client.Client;
import org.opensearch.common.settings.Settings;
Expand Down Expand Up @@ -76,9 +77,9 @@ public void testCreateAndCloseExporter() {
}

public void testUpdateExporter() {
LocalIndexExporter exporter = new LocalIndexExporter(client, DateTimeFormat.forPattern("yyyy-MM-dd"));
LocalIndexExporter exporter = new LocalIndexExporter(client, DateTimeFormatter.ofPattern(format, Locale.ROOT));
queryInsightsExporterFactory.updateExporter(exporter, "yyyy-MM-dd-HH");
assertEquals(DateTimeFormat.forPattern("yyyy-MM-dd-HH"), exporter.getIndexPattern());
assertEquals(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH", Locale.ROOT).toString(), exporter.getIndexPattern().toString());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import org.apache.lucene.search.TotalHits;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.junit.Before;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.action.search.SearchResponse;
Expand All @@ -41,7 +41,7 @@
* Granular tests for the {@link LocalIndexReaderTests} class.
*/
public class LocalIndexReaderTests extends OpenSearchTestCase {
private final DateTimeFormatter format = DateTimeFormat.forPattern("YYYY.MM.dd");
private final DateTimeFormatter format = DateTimeFormatter.ofPattern("YYYY.MM.dd", Locale.ROOT);
private final Client client = mock(Client.class);
private final NamedXContentRegistry namedXContentRegistry = mock(NamedXContentRegistry.class);
private LocalIndexReader localIndexReader;
Expand All @@ -55,7 +55,7 @@ public void setup() {
public void testReadRecords() {
ActionFuture<SearchResponse> responseActionFuture = mock(ActionFuture.class);
Map<String, Object> sourceMap = new HashMap<>();
sourceMap.put("timestamp", DateTime.now(DateTimeZone.UTC).getMillis());
sourceMap.put("timestamp", ZonedDateTime.now(ZoneOffset.UTC).toInstant().toEpochMilli());
sourceMap.put("indices", Collections.singletonList("my-index-0"));
sourceMap.put("source", Map.of());
sourceMap.put("labels", Map.of());
Expand Down Expand Up @@ -83,7 +83,7 @@ public void testReadRecords() {
when(searchResponse.getHits()).thenReturn(searchHits);
when(responseActionFuture.actionGet()).thenReturn(searchResponse);
when(client.search(any(SearchRequest.class))).thenReturn(responseActionFuture);
String time = DateTime.now(DateTimeZone.UTC).toString();
String time = ZonedDateTime.now(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME);
List<SearchQueryRecord> records = List.of();
try {
records = localIndexReader.read(time, time);
Expand All @@ -103,7 +103,7 @@ public void testClose() {
}

public void testGetAndSetIndexPattern() {
DateTimeFormatter newFormatter = mock(DateTimeFormatter.class);
final DateTimeFormatter newFormatter = DateTimeFormatter.ofPattern("YYYY-MM-dd", Locale.ROOT);
localIndexReader.setIndexPattern(newFormatter);
assert (localIndexReader.getIndexPattern() == newFormatter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
import static org.mockito.Mockito.when;
import static org.opensearch.plugin.insights.settings.QueryInsightsSettings.DEFAULT_TOP_N_QUERIES_INDEX_PATTERN;

import org.joda.time.format.DateTimeFormat;
import java.time.format.DateTimeFormatter;
import java.util.Locale;
import org.junit.Before;
import org.opensearch.client.Client;
import org.opensearch.core.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -55,9 +56,9 @@ public void testCreateAndCloseReader() {
}

public void testUpdateReader() {
LocalIndexReader reader = new LocalIndexReader(client, DateTimeFormat.forPattern(format), namedXContentRegistry);
LocalIndexReader reader = new LocalIndexReader(client, DateTimeFormatter.ofPattern(format, Locale.ROOT), namedXContentRegistry);
queryInsightsReaderFactory.updateReader(reader, "yyyy-MM-dd-HH");
assertEquals(DateTimeFormat.forPattern("yyyy-MM-dd-HH"), reader.getIndexPattern());
assertEquals(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH", Locale.ROOT).toString(), reader.getIndexPattern().toString());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

package org.opensearch.plugin.insights.core.service;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

import java.util.List;
import java.util.Map;
Expand All @@ -21,12 +23,15 @@
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.plugin.insights.QueryInsightsTestUtils;
import org.opensearch.plugin.insights.core.metrics.OperationalMetricsCounter;
import org.opensearch.plugin.insights.rules.model.GroupingType;
import org.opensearch.plugin.insights.rules.model.MetricType;
import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;
import org.opensearch.plugin.insights.rules.model.healthStats.QueryInsightsHealthStats;
import org.opensearch.plugin.insights.rules.model.healthStats.TopQueriesHealthStats;
import org.opensearch.plugin.insights.settings.QueryInsightsSettings;
import org.opensearch.telemetry.metrics.Counter;
import org.opensearch.telemetry.metrics.MetricsRegistry;
import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.threadpool.ScalingExecutorBuilder;
Expand Down Expand Up @@ -64,6 +69,12 @@ public void setup() {
queryInsightsService.enableCollection(MetricType.CPU, true);
queryInsightsService.enableCollection(MetricType.MEMORY, true);
queryInsightsServiceSpy = spy(queryInsightsService);

MetricsRegistry metricsRegistry = mock(MetricsRegistry.class);
when(metricsRegistry.createCounter(any(String.class), any(String.class), any(String.class))).thenAnswer(
invocation -> mock(Counter.class)
);
OperationalMetricsCounter.initialize("cluster", metricsRegistry);
}

@Override
Expand Down
Loading
Loading