Merge branch '37986-log_explorer_time_filtering' into 'master'

Add ability to filter by time for k8s log explorer

Closes #37986

See merge request gitlab-org/gitlab!22734

ee/app/controllers/projects/logs_controller.rb

@@ -38,7 +38,7 @@ module Projects
     end
 
     def filter_params
-      params.permit(:container_name, :pod_name, :search)
+      params.permit(:container_name, :pod_name, :search, :start, :end)
     end
 
     def environment

ee/app/models/ee/clusters/platforms/kubernetes.rb

@@ -29,7 +29,7 @@ module EE
           ::Gitlab::Kubernetes::RolloutStatus.from_deployments(*deployments, pods: pods, legacy_deployments: legacy_deployments)
         end
 
-        def read_pod_logs(environment_id, pod_name, namespace, container: nil, search: nil)
+        def read_pod_logs(environment_id, pod_name, namespace, container: nil, search: nil, start_time: nil, end_time: nil)
           # environment_id is required for use in reactive_cache_updated(),
           # to invalidate the ETag cache.
           with_reactive_cache(
@@ -38,7 +38,9 @@ module EE
             'pod_name' => pod_name,
             'namespace' => namespace,
             'container' => container,
-            'search' => search
+            'search' => search,
+            "start_time" => start_time,
+            "end_time" => end_time
           ) do |result|
             result
           end
@@ -51,11 +53,13 @@ module EE
             pod_name = opts['pod_name']
             namespace = opts['namespace']
             search = opts['search']
+            start_time = opts['start_time']
+            end_time = opts['end_time']
 
-            handle_exceptions(_('Pod not found'), pod_name: pod_name, container_name: container, search: search) do
+            handle_exceptions(_('Pod not found'), pod_name: pod_name, container_name: container, search: search, start_time: start_time, end_time: end_time) do
               container ||= container_names_of(pod_name, namespace).first
 
-              pod_logs(pod_name, namespace, container: container, search: search)
+              pod_logs(pod_name, namespace, container: container, search: search, start_time: start_time, end_time: end_time)
             end
           end
         end
@@ -84,10 +88,10 @@ module EE
 
         private
 
-        def pod_logs(pod_name, namespace, container: nil, search: nil)
+        def pod_logs(pod_name, namespace, container: nil, search: nil, start_time: nil, end_time: nil)
           enable_advanced_querying = ::Feature.enabled?(:enable_cluster_application_elastic_stack) && !!elastic_stack_client
           logs = if enable_advanced_querying
-                   elastic_stack_pod_logs(namespace, pod_name, container, search)
+                   elastic_stack_pod_logs(namespace, pod_name, container, search, start_time, end_time)
                  else
                    platform_pod_logs(namespace, pod_name, container)
                  end
@@ -117,11 +121,11 @@ module EE
           end
         end
 
-        def elastic_stack_pod_logs(namespace, pod_name, container_name, search)
+        def elastic_stack_pod_logs(namespace, pod_name, container_name, search, start_time, end_time)
           client = elastic_stack_client
           return [] if client.nil?
 
-          ::Gitlab::Elasticsearch::Logs.new(client).pod_logs(namespace, pod_name, container_name, search)
+          ::Gitlab::Elasticsearch::Logs.new(client).pod_logs(namespace, pod_name, container_name, search, start_time, end_time)
         end
 
         def elastic_stack_client

ee/app/services/pod_logs_service.rb

@@ -7,7 +7,7 @@ class PodLogsService < ::BaseService
 
   K8S_NAME_MAX_LENGTH = 253
 
-  PARAMS = %w(pod_name container_name search).freeze
+  PARAMS = %w(pod_name container_name search start end).freeze
 
   SUCCESS_RETURN_KEYS = [:status, :logs, :pod_name, :container_name, :pods, :enable_advanced_querying].freeze
 
@@ -15,6 +15,7 @@ class PodLogsService < ::BaseService
     :check_deployment_platform,
     :check_pod_names,
     :check_pod_name,
+    :check_times,
     :pod_logs,
     :filter_return_keys
 
@@ -72,13 +73,24 @@ class PodLogsService < ::BaseService
     success(result)
   end
 
+  def check_times(result)
+    Time.iso8601(params['start']) if params['start']
+    Time.iso8601(params['end']) if params['end']
+
+    success(result)
+  rescue ArgumentError
+    error(_('Invalid start or end time format'))
+  end
+
   def pod_logs(result)
     response = environment.deployment_platform.read_pod_logs(
       environment.id,
       result[:pod_name],
       namespace,
       container: result[:container_name],
-      search: params['search']
+      search: params['search'],
+      start_time: params['start'],
+      end_time: params['end']
     )
 
     return { status: :processing } unless response

ee/lib/gitlab/elasticsearch/logs.rb

@@ -10,51 +10,25 @@ module Gitlab
         @client = client
       end
 
-      def pod_logs(namespace, pod_name, container_name = nil, search = nil)
-        query = {
-          bool: {
-            must: [
-              {
-                match_phrase: {
-                  "kubernetes.pod.name" => {
-                    query: pod_name
-                  }
-                }
-              },
-              {
-                match_phrase: {
-                  "kubernetes.namespace" => {
-                    query: namespace
-                  }
-                }
-              }
-            ]
-          }
-        }
-
-        # A pod can contain multiple containers.
-        # By default we return logs from every container
-        unless container_name.nil?
-          query[:bool][:must] << {
-            match_phrase: {
-              "kubernetes.container.name" => {
-                query: container_name
-              }
-            }
-          }
+      def pod_logs(namespace, pod_name, container_name = nil, search = nil, start_time = nil, end_time = nil)
+        query = { bool: { must: [] } }.tap do |q|
+          filter_pod_name(q, pod_name)
+          filter_namespace(q, namespace)
+          filter_container_name(q, container_name)
+          filter_search(q, search)
+          filter_times(q, start_time, end_time)
         end
 
-        unless search.nil?
-          query[:bool][:must] << {
-            simple_query_string: {
-              query: search,
-              fields: [:message],
-              default_operator: :and
-            }
-          }
-        end
+        body = build_body(query)
+        response = @client.search body: body
 
-        body = {
+        format_response(response)
+      end
+
+      private
+
+      def build_body(query)
+        {
           query: query,
           # reverse order so we can query N-most recent records
           sort: [
@@ -66,8 +40,66 @@ module Gitlab
           # fixed limit for now, we should support paginated queries
           size: ::Gitlab::Elasticsearch::Logs::LOGS_LIMIT
         }
+      end
 
-        response = @client.search body: body
+      def filter_pod_name(query, pod_name)
+        query[:bool][:must] << {
+          match_phrase: {
+            "kubernetes.pod.name" => {
+              query: pod_name
+            }
+          }
+        }
+      end
+
+      def filter_namespace(query, namespace)
+        query[:bool][:must] << {
+          match_phrase: {
+            "kubernetes.namespace" => {
+              query: namespace
+            }
+          }
+        }
+      end
+
+      def filter_container_name(query, container_name)
+        # A pod can contain multiple containers.
+        # By default we return logs from every container
+        return if container_name.nil?
+
+        query[:bool][:must] << {
+          match_phrase: {
+            "kubernetes.container.name" => {
+              query: container_name
+            }
+          }
+        }
+      end
+
+      def filter_search(query, search)
+        return if search.nil?
+
+        query[:bool][:must] << {
+          simple_query_string: {
+            query: search,
+            fields: [:message],
+            default_operator: :and
+          }
+        }
+      end
+
+      def filter_times(query, start_time, end_time)
+        return unless start_time || end_time
+
+        time_range = { range: { :@timestamp => {} } }.tap do |tr|
+          tr[:range][:@timestamp][:gte] = start_time if start_time
+          tr[:range][:@timestamp][:lt] = end_time if end_time
+        end
+
+        query[:bool][:filter] = [time_range]
+      end
+
+      def format_response(response)
         result = response.fetch("hits", {}).fetch("hits", []).map do |hit|
           {
             timestamp: hit["_source"]["@timestamp"],

ee/spec/fixtures/lib/elasticsearch/query_with_end_time.json

+{
+  "query": {
+    "bool": {
+      "must": [
+        {
+          "match_phrase": {
+            "kubernetes.pod.name": {
+              "query": "production-6866bc8974-m4sk4"
+            }
+          }
+        },
+        {
+          "match_phrase": {
+            "kubernetes.namespace": {
+              "query": "autodevops-deploy-9-production"
+            }
+          }
+        }
+      ],
+      "filter": [
+        {
+          "range": {
+            "@timestamp": {
+              "lt": "2019-12-13T14:35:34.034Z"
+            }
+          }
+        }
+      ]
+    }
+  },
+  "sort": [
+    {
+      "@timestamp": {
+        "order": "desc"
+      }
+    },
+    {
+      "offset": {
+        "order": "desc"
+      }
+    }
+  ],
+  "_source": [
+    "@timestamp",
+    "message"
+  ],
+  "size": 500
+}

ee/spec/fixtures/lib/elasticsearch/query_with_start_time.json

+{
+  "query": {
+    "bool": {
+      "must": [
+        {
+          "match_phrase": {
+            "kubernetes.pod.name": {
+              "query": "production-6866bc8974-m4sk4"
+            }
+          }
+        },
+        {
+          "match_phrase": {
+            "kubernetes.namespace": {
+              "query": "autodevops-deploy-9-production"
+            }
+          }
+        }
+      ],
+      "filter": [
+        {
+          "range": {
+            "@timestamp": {
+              "gte": "2019-12-13T14:35:34.034Z"
+            }
+          }
+        }
+      ]
+    }
+  },
+  "sort": [
+    {
+      "@timestamp": {
+        "order": "desc"
+      }
+    },
+    {
+      "offset": {
+        "order": "desc"
+      }
+    }
+  ],
+  "_source": [
+    "@timestamp",
+    "message"
+  ],
+  "size": 500
+}

ee/spec/fixtures/lib/elasticsearch/query_with_times.json

+{
+  "query": {
+    "bool": {
+      "must": [
+        {
+          "match_phrase": {
+            "kubernetes.pod.name": {
+              "query": "production-6866bc8974-m4sk4"
+            }
+          }
+        },
+        {
+          "match_phrase": {
+            "kubernetes.namespace": {
+              "query": "autodevops-deploy-9-production"
+            }
+          }
+        }
+      ],
+      "filter": [
+        {
+          "range": {
+            "@timestamp": {
+              "gte": "2019-12-13T14:35:34.034Z",
+              "lt": "2019-12-13T14:35:34.034Z"
+            }
+          }
+        }
+      ]
+    }
+  },
+  "sort": [
+    {
+      "@timestamp": {
+        "order": "desc"
+      }
+    },
+    {
+      "offset": {
+        "order": "desc"
+      }
+    }
+  ],
+  "_source": [
+    "@timestamp",
+    "message"
+  ],
+  "size": 500
+}

ee/spec/lib/gitlab/elasticsearch/logs_spec.rb

@@ -18,10 +18,15 @@ describe Gitlab::Elasticsearch::Logs do
   let(:pod_name) { "production-6866bc8974-m4sk4" }
   let(:container_name) { "auto-deploy-app" }
   let(:search) { "foo +bar "}
+  let(:start_time) { "2019-12-13T14:35:34.034Z" }
+  let(:end_time) { "2019-12-13T14:35:34.034Z" }
 
   let(:body) { JSON.parse(fixture_file('lib/elasticsearch/query.json', dir: 'ee')) }
   let(:body_with_container) { JSON.parse(fixture_file('lib/elasticsearch/query_with_container.json', dir: 'ee')) }
   let(:body_with_search) { JSON.parse(fixture_file('lib/elasticsearch/query_with_search.json', dir: 'ee')) }
+  let(:body_with_times) { JSON.parse(fixture_file('lib/elasticsearch/query_with_times.json', dir: 'ee')) }
+  let(:body_with_start_time) { JSON.parse(fixture_file('lib/elasticsearch/query_with_start_time.json', dir: 'ee')) }
+  let(:body_with_end_time) { JSON.parse(fixture_file('lib/elasticsearch/query_with_end_time.json', dir: 'ee')) }
 
   RSpec::Matchers.define :a_hash_equal_to_json do |expected|
     match do |actual|
@@ -50,5 +55,26 @@ describe Gitlab::Elasticsearch::Logs do
       result = subject.pod_logs(namespace, pod_name, nil, search)
       expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1])
     end
+
+    it 'can further filter the logs by start_time and end_time' do
+      expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_times)).and_return(es_response)
+
+      result = subject.pod_logs(namespace, pod_name, nil, nil, start_time, end_time)
+      expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1])
+    end
+
+    it 'can further filter the logs by only start_time' do
+      expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_start_time)).and_return(es_response)
+
+      result = subject.pod_logs(namespace, pod_name, nil, nil, start_time)
+      expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1])
+    end
+
+    it 'can further filter the logs by only end_time' do
+      expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_end_time)).and_return(es_response)
+
+      result = subject.pod_logs(namespace, pod_name, nil, nil, nil, end_time)
+      expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1])
+    end
   end
 end

ee/spec/models/ee/clusters/platforms/kubernetes_spec.rb

@@ -268,7 +268,9 @@ describe Clusters::Platforms::Kubernetes do
             'pod_name' => pod_name,
             'namespace' => namespace,
             'container' => container,
-            'search' => nil
+            'search' => nil,
+            'start_time' => nil,
+            'end_time' => nil
           }
         ]
       end

ee/spec/services/pod_logs_service_spec.rb

@@ -17,13 +17,17 @@ describe PodLogsService do
     let(:enable_advanced_querying) { false }
     let(:logs) { ['Log 1', 'Log 2', 'Log 3'] }
     let(:result) { subject.execute }
+    let(:start_time) { nil }
+    let(:end_time) { nil }
 
     let(:params) do
       ActionController::Parameters.new(
         {
           'pod_name' => pod_name,
           'container_name' => container_name,
-          'search' => search
+          'search' => search,
+          'start' => start_time,
+          'end' => end_time
         }
       ).permit!
     end
@@ -58,7 +62,7 @@ describe PodLogsService do
     shared_context 'return error' do |message|
       before do
         allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
-          .with(environment.id, pod_name, environment.deployment_namespace, container: container_name, search: search)
+          .with(environment.id, pod_name, environment.deployment_namespace, container: container_name, search: search, start_time: start_time, end_time: end_time)
           .and_return({
             status: :error,
             error: message,
@@ -72,7 +76,7 @@ describe PodLogsService do
     shared_context 'return success' do
       before do
         allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
-          .with(environment.id, response_pod_name, environment.deployment_namespace, container: container_name, search: search)
+          .with(environment.id, response_pod_name, environment.deployment_namespace, container: container_name, search: search, start_time: start_time, end_time: end_time)
           .and_return({
             status: :success,
             logs: ["Log 1", "Log 2", "Log 3"],
@@ -157,7 +161,7 @@ describe PodLogsService do
 
         it 'returns logs of first pod' do
           expect_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
-            .with(environment.id, first_pod_name, environment.deployment_namespace, container: nil, search: search)
+            .with(environment.id, first_pod_name, environment.deployment_namespace, container: nil, search: search, start_time: start_time, end_time: end_time)
 
           subject.execute
         end
@@ -193,6 +197,26 @@ describe PodLogsService do
         it_behaves_like 'success'
       end
 
+      context 'when start and end time is specified' do
+        let(:pod_name) { 'some-pod' }
+        let(:container_name) { nil }
+        let(:start_time) { '2019-12-13T14:35:34.034Z' }
+        let(:end_time) { '2019-12-13T14:35:34.034Z' }
+
+        include_context 'return success'
+
+        it_behaves_like 'success'
+      end
+
+      context 'when start and end time are invalid' do
+        let(:pod_name) { 'some-pod' }
+        let(:container_name) { nil }
+        let(:start_time) { '1' }
+        let(:end_time) { '2' }
+
+        it_behaves_like 'error', 'Invalid start or end time format'
+      end
+
       context 'when error is returned' do
         include_context 'return error', 'Kubernetes API returned status code: 400'
 
@@ -204,7 +228,7 @@ describe PodLogsService do
       context 'when nil is returned' do
         before do
           allow_any_instance_of(EE::Clusters::Platforms::Kubernetes).to receive(:read_pod_logs)
-            .with(environment.id, pod_name, environment.deployment_namespace, container: container_name, search: search)
+            .with(environment.id, pod_name, environment.deployment_namespace, container: container_name, search: search, start_time: start_time, end_time: end_time)
             .and_return(nil)
         end
 

locale/gitlab.pot

@@ -10227,6 +10227,9 @@ msgstr ""
 msgid "Invalid server response"
 msgstr ""
 
+msgid "Invalid start or end time format"
+msgstr ""
+
 msgid "Invalid two-factor code."
 msgstr ""