fix: address review comments

Author: devbgv

internal/collector/syslogprocessor/processor.go

@@ -65,10 +65,9 @@ func (p *syslogProcessor) ConsumeLogs(ctx context.Context, ld plog.Logs) error {
 	var errs error
 
 	rl := ld.ResourceLogs()
-	for i := range rl.Len() {
-		sl := rl.At(i).ScopeLogs()
-		for j := range sl.Len() {
-			if err := p.processLogRecords(sl.At(j).LogRecords()); err != nil {
+	for _, sl := range rl.All() {
+		for _, lr := range sl.ScopeLogs().All() {
+			if err := p.processLogRecords(lr.LogRecords()); err != nil {
 				errs = multierr.Append(errs, err)
 			}
 		}
@@ -83,23 +82,32 @@ func (p *syslogProcessor) ConsumeLogs(ctx context.Context, ld plog.Logs) error {
 
 func (p *syslogProcessor) processLogRecords(lrs plog.LogRecordSlice) error {
 	// Drop anything that isn't a string-bodied log before processing.
+	var skipped, errCount int
+	var t pcommon.ValueType
+	var errs error
 	lrs.RemoveIf(func(lr plog.LogRecord) bool {
-		t := lr.Body().Type()
+		t = lr.Body().Type()
 		if t == pcommon.ValueTypeStr {
 			return false
 		}
-		p.settings.Logger.Debug("Skipping log record with unsupported body type", zap.Any("type", t))
 
+		skipped++
 		return true
 	})
-
-	for k := range lrs.Len() {
-		lr := lrs.At(k)
+	if skipped > 0 {
+		p.settings.Logger.Debug("Skipping log record with unsupported body type", zap.Any("type", t))
+	}
+	errCount = 0
+	for _, lr := range lrs.All() {
 		if err := p.processLogRecord(lr); err != nil {
-			p.settings.Logger.Debug("failed to process log record", zap.Error(err))
-			return err
+			errs = multierr.Append(errs, err)
+			errCount++
 		}
 	}
+	if errCount > 0 {
+		p.settings.Logger.Debug("Some log records failed to process", zap.Int("count", errCount))
+		return errs
+	}
 
 	return nil
 }
@@ -138,9 +146,6 @@ func (p *syslogProcessor) setSyslogAttributes(lr plog.LogRecord, m *rfc3164.Sysl
 	if m.Timestamp != nil {
 		attrs.PutStr("syslog.timestamp", m.Timestamp.Format(time.RFC3339))
 	}
-	if m.Hostname != nil {
-		attrs.PutStr("syslog.hostname", *m.Hostname)
-	}
 	if m.Appname != nil {
 		attrs.PutStr("syslog.appname", *m.Appname)
 	}
@@ -417,17 +422,22 @@ func (p *syslogProcessor) extractSignatureData(kvMap map[string]string) []Signat
 }
 
 func splitAndTrim(value string) []string {
-	if value == "" || value == notAvailable {
+	if strings.TrimSpace(value) == "" || value == notAvailable {
 		return nil
 	}
+
 	parts := strings.Split(value, ",")
-	for i := range parts {
-		parts[i] = strings.TrimSpace(parts[i])
+
+	var trimmedParts []string
+	for _, part := range parts {
+		trimmed := strings.TrimSpace(part)
+		if trimmed != "" {
+			trimmedParts = append(trimmedParts, trimmed)
+		}
 	}
 
-	return parts
+	return trimmedParts
 }
-
 func buildSignatures(ids, names []string, mask, offset, length string) []SignatureData {
 	signatures := make([]SignatureData, 0, len(ids))
 	for i, id := range ids {

internal/collector/syslogprocessor/processor_test.go

@@ -8,6 +8,7 @@ package syslogprocessor
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -31,7 +32,6 @@ func TestSyslogProcessor(t *testing.T) {
 			name: "csv nginx app protect syslog message",
 			body: `<130>Aug 22 03:28:35 ip-172-16-0-213 ASM:N/A,80,127.0.0.1,false,GET,nms_app_protect_default_policy,HTTP,blocked,0,N/A,N/A::N/A,{High Accuracy Signatures;Cross Site Scripting Signatures}::{High Accuracy Signatures; Cross Site Scripting Signatures},56064,N/A,5377540117854870581,N/A,5,1-localhost:1-/,N/A,REJECTED,SECURITY_WAF_VIOLATION,Illegal meta character in URL::Attack signature detected::Violation Rating Threat detected::Bot Client Detected,<?xml version='1.0' encoding='UTF-8'?><BAD_MSG><violation_masks><block>414000000200c00-3a03030c30000072-8000000000000000-0</block><alarm>475f0ffcbbd0fea-befbf35cb000007e-f400000000000000-0</alarm><learn>0-0-0-0</learn><staging>0-0-0-0</staging></violation_masks><request-violations><violation><viol_index>42</viol_index><viol_name>VIOL_ATTACK_SIGNATURE</viol_name><context>url</context><sig_data><sig_id>200000099</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>Lzw+PHNjcmlwdD4=</buffer><offset>3</offset><length>7</length></kw_data></sig_data><sig_data><sig_id>200000093</sig_id><blocking_mask>3</blocking_mask><kw_data><buffer>Lzw+PHNjcmlwdD4=</buffer><offset>4</offset><length>7</length></kw_data></sig_data></violation><violation><viol_index>26</viol_index><viol_name>VIOL_URL_METACHAR</viol_name><uri>Lzw+PHNjcmlwdD4=</uri><metachar_index>60</metachar_index><wildcard_entity>*</wildcard_entity><staging>0</staging></violation><violation><viol_index>26</viol_index><viol_name>VIOL_URL_METACHAR</viol_name><uri>Lzw+PHNjcmlwdD4=</uri><metachar_index>62</metachar_index><wildcard_entity>*</wildcard_entity><staging>0</staging></violation><violation><viol_index>122</viol_index><viol_name>VIOL_BOT_CLIENT</viol_name></violation><violation><viol_index>93</viol_index><viol_name>VIOL_RATING_THREAT</viol_name></violation></request-violations></BAD_MSG>,curl,HTTP Library,N/A,N/A,Untrusted Bot,N/A,N/A,HTTP/1.1,/<><script>,GET /<><script> HTTP/1.1\\r\\nHost: localhost\\r\\nUser-Agent: curl/7.81.0\\r\\nAccept: */*\\r\\n\\r\\n`,
 			expectAttrs: map[string]string{
-				"syslog.hostname":         "ip-172-16-0-213",
 				"syslog.appname":          "ASM",
 				"app_protect.policy_name": "nms_app_protect_default_policy",
 				"app_protect.support_id":  "5377540117854870581",
@@ -44,8 +44,7 @@ func TestSyslogProcessor(t *testing.T) {
 			name: "simple valid syslog message",
 			body: "<34>Oct 11 22:14:15 mymachine su: 'su root' failed for lonvick on /dev/pts/8",
 			expectAttrs: map[string]string{
-				"syslog.hostname": "mymachine",
-				"syslog.appname":  "su",
+				"syslog.appname": "su",
 			},
 			expectRecords: 1,
 		},
@@ -159,16 +158,14 @@ func TestSyslogProcessor(t *testing.T) {
 
 func TestSyslogProcessorFailure(t *testing.T) {
 	testCases := []struct {
-		expectAttrs   map[string]string
-		body          any
 		name          string
-		expectJSON    string
+		body          any
 		expectRecords int
 	}{
 		{
 			name:          "invalid syslog message",
 			body:          "not a syslog line",
-			expectRecords: 1,
+			expectRecords: 0,
 		},
 	}
 
@@ -177,31 +174,35 @@ func TestSyslogProcessorFailure(t *testing.T) {
 			ctx := context.Background()
 			settings := processortest.NewNopSettings(processortest.NopType)
 			settings.Logger = zap.NewNop()
-
 			logs := plog.NewLogs()
-			lr := logs.ResourceLogs().AppendEmpty().ScopeLogs().AppendEmpty().LogRecords().AppendEmpty()
+			logRecord := logs.ResourceLogs().
+				AppendEmpty().
+				ScopeLogs().
+				AppendEmpty().
+				LogRecords().
+				AppendEmpty()
+
 			switch v := tc.body.(type) {
 			case string:
-				lr.Body().SetStr(v)
+				logRecord.Body().SetStr(v)
 			case int:
-				lr.Body().SetInt(int64(v))
+				logRecord.Body().SetInt(int64(v))
 			case []byte:
-				lr.Body().SetEmptyBytes().FromRaw(v)
+				logRecord.Body().SetEmptyBytes().FromRaw(v)
 			}
 
+			// Create sink and processor.
 			sink := &consumertest.LogsSink{}
-			p := newSyslogProcessor(sink, settings)
-			require.NoError(t, p.Start(ctx, nil))
+			processor := newSyslogProcessor(sink, settings)
 
-			err := p.ConsumeLogs(ctx, logs)
+			require.NoError(t, processor.Start(ctx, nil))
+			err := processor.ConsumeLogs(ctx, logs)
+			fmt.Println(err)
 			require.Error(t, err)
 
-			if tc.expectRecords == 0 {
-				assert.Equal(t, 0, sink.LogRecordCount(), "no logs should be produced")
-				require.NoError(t, p.Shutdown(ctx))
+			assert.Equal(t, tc.expectRecords, sink.LogRecordCount(), "unexpected number of logs produced")
 
-				return
-			}
+			require.NoError(t, processor.Shutdown(ctx))
 		})
 	}
 }

internal/config/config.go

@@ -186,7 +186,7 @@ func addDefaultPipelines(collector *Collector) {
 	if _, ok := collector.Pipelines.Logs[DefaultPipeline]; !ok {
 		collector.Pipelines.Logs[DefaultPipeline] = &Pipeline{
 			Receivers:  []string{"tcplog/nginx_app_protect"},
-			Processors: []string{"syslog/default", "logsgzip/default", "batch/default_logs"},
+			Processors: []string{"logsgzip/default", "batch/default_logs"},
 			Exporters:  []string{"otlp/default"},
 		}
 	}