Skip to content

Commit

Permalink
improvement: Rework options
Browse files Browse the repository at this point in the history 
Splits options into logger-level and message-level options.

Semver: Patch
Ref: LOG-7386
  • Loading branch information
@dm36
dm36 committed Sep 28, 2020
1 parent 1bec0a7 commit a9305ea
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 68 deletions.
12 changes: 6 additions & 6 deletions logger/logger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ import (

// Logger is the means by which a user can begin to send logs.
type Logger struct {
Options Options

Options Options
transport *transport
}

// Message represents a single log message and associated options.
type Message struct {
Body string
Options Options
Options MessageOptions
}

// Payload contains the properties sent to the ingestion endpoint.
Expand Down Expand Up @@ -84,16 +83,17 @@ func (l *Logger) Close() {

// Log sends a provided log message to LogDNA.
func (l *Logger) Log(message string) {
msgOpts := MessageOptions{App: l.Options.App, Env: l.Options.Env, FlushInterval: l.Options.FlushInterval, SendTimeout: l.Options.SendTimeout, Hostname: l.Options.Hostname, IndexMeta: l.Options.IndexMeta, IngestURL: l.Options.IngestURL, IPAddress: l.Options.IPAddress, Level: l.Options.Level, MacAddress: l.Options.MacAddress, MaxBufferLen: l.Options.MaxBufferLen, Meta: l.Options.Meta, Tags: l.Options.Tags}
logMsg := Message{
Body: message,
Options: l.Options,
Options: msgOpts,
}
l.transport.add(logMsg)
}

// LogWithOptions allows the user to update options uniquely for a given log message
// before sending the log to LogDNA.
func (l *Logger) LogWithOptions(message string, options Options) error {
func (l *Logger) LogWithOptions(message string, options MessageOptions) error {
msgOpts := l.Options.merge(options)
err := msgOpts.validate()
if err != nil {
Expand All @@ -111,7 +111,7 @@ func (l *Logger) LogWithOptions(message string, options Options) error {

// LogWithLevel sends a log message to LogDNA with a parameterized level.
func (l *Logger) LogWithLevel(message string, level string) error {
options := Options{Level: level}
options := MessageOptions{Level: level}
return l.LogWithOptions(message, options)
}

Expand Down
20 changes: 10 additions & 10 deletions logger/logger_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestLogger_NewLogger(t *testing.T) {

t.Run("Invalid options", func(t *testing.T) {
o := Options{
Level: strings.Repeat("a", 33),
Level: strings.Repeat("a", 83),
}

_, err := NewLogger(o, "abc123")
Expand All @@ -58,7 +58,7 @@ func TestLogger_Log(t *testing.T) {
Hostname: "foo",
App: "test",
IPAddress: "127.0.0.1",
MacAddress: "C0:FF:EE:C0:FF:EE",
MacAddress: "c0:ff:ee:c0:ff:ee",
}

l, err := NewLogger(o, "abc123")
Expand All @@ -71,7 +71,7 @@ func TestLogger_Log(t *testing.T) {
assert.Equal(t, "abc123", body["apikey"])
assert.Equal(t, "foo", body["hostname"])
assert.Equal(t, "127.0.0.1", body["ip"])
assert.Equal(t, "C0:FF:EE:C0:FF:EE", body["mac"])
assert.Equal(t, "c0:ff:ee:c0:ff:ee", body["mac"])
assert.NotEmpty(t, body["lines"])

ls := body["lines"].([]interface{})
Expand Down Expand Up @@ -103,16 +103,16 @@ func TestLogger_LogWithOptions(t *testing.T) {
App: "app",
Env: "development",
Level: "info",
Timestamp: now,
}

l, err := NewLogger(o, "abc123")
assert.Equal(t, nil, err)

l.LogWithOptions("testing", Options{
App: "anotherapp",
Env: "production",
Level: "error",
l.LogWithOptions("testing", MessageOptions{
App: "anotherapp",
Env: "production",
Level: "error",
Timestamp: time.Now(),
})
l.Close()

Expand All @@ -138,8 +138,8 @@ func TestLogger_LogWithOptions(t *testing.T) {
l, err := NewLogger(o, "abc123")
assert.Equal(t, nil, err)

err = l.LogWithOptions("testing", Options{
App: strings.Repeat("a", 33),
err = l.LogWithOptions("testing", MessageOptions{
App: strings.Repeat("a", 83),
})

assert.Error(t, err)
Expand Down
130 changes: 105 additions & 25 deletions logger/options.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package logger

import (
"errors"
"fmt"
"net"
"regexp"
"strings"
"time"
Expand All @@ -13,6 +13,7 @@ const (
defaultSendTimeout = 30 * time.Second
defaultFlushInterval = 250 * time.Millisecond
defaultMaxBufferLen = 50
maxOptionLength = 80
)

// InvalidOptionMessage represents an issue with the supplied configuration.
Expand All @@ -21,8 +22,7 @@ type InvalidOptionMessage struct {
Message string
}

// Options encapsulates user-provided options such as the Level and App
// that are passed along with each log.
// Options encapsulates user-provided options.
type Options struct {
App string
Env string
Expand All @@ -37,50 +37,131 @@ type Options struct {
MaxBufferLen int
Meta string
Tags string
}

// MessageOptions encapsulates user-provided options such as the Level and App
// that are passed along with each log.
type MessageOptions struct {
App string
Env string
FlushInterval time.Duration
SendTimeout time.Duration
Hostname string
IndexMeta bool
IngestURL string
IPAddress string
Level string
MacAddress string
MaxBufferLen int
Meta string
Tags string
Timestamp time.Time
}

func (e InvalidOptionMessage) String() string {
return fmt.Sprintf("Options.%s: %s", e.Option, e.Message)
type fieldIssue struct {
field string
prob string
}

func validateOptionLength(option string, value string, problems *[]string) {
if len(value) > 32 {
*problems = append(*problems, InvalidOptionMessage{option, "length must be less than 32"}.String())
type optionsError struct {
issues []fieldIssue
}

func (e *optionsError) Error() string {
var str strings.Builder
str.WriteString("One or more invalid options:\n")
for i := 0; i < len(e.issues); i++ {
str.WriteString(fmt.Sprintf("%s: %s\n", e.issues[i].field, e.issues[i].prob))
}
return str.String()
}

func validateOptionLength(option string, value string) *fieldIssue {
if len(value) > maxOptionLength {
return &fieldIssue{field: option, prob: "length must be less than 80"}
}
return nil
}

func (options *Options) validate() error {
var problems []string
reMacAddress := regexp.MustCompile(`^([0-9a-fA-F][0-9a-fA-F]:){5}([0-9a-fA-F][0-9a-fA-F])`)
reHostname := regexp.MustCompile(`(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9])`)
reIPAddress := regexp.MustCompile(`(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}`)

validateOptionLength("App", options.App, &problems)
validateOptionLength("Env", options.Env, &problems)
validateOptionLength("Hostname", options.Hostname, &problems)
validateOptionLength("Level", options.Level, &problems)
var issues []fieldIssue

if options.MacAddress != "" && (!reMacAddress.MatchString(options.MacAddress)) {
problems = append(problems, InvalidOptionMessage{"MacAddress", "invalid format"}.String())
if issue := validateOptionLength("App", options.App); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Env", options.Env); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Hostname", options.Hostname); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Level", options.Level); issue != nil {
issues = append(issues, *issue)
}

if options.MacAddress != "" {
mac, err := net.ParseMAC(options.MacAddress)
if err != nil {
issues = append(issues, fieldIssue{"MacAddress", "Invalid format"})
} else {
options.MacAddress = mac.String()
}
}
if options.Hostname != "" && !reHostname.MatchString(options.Hostname) {
problems = append(problems, InvalidOptionMessage{"Hostname", "invalid format"}.String())
issues = append(issues, fieldIssue{"Hostname", "Invalid format"})
}
if options.IPAddress != "" && net.ParseIP(options.IPAddress) == nil {
issues = append(issues, fieldIssue{"IPAddress", "Invalid format"})
}

if options.IPAddress != "" && !reIPAddress.MatchString(options.IPAddress) {
problems = append(problems, InvalidOptionMessage{"IPAddress", "invalid format"}.String())
if len(issues) > 0 {
return &optionsError{issues: issues}
}
return nil
}

func (options *MessageOptions) validate() error {
reHostname := regexp.MustCompile(`(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9])`)

var issues []fieldIssue

if len(problems) > 0 {
return errors.New(strings.Join(problems, ", "))
if issue := validateOptionLength("App", options.App); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Env", options.Env); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Hostname", options.Hostname); issue != nil {
issues = append(issues, *issue)
}
if issue := validateOptionLength("Level", options.Level); issue != nil {
issues = append(issues, *issue)
}

if options.MacAddress != "" {
mac, err := net.ParseMAC(options.MacAddress)
if err != nil {
issues = append(issues, fieldIssue{"MacAddress", "Invalid format"})
} else {
options.MacAddress = mac.String()
}
}
if options.Hostname != "" && !reHostname.MatchString(options.Hostname) {
issues = append(issues, fieldIssue{"Hostname", "Invalid format"})
}
if options.IPAddress != "" && net.ParseIP(options.IPAddress) == nil {
issues = append(issues, fieldIssue{"IPAddress", "Invalid format"})
}

if len(issues) > 0 {
return &optionsError{issues: issues}
}
return nil
}

func (options Options) merge(merge Options) Options {
func (options Options) merge(merge MessageOptions) MessageOptions {
newOpts := options
if merge.App != "" {
newOpts.App = merge.App
Expand All @@ -94,8 +175,7 @@ func (options Options) merge(merge Options) Options {
if merge.Meta != "" {
newOpts.Meta = merge.Meta
}

return newOpts
return MessageOptions{App: newOpts.App, Env: newOpts.Env, FlushInterval: newOpts.FlushInterval, SendTimeout: newOpts.SendTimeout, Hostname: newOpts.Hostname, IndexMeta: newOpts.IndexMeta, IngestURL: newOpts.IngestURL, IPAddress: newOpts.IPAddress, Level: newOpts.Level, MacAddress: newOpts.MacAddress, MaxBufferLen: newOpts.MaxBufferLen, Meta: newOpts.Meta, Tags: newOpts.Tags, Timestamp: merge.Timestamp}
}

func (options *Options) setDefaults() {
Expand Down
63 changes: 46 additions & 17 deletions logger/options_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,54 @@ func TestOptions_Validate(t *testing.T) {
testCases := []struct {
label string
options Options
valid bool
errStr string
}{
{"Base case", Options{}, true},
{"With options", Options{Level: "error", Hostname: "foo", App: "app", IPAddress: "127.0.0.1", MacAddress: "C0:FF:EE:C0:FF:EE"}, true},
{"App length", Options{App: strings.Repeat("a", 33)}, false},
{"Env length", Options{Env: strings.Repeat("a", 33)}, false},
{"Hostname length", Options{Hostname: strings.Repeat("a", 33)}, false},
{"Level length", Options{Level: strings.Repeat("a", 33)}, false},
{"Invalid MacAddress", Options{MacAddress: "in:va:lid"}, false},
{"Invalid Hostname", Options{Hostname: "-"}, false},
{"Invalid IPAddress", Options{IPAddress: "localhost"}, false},
{"Base case", Options{}, ""},
{"With options", Options{Level: "error", Hostname: "foo", App: "app", IPAddress: "127.0.0.1", MacAddress: "C0:FF:EE:C0:FF:EE"}, ""},
{"App length", Options{App: strings.Repeat("a", 83)}, "One or more invalid options:\nApp: length must be less than 80\n"},
{"Env length", Options{Env: strings.Repeat("a", 83)}, "One or more invalid options:\nEnv: length must be less than 80\n"},
{"Hostname length", Options{Hostname: strings.Repeat("a", 83)}, "One or more invalid options:\nHostname: length must be less than 80\n"},
{"Level length", Options{Level: strings.Repeat("a", 83)}, "One or more invalid options:\nLevel: length must be less than 80\n"},
{"Invalid MacAddress", Options{MacAddress: "in:va:lid"}, "One or more invalid options:\nMacAddress: Invalid format\n"},
{"Invalid Hostname", Options{Hostname: "-"}, "One or more invalid options:\nHostname: Invalid format\n"},
{"Invalid IPAddress", Options{IPAddress: "localhost"}, "One or more invalid options:\nIPAddress: Invalid format\n"},
}

for _, tc := range testCases {
t.Run(tc.label, func(t *testing.T) {
err := tc.options.validate()
if tc.valid {
if tc.errStr == "" {
assert.Equal(t, nil, err)
} else {
assert.Error(t, err)
assert.EqualError(t, err, tc.errStr)
}
})
}
}

func TestMessageOptions_Validate(t *testing.T) {
testCases := []struct {
label string
options MessageOptions
errStr string
}{
{"Base case", MessageOptions{}, ""},
{"With options", MessageOptions{Level: "error", Hostname: "foo", App: "app", IPAddress: "127.0.0.1", MacAddress: "C0:FF:EE:C0:FF:EE"}, ""},
{"App length", MessageOptions{App: strings.Repeat("a", 83)}, "One or more invalid options:\nApp: length must be less than 80\n"},
{"Env length", MessageOptions{Env: strings.Repeat("a", 83)}, "One or more invalid options:\nEnv: length must be less than 80\n"},
{"Hostname length", MessageOptions{Hostname: strings.Repeat("a", 83)}, "One or more invalid options:\nHostname: length must be less than 80\n"},
{"Level length", MessageOptions{Level: strings.Repeat("a", 83)}, "One or more invalid options:\nLevel: length must be less than 80\n"},
{"Invalid MacAddress", MessageOptions{Hostname: "-"}, "One or more invalid options:\nHostname: Invalid format\n"},
{"Invalid IPAddress", MessageOptions{IPAddress: "localhost"}, "One or more invalid options:\nIPAddress: Invalid format\n"},
}

for _, tc := range testCases {
t.Run(tc.label, func(t *testing.T) {
err := tc.options.validate()
if tc.errStr == "" {
assert.Equal(t, nil, err)
} else {
assert.EqualError(t, err, tc.errStr)
}
})
}
Expand All @@ -45,17 +73,18 @@ func TestOptions_Merge(t *testing.T) {
Meta: `{"foo": "bar"}`,
}

o = o.merge(Options{
newOptions := MessageOptions{}
newOptions = o.merge(MessageOptions{
App: "merge",
Env: "merge",
Level: "merge",
Meta: `{"baz": "merge"}`,
})

assert.Equal(t, "merge", o.App)
assert.Equal(t, "merge", o.Env)
assert.Equal(t, "merge", o.Level)
assert.Equal(t, `{"baz": "merge"}`, o.Meta)
assert.Equal(t, "merge", newOptions.App)
assert.Equal(t, "merge", newOptions.Env)
assert.Equal(t, "merge", newOptions.Level)
assert.Equal(t, `{"baz": "merge"}`, newOptions.Meta)
}

func TestOptions_SetDefaults(t *testing.T) {
Expand Down
Loading

0 comments on commit a9305ea

Please sign in to comment.