Migrate from Joda-Time to java.time API

Signed-off-by: David Zane <[email protected]>
opensearch-project · Dec 23, 2024 · 31da38e · 31da38e
1 parent a97099c
commit 31da38e
Show file tree

Hide file tree

Showing 12 changed files with 76 additions and 59 deletions.
diff --git a/src/main/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporter.java b/src/main/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporter.java
@@ -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;
@@ -111,6 +111,6 @@ public void close() {
     }
 
     private String getDateTimeFromFormat() {
-        return indexPattern.print(DateTime.now(DateTimeZone.UTC));
+        return indexPattern.format(ZonedDateTime.now(ZoneOffset.UTC));
     }
 }
diff --git a/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java b/src/main/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactory.java
@@ -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;
@@ -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;
         }
@@ -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;
     }

diff --git a/src/main/java/org/opensearch/plugin/insights/core/reader/LocalIndexReader.java b/src/main/java/org/opensearch/plugin/insights/core/reader/LocalIndexReader.java
@@ -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;
@@ -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);
@@ -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);
     }
 }
diff --git a/src/main/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactory.java b/src/main/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactory.java
@@ -9,11 +9,11 @@
 package org.opensearch.plugin.insights.core.reader;
 
 import java.io.IOException;
+import java.time.format.DateTimeFormatter;
 import java.util.HashSet;
 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;
 
@@ -46,7 +46,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;
     }
@@ -60,7 +64,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;
     }

diff --git a/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java b/src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java
@@ -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;
@@ -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;
@@ -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)
@@ -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());

diff --git a/...va/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java b/...va/org/opensearch/plugin/insights/rules/resthandler/top_queries/RestTopQueriesAction.java
@@ -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;
@@ -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;

diff --git a/src/test/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporterTests.java b/src/test/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporterTests.java
@@ -14,9 +14,8 @@
 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 org.junit.Before;
 import org.opensearch.action.bulk.BulkAction;
 import org.opensearch.action.bulk.BulkRequestBuilder;
@@ -31,7 +30,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");
     private final Client client = mock(Client.class);
     private LocalIndexExporter localIndexExporter;
 
@@ -91,7 +90,7 @@ public void testClose() {
     }
 
     public void testGetAndSetIndexPattern() {
-        DateTimeFormatter newFormatter = mock(DateTimeFormatter.class);
+        final DateTimeFormatter newFormatter = DateTimeFormatter.ofPattern("YYYY-MM-dd");
         localIndexExporter.setIndexPattern(newFormatter);
         assert (localIndexExporter.getIndexPattern() == newFormatter);
     }

diff --git a/.../java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactoryTests.java b/.../java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterFactoryTests.java
@@ -13,7 +13,7 @@
 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 org.junit.Before;
 import org.opensearch.client.Client;
 import org.opensearch.common.settings.Settings;
@@ -76,9 +76,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));
         queryInsightsExporterFactory.updateExporter(exporter, "yyyy-MM-dd-HH");
-        assertEquals(DateTimeFormat.forPattern("yyyy-MM-dd-HH"), exporter.getIndexPattern());
+        assertEquals(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH").toString(), exporter.getIndexPattern().toString());
     }
 
 }
diff --git a/src/test/java/org/opensearch/plugin/insights/core/reader/LocalIndexReaderTests.java b/src/test/java/org/opensearch/plugin/insights/core/reader/LocalIndexReaderTests.java
@@ -13,15 +13,14 @@
 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.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;
@@ -41,7 +40,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");
     private final Client client = mock(Client.class);
     private final NamedXContentRegistry namedXContentRegistry = mock(NamedXContentRegistry.class);
     private LocalIndexReader localIndexReader;
@@ -55,7 +54,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());
@@ -83,7 +82,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);
@@ -103,7 +102,7 @@ public void testClose() {
     }
 
     public void testGetAndSetIndexPattern() {
-        DateTimeFormatter newFormatter = mock(DateTimeFormatter.class);
+        final DateTimeFormatter newFormatter = DateTimeFormatter.ofPattern("YYYY-MM-dd");
         localIndexReader.setIndexPattern(newFormatter);
         assert (localIndexReader.getIndexPattern() == newFormatter);
     }

diff --git a/...test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactoryTests.java b/...test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderFactoryTests.java
@@ -13,7 +13,7 @@
 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 org.junit.Before;
 import org.opensearch.client.Client;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
@@ -55,9 +55,9 @@ public void testCreateAndCloseReader() {
     }
 
     public void testUpdateReader() {
-        LocalIndexReader reader = new LocalIndexReader(client, DateTimeFormat.forPattern(format), namedXContentRegistry);
+        LocalIndexReader reader = new LocalIndexReader(client, DateTimeFormatter.ofPattern(format), namedXContentRegistry);
         queryInsightsReaderFactory.updateReader(reader, "yyyy-MM-dd-HH");
-        assertEquals(DateTimeFormat.forPattern("yyyy-MM-dd-HH"), reader.getIndexPattern());
+        assertEquals(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH").toString(), reader.getIndexPattern().toString());
     }
 
 }
diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java
@@ -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;
@@ -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;
@@ -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