Skip to content

Commit

Permalink
sllogformat: report warning instead of error for empty line on the la…
Browse files Browse the repository at this point in the history
…st profile (#244)
  • Loading branch information
watercraft authored Sep 23, 2024
1 parent aba67b9 commit f9cb31e
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 6 deletions.
12 changes: 9 additions & 3 deletions sllogformatprocessor/batch_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,15 @@ func (bl *batchLogs) addToBatch(ld plog.Logs) {
ils.LogRecords().RemoveIf(func(lr plog.LogRecord) bool {
gen, req, err := bl.cfg.MatchProfile(bl.log, rl, ils, lr)
if err != nil {
bl.log.Error("Failed to match profile",
zap.String("err", err.Error()))
bl.dumpLogRecord(rl, ils, lr)
switch err {
case errEmptyLine:
bl.log.Warn("Skipping log record",
zap.String("err", err.Error()))
default:
bl.log.Error("Failed to match profile",
zap.String("err", err.Error()))
bl.dumpLogRecord(rl, ils, lr)
}
return true
}
reqBytes, err := json.Marshal(req)
Expand Down
14 changes: 11 additions & 3 deletions sllogformatprocessor/match_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package sllogformatprocessor // import "github.com/open-telemetry/opentelemetry-collector-contrib/processor/sllogformatprocessor"
import (
"encoding/json"
"errors"
"fmt"
"regexp"
"strconv"
Expand All @@ -27,6 +28,8 @@ import (
"go.uber.org/zap"
)

var errEmptyLine error = errors.New("Empty log message")

type StreamTokenReq struct {
Stream string `json:"stream"`
Logbasename string `json:"logbasename"`
Expand Down Expand Up @@ -389,7 +392,7 @@ type ConfigResult struct {
func (c *Config) MatchProfile(log *zap.Logger, rl plog.ResourceLogs, ils plog.ScopeLogs, lr plog.LogRecord) (*ConfigResult, *StreamTokenReq, error) {
var id, ret string
reasons := []string{}
for _, profile := range c.Profiles {
for idx, profile := range c.Profiles {
req := newStreamTokenReq()
gen := ConfigResult{}
parser := Parser{
Expand Down Expand Up @@ -452,6 +455,11 @@ func (c *Config) MatchProfile(log *zap.Logger, rl plog.ResourceLogs, ils plog.Sc
}
_, gen.Message = parser.EvalElem(profile.Message)
if gen.Message == "" {
if idx >= len(c.Profiles)-1 {
// If this is the last configured profile and we have no message body,
// report it as a warning instead of an error
return nil, nil, errEmptyLine
}
reasons = append(reasons, "message")
continue
}
Expand Down Expand Up @@ -483,8 +491,8 @@ func (c *Config) MatchProfile(log *zap.Logger, rl plog.ResourceLogs, ils plog.Sc
}
}
}
fmt.Printf("MSG bytes: %+v\n", []byte(gen.Message))
fmt.Printf("MSG string: %s\n", gen.Message)
// fmt.Printf("MSG bytes: %+v\n", []byte(gen.Message))
// fmt.Printf("MSG string: %s\n", gen.Message)
gen.Format = profile.Format
return &gen, &req, nil
}
Expand Down
199 changes: 199 additions & 0 deletions sllogformatprocessor/match_profile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package sllogformatprocessor

import (
"testing"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/pdata/plog"
"go.uber.org/zap"
)

func TestMatchProfileSkipLogic(t *testing.T) {
factory := NewFactory()
cfg := factory.CreateDefaultConfig()

// Assert type assertion for *Config
config, ok := cfg.(*Config)
if !ok {
t.Fatalf("Expected *Config but got %T", cfg)
}

// Update the config with the profile information
config.Profiles = []ConfigProfile{
{
ServiceGroup: &ConfigAttribute{
Rename: "service_group",
Exp: &ConfigExpression{
Source: "lit:default-group",
},
},
Host: &ConfigAttribute{
Rename: "host",
Exp: &ConfigExpression{
Source: "lit:test-host",
},
},
Logbasename: &ConfigAttribute{
Rename: "logbasename",
Exp: &ConfigExpression{
Source: "lit:example-log",
},
},
Severity: &ConfigAttribute{
Exp: &ConfigExpression{
Source: "lit:INFO",
},
},
Message: &ConfigAttribute{
Exp: &ConfigExpression{
Source: "body", // Message comes from the log body
},
},
},
}

originalProfile := config.Profiles[0]

logger := zap.NewNop()
mockResourceLogs := plog.NewResourceLogs()
mockScopeLogs := mockResourceLogs.ScopeLogs().AppendEmpty()

// Table-driven test cases
testCases := []struct {
name string
ServiceGroup string
Host string
Logbasename string
Severity string
Message string
expectError bool
specificError error
}{
{
name: "All fields valid",
ServiceGroup: "default-group",
Host: "test-host",
Logbasename: "example-log",
Severity: "INFO",
Message: "valid log line",
expectError: false,
specificError: nil,
},
{
name: "Empty service_group",
ServiceGroup: "",
Host: "test-host",
Logbasename: "example-log",
Severity: "INFO",
Message: "valid log line",
expectError: true,
specificError: nil,
},
{
name: "Empty host",
ServiceGroup: "default-group",
Host: "",
Logbasename: "example-log",
Severity: "INFO",
Message: "valid log line",
expectError: true,
specificError: nil,
},
{
name: "Empty logbasename",
ServiceGroup: "default-group",
Host: "test-host",
Logbasename: "",
Severity: "INFO",
Message: "valid log line",
expectError: true,
specificError: nil,
},
{
name: "Empty severity",
ServiceGroup: "default-group",
Host: "test-host",
Logbasename: "example-log",
Severity: "",
Message: "valid log line",
// By setting the severity config to nil (below) we are asking the match
// to use the log stream's severity number which does not generate an error.
expectError: false,
specificError: nil,
},
{
name: "Empty message",
ServiceGroup: "default-group",
Host: "test-host",
Logbasename: "example-log",
Severity: "INFO",
Message: "",
expectError: true,
specificError: errEmptyLine,
},
{
name: "Log line with unprintable characters",
ServiceGroup: "default-group",
Host: "test-host",
Logbasename: "example-log",
Severity: "INFO",
Message: "\x00\x01\x02\x03",
expectError: true,
specificError: errEmptyLine,
},
}

// Iterate through test cases
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Modify the first profile based on the test case
if tc.ServiceGroup == "" {
config.Profiles[0].ServiceGroup = nil
}
if tc.Host == "" {
config.Profiles[0].Host = nil
}
if tc.Logbasename == "" {
config.Profiles[0].Logbasename = nil
}
if tc.Severity == "" {
config.Profiles[0].Severity = nil
}
if tc.Message == "" {
config.Profiles[0].Message = nil
}

// Create log record for the test case
createLogRecord(mockScopeLogs, tc.Message)

// Get the last log record (the one just created), and attempt to match a profile
logRecord := mockScopeLogs.LogRecords().At(mockScopeLogs.LogRecords().Len() - 1)
gen, req, err := config.MatchProfile(logger, mockResourceLogs, mockScopeLogs, logRecord)

// Validate the error based on test case expectations
if tc.expectError {
if tc.specificError != nil {
assert.ErrorAs(t, err, &tc.specificError, "expected specific error for log record")
} else {
assert.Error(t, err, "expected some error for log record")
}
assert.Empty(t, gen, "gen should be empty when log record is skipped")
assert.Empty(t, req, "req should be empty when log record is skipped")
} else {
assert.NoError(t, err, "expected no error for valid log record")
assert.NotEmpty(t, gen, "gen should not be empty for valid log record")
assert.NotEmpty(t, req, "req should not be empty for valid log record")
}

// Restore the original profile
config.Profiles[0] = originalProfile
})
}

}

// Creates a log record with specific attributes and log line
func createLogRecord(scopeLogs plog.ScopeLogs, logLine string) {
logRecord := scopeLogs.LogRecords().AppendEmpty()
logRecord.Body().SetStr(logLine)
}

0 comments on commit f9cb31e

Please sign in to comment.