Merge pull request #2019 from cahartma/LOG-4090-keep-structured-time-key

[release-5.6] LOG-4090: Fix log parsing and index logic in fluentd
openshift · May 25, 2023 · e5c60be · e5c60be
2 parents 04eed89 + c369c62
commit e5c60be
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 37 deletions.
diff --git a/internal/generator/fluentd/conf_test.go b/internal/generator/fluentd/conf_test.go
@@ -576,7 +576,7 @@ var _ = Describe("Testing Complete Config Generation", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>

diff --git a/internal/generator/fluentd/fluent_conf_test.go b/internal/generator/fluentd/fluent_conf_test.go
@@ -621,7 +621,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -773,7 +773,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -1365,7 +1365,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -1517,7 +1517,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -2113,7 +2113,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -2265,7 +2265,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -3269,7 +3269,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -3421,7 +3421,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -3573,7 +3573,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -3725,7 +3725,7 @@ var _ = Describe("Generating fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -3887,6 +3887,7 @@ var _ = Describe("Generating fluentd config", func() {
     <parse>
       @type json
       json_parser oj
+      keep_time_key true
     </parse>
   </filter>
   

diff --git a/internal/generator/fluentd/output/elasticsearch/elasticsearch_test.go b/internal/generator/fluentd/output/elasticsearch/elasticsearch_test.go
@@ -96,7 +96,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -269,7 +269,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -447,7 +447,7 @@ var _ = Describe("Generate fluentd config", func() {
       <filter **>
         @type record_modifier
         <record>
-        _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+        _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
         </record>
         remove_keys _dummy_
       </filter>
@@ -616,7 +616,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -772,7 +772,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -940,7 +940,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -1109,7 +1109,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -1273,7 +1273,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>
@@ -1429,7 +1429,7 @@ var _ = Describe("Generate fluentd config", func() {
   <filter **>
     @type record_modifier
     <record>
-    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+    _dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
     </record>
     remove_keys _dummy_
   </filter>

diff --git a/internal/generator/fluentd/output/elasticsearch/viaq_data_model.go b/internal/generator/fluentd/output/elasticsearch/viaq_data_model.go
@@ -40,11 +40,13 @@ func ViaqDataModel(bufspec *logging.FluentdBufferSpec, secret *corev1.Secret, o
 		RemoveKeys: []string{"_dummy_, _dummy2_, _dummy3_"},
 	}
 
+	// LOG-3249: If viaq_index_name is still 'app-write', it means we cant find the specified index key in the message
+	// and/or the structuredIndexName was not provided. Do not parse and instead re-assemble message and remove structured
 	modRecordRebuildMessage := RecordModifier{
 		Records: []Record{
 			{
 				Key:        "_dummy_",
-				Expression: `${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}`,
+				Expression: `${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}`,
 			},
 		},
 		RemoveKeys: []string{"_dummy_"},

diff --git a/internal/generator/fluentd/output/output_fluentd_buffer_test.go b/internal/generator/fluentd/output/output_fluentd_buffer_test.go
@@ -136,7 +136,7 @@ var _ = Describe("Generating fluentd config", func() {
 		<filter **>
 			@type record_modifier
 			<record>
-			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
 			</record>
 			remove_keys _dummy_
 		</filter>
@@ -300,7 +300,7 @@ var _ = Describe("Generating fluentd config", func() {
 		<filter **>
 			@type record_modifier
 			<record>
-			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
 			</record>
 			remove_keys _dummy_
 		</filter>
@@ -451,7 +451,7 @@ var _ = Describe("Generating fluentd config", func() {
 		<filter **>
 			@type record_modifier
 			<record>
-			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured'])) if record['structured'] and record['viaq_index_name'] == 'app-write'}
+			_dummy_ ${(require 'json';record['message']=JSON.dump(record['structured']);record.delete('structured')) if record['structured'] and record['viaq_index_name'] == 'app-write'}
 			</record>
 			remove_keys _dummy_
 		</filter>

diff --git a/internal/generator/fluentd/pipeline_to_output.go b/internal/generator/fluentd/pipeline_to_output.go
@@ -44,6 +44,7 @@ const (
   <parse>
     @type json
     json_parser oj
+    keep_time_key true
   </parse>
 </filter>
 {{end}}`

diff --git a/internal/generator/fluentd/pipeline_to_output_test.go b/internal/generator/fluentd/pipeline_to_output_test.go
@@ -162,6 +162,7 @@ var _ = Describe("Testing Config Generation", func() {
     <parse>
       @type json
       json_parser oj
+      keep_time_key true
     </parse>
   </filter>
   

diff --git a/test/functional/normalization/json_parsing_test.go b/test/functional/normalization/json_parsing_test.go
@@ -166,7 +166,6 @@ var _ = Describe("[Functional][Normalization] Json log parsing", func() {
 		Expect(logs[0].Message).To(Equal(expectedMessage), "received message not matching")
 	})
 	It("should not parse invalid json message into structured", func() {
-		// This test case is disabled to fix the behavior of invalid json parsing
 		clfb.Forwarder.Spec.Pipelines[0].Parse = "json"
 		Expect(framework.Deploy()).To(BeNil())
 
@@ -229,8 +228,7 @@ var _ = Describe("[Functional][Normalization] Json log parsing", func() {
 		Expect(logs[0].Message).To(BeEmpty())
 	})
 
-	It("should not parse raise warn message in collector container : LOG-1806", func() {
-		// This test case is disabled to fix the behavior of invalid json parsing
+	It("should not raise parser error message in collector container : LOG-1806", func() {
 		clfb.Forwarder.Spec.Pipelines[0].Parse = "json"
 		Expect(framework.Deploy()).To(BeNil())
 

diff --git a/test/functional/normalization/loglevel/conainer_logs_test.go b/test/functional/normalization/loglevel/conainer_logs_test.go
@@ -45,7 +45,7 @@ var _ = Describe("[functional][normalization][loglevel] tests for message format
 
 		// Write log line as input to fluentd
 		applicationLogLine := functional.NewCRIOLogMessage(timestamp, message, false)
-		Expect(framework.WriteMessagesToApplicationLog(applicationLogLine, 10)).To(BeNil())
+		Expect(framework.WriteMessagesToApplicationLog(applicationLogLine, 2)).To(BeNil())
 		// Read line from Log Forward output
 		raw, err := framework.ReadRawApplicationLogsFrom(logging.OutputTypeFluentdForward)
 		Expect(err).To(BeNil(), "Expected no errors reading the logs")