Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type suffixes to all keys when logging #275

Merged
merged 4 commits into from Jul 18, 2019
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -73,6 +73,7 @@ func main() {
// TODO(albrow): Don't use global settings for logger.
log.SetFormatter(&log.JSONFormatter{})
log.SetLevel(log.Level(config.Verbosity))
log.AddHook(loghooks.NewKeySuffixHook())

// Parse private key file
privKey, err := initPrivateKey(config.PrivateKeyPath)
@@ -104,6 +104,7 @@ func New(config Config) (*App, error) {
// Configure logger
// TODO(albrow): Don't use global variables for log settings.
log.SetLevel(log.Level(config.Verbosity))
log.AddHook(loghooks.NewKeySuffixHook())
log.WithFields(map[string]interface{}{
"config": config,
"version": "development",
@@ -0,0 +1,116 @@
package loghooks

import (
"bytes"
"encoding"
"encoding/json"
"errors"
"fmt"
"reflect"
"strings"

log "github.com/sirupsen/logrus"
)

var errNestedMapType = errors.New("nested map types are not supported")

// KeySuffixHook is a logger hook that adds suffixes to all keys based on their
// type.
type KeySuffixHook struct{}

// NewKeySuffixHook creates and returns a new KeySuffixHook.
func NewKeySuffixHook() *KeySuffixHook {
return &KeySuffixHook{}
}

// Ensure that KeySuffixHook implements log.Hook.
var _ log.Hook = &KeySuffixHook{}

func (h *KeySuffixHook) Levels() []log.Level {
return log.AllLevels
}

func (h *KeySuffixHook) Fire(entry *log.Entry) error {
for key, value := range entry.Data {
typ, err := getTypeForValue(value)
if err != nil {
if err == errNestedMapType {
// We can't safely log nested map types, so replace the value with a
// string.
newKey := fmt.Sprintf("%s_json_string", key)
delete(entry.Data, key)
mapString, err := json.Marshal(value)
if err != nil {
return err
}
entry.Data[newKey] = string(mapString)
continue
} else {
return err
}
}
newKey := fmt.Sprintf("%s_%s", key, typ)
delete(entry.Data, key)
entry.Data[newKey] = value
}
return nil
}

// getTypeForValue returns a string representation of the type of the given val.
func getTypeForValue(val interface{}) (string, error) {
if _, ok := val.(json.Marshaler); ok {
// If val implements json.Marhsler, return the type of json.Marshal(val)
This conversation was marked as resolved by albrow

This comment has been minimized.

Copy link
@fabioberger

fabioberger Jul 18, 2019

Contributor
Suggested change
// If val implements json.Marhsler, return the type of json.Marshal(val)
// If val implements json.Marshaler, return the type of json.Marshal(val)
// instead of the type of val.
buf := &bytes.Buffer{}
if err := json.NewEncoder(buf).Encode(val); err != nil {
return "", err
}
var holder interface{}
if err := json.NewDecoder(buf).Decode(&holder); err != nil {
return "", err
}
return getTypeForValue(holder)
}
if _, ok := val.(encoding.TextMarshaler); ok {
// The json package always encodes values that implement
// encoding.TextMarshaler as a string.
return "string", nil
}

underlyingType := getUnderlyingType(reflect.TypeOf(val))
switch kind := underlyingType.Kind(); kind {
case reflect.Bool:
return "bool", nil
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Float32, reflect.Float64:
return "number", nil
case reflect.String, reflect.Complex64, reflect.Complex128, reflect.Func, reflect.Chan:
return "string", nil
case reflect.Array, reflect.Slice:
return "array", nil
case reflect.Map:
// Nested map types can't be efficiently indexed because they allow for
// arbitrary keys. We don't allow them.
return "", errNestedMapType
case reflect.Struct:
return getSafeStructTypeName(underlyingType)
default:
return "", fmt.Errorf("cannot determine type suffix for kind: %s", kind)
}
}

// getUnderlyingType returns the underlying type for the given type by
// recursively dereferencing pointer types.
func getUnderlyingType(typ reflect.Type) reflect.Type {
if typ.Kind() == reflect.Ptr {
return getUnderlyingType(typ.Elem())
}
return typ
}

// getSafeStructTypeName replaces dots in the name of the given type with
// underscores. Elasticsearch does not allow dots in key names.
func getSafeStructTypeName(typ reflect.Type) (string, error) {
unsafeTypeName := typ.String()
safeTypeName := strings.ReplaceAll(unsafeTypeName, ".", "_")
return safeTypeName, nil
}
@@ -0,0 +1,146 @@
package loghooks

import (
"fmt"
"testing"

"github.com/0xProject/0x-mesh/constants"

log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type myStruct struct {
myInt int
myString string
}

func TestGetTypeForValue(t *testing.T) {
testCases := []struct {
input interface{}
expected string
}{
{
input: true,
expected: "bool",
},
{
input: int(42),
expected: "number",
},
{
input: int8(42),
expected: "number",
},
{
input: int16(42),
expected: "number",
},
{
input: int32(42),
expected: "number",
},
{
input: int64(42),
expected: "number",
},
{
input: uint(42),
expected: "number",
},
{
input: uint8(42),
expected: "number",
},
{
input: uint16(42),
expected: "number",
},
{
input: uint32(42),
expected: "number",
},
{
input: uint64(42),
expected: "number",
},
{
input: float32(42),
expected: "number",
},
{
input: float64(42),
expected: "number",
},
{
input: "foo",
expected: "string",
},
{
input: complex64(42i + 7),
expected: "string",
},
{
input: complex128(42i + 7),
expected: "string",
},
{
input: func() {},
expected: "string",
},
{
input: make(chan struct{}),
expected: "string",
},
{
input: []int{},
expected: "array",
},
{
input: [...]int{},
expected: "array",
},
{
input: myStruct{},
expected: "loghooks_myStruct",
},
{
// Implements encoding.TextUnmarshaler but not json.Marshaler.
input: constants.NullAddress,
expected: "string",
},
{
// We don't expect the case of anonymous structs to come up often, but we
// do handle it correcly. " ", "{", "}", and ";" are allowed in
// Elasticsearch. The resulting string is ugly but at least it is
// guaranteed to prevent field mapping conflicts.
input: struct {
myInt int
myString string
}{},
expected: "struct { myInt int; myString string }",
},
}

for _, testCase := range testCases {
testCaseInfo := fmt.Sprintf("input: (%T) %v", testCase.input, testCase.input)
actual, err := getTypeForValue(testCase.input)
require.NoError(t, err, testCaseInfo)
assert.Equal(t, testCase.expected, actual, testCaseInfo)
}
}

func TestKeySuffixHookWithNestedMapType(t *testing.T) {
hook := NewKeySuffixHook()
entry := &log.Entry{
Data: log.Fields{
"myMap": map[string]int{"one": 1},
},
}
require.NoError(t, hook.Fire(entry))
expectedData := log.Fields{
"myMap_json_string": `{"one":1}`,
}
assert.Equal(t, expectedData, entry.Data)
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.