Skip to content

Commit

Permalink
Move json_error to error.message and error.type (#4167)
Browse files Browse the repository at this point in the history
All errors are now under the error namespace. This should also be true for json errors. Currently they are put under `json_error`. This PR changes it to `error.message` and `error.type: json`. This makes it easy to search for all json errors. This is a breaking change.

* Docs updated
* Tests updated

The naming of the config option was not touched for better backward compatibility.
  • Loading branch information
ruflin authored and tsg committed May 10, 2017
1 parent 8662241 commit 97a549a
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 36 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ https://github.com/elastic/beats/compare/v5.4.0...v6.0.0-alpha1[View commits]
*Affecting all Beats*
- Introduce beat version in the Elasticsearch index and mapping template {pull}3527[3527]
- Introduce beat version in the Elasticsearch index and mapping template {pull}3527[3527]
- Usage of field `_type` is now ignored and hardcoded to `doc`. {pull}3757[3757]
- Change vendor manager from glide to govendor. {pull}3851[3851]
- Rename `error` field to `error.message`. {pull}3987[3987]
Expand All @@ -98,6 +98,7 @@ https://github.com/elastic/beats/compare/v5.4.0...v6.0.0-alpha1[View commits]
- Remove deprecated config options force_close_files and close_older. {pull}3768[3768]
- Change clean_removed behaviour to also remove states for files which cannot be found anymore under the same name. {pull}3827[3827]
- Remove `document_type` config option. Use `fields` instead. {pull}4204[4204]
- Move json_error under error.message and error.key. {pull}4167[4167]
*Packetbeat*
Expand Down
2 changes: 1 addition & 1 deletion filebeat/_meta/common.full.p2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ filebeat.prospectors:
# in case of conflicts.
#json.overwrite_keys: false

# If this setting is enabled, Filebeat adds a "json_error" key in case of JSON
# If this setting is enabled, Filebeat adds a "error.message" and "error.key: json" key in case of JSON
# unmarshaling errors or when a text key is defined in the configuration but cannot
# be used.
#json.add_error_key: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ If you enable this setting, the keys are copied top level in the output document
*`overwrite_keys`*:: If `keys_under_root` and this setting are enabled, then the values from the decoded
JSON object overwrite the fields that Filebeat normally adds (type, source, offset, etc.) in case of conflicts.

*`add_error_key`*:: If this setting is enabled, Filebeat adds a "json_error" key in case of JSON
*`add_error_key`*:: If this setting is enabled, Filebeat adds a "error.message" and "error.type: json" key in case of JSON
unmarshalling errors or when a `message_key` is defined in the configuration but cannot be used.

*`message_key`*:: An optional configuration setting that specifies a JSON key on
Expand Down
2 changes: 1 addition & 1 deletion filebeat/filebeat.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ filebeat.prospectors:
# in case of conflicts.
#json.overwrite_keys: false

# If this setting is enabled, Filebeat adds a "json_error" key in case of JSON
# If this setting is enabled, Filebeat adds a "error.message" and "error.key: json" key in case of JSON
# unmarshaling errors or when a text key is defined in the configuration but cannot
# be used.
#json.add_error_key: false
Expand Down
16 changes: 8 additions & 8 deletions filebeat/harvester/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestAddJSONFields(t *testing.T) {
ExpectedItems: common.MapStr{
"@timestamp": common.Time(now),
"type": "test",
"json_error": "@timestamp not overwritten (parse error on 2016-04-05T18:47:18.44XX4Z)",
"error": common.MapStr{"type": "json", "message": "@timestamp not overwritten (parse error on 2016-04-05T18:47:18.44XX4Z)"},
},
},
{
Expand All @@ -93,7 +93,7 @@ func TestAddJSONFields(t *testing.T) {
ExpectedItems: common.MapStr{
"@timestamp": common.Time(now),
"type": "test",
"json_error": "@timestamp not overwritten (not string)",
"error": common.MapStr{"type": "json", "message": "@timestamp not overwritten (not string)"},
},
},
{
Expand All @@ -102,8 +102,8 @@ func TestAddJSONFields(t *testing.T) {
Text: &text,
JSONConfig: reader.JSONConfig{KeysUnderRoot: true, OverwriteKeys: true},
ExpectedItems: common.MapStr{
"type": "test_type",
"json_error": "type not overwritten (not string)",
"type": "test_type",
"error": common.MapStr{"type": "json", "message": "type not overwritten (not string)"},
},
},
{
Expand All @@ -112,8 +112,8 @@ func TestAddJSONFields(t *testing.T) {
Text: &text,
JSONConfig: reader.JSONConfig{KeysUnderRoot: true, OverwriteKeys: true},
ExpectedItems: common.MapStr{
"type": "test_type",
"json_error": "type not overwritten (invalid value [])",
"type": "test_type",
"error": common.MapStr{"type": "json", "message": "type not overwritten (invalid value [])"},
},
},
{
Expand All @@ -122,8 +122,8 @@ func TestAddJSONFields(t *testing.T) {
Text: &text,
JSONConfig: reader.JSONConfig{KeysUnderRoot: true, OverwriteKeys: true},
ExpectedItems: common.MapStr{
"type": "test_type",
"json_error": "type not overwritten (invalid value [_type])",
"type": "test_type",
"error": common.MapStr{"type": "json", "message": "type not overwritten (invalid value [_type])"},
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion filebeat/harvester/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,6 @@ func (h *Harvester) mergeJSONFields(data common.MapStr, jsonFields common.MapStr
// Delete existing json key
delete(data, "json")

jsontransform.WriteJSONKeys(data, jsonFields, h.config.JSON.OverwriteKeys, reader.JsonErrorKey)
jsontransform.WriteJSONKeys(data, jsonFields, h.config.JSON.OverwriteKeys)
}
}
14 changes: 7 additions & 7 deletions filebeat/harvester/reader/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ import (
"github.com/elastic/beats/libbeat/logp"
)

const (
JsonErrorKey = "json_error"
)

type JSON struct {
reader Reader
cfg *JSONConfig
Expand All @@ -33,7 +29,7 @@ func (r *JSON) decodeJSON(text []byte) ([]byte, common.MapStr) {
if err != nil || jsonFields == nil {
logp.Err("Error decoding JSON: %v", err)
if r.cfg.AddErrorKey {
jsonFields = common.MapStr{JsonErrorKey: fmt.Sprintf("Error decoding JSON: %v", err)}
jsonFields = common.MapStr{"error": createJSONError(fmt.Sprintf("Error decoding JSON: %v", err))}
}
return text, jsonFields
}
Expand All @@ -45,15 +41,15 @@ func (r *JSON) decodeJSON(text []byte) ([]byte, common.MapStr) {
textValue, ok := jsonFields[r.cfg.MessageKey]
if !ok {
if r.cfg.AddErrorKey {
jsonFields[JsonErrorKey] = fmt.Sprintf("Key '%s' not found", r.cfg.MessageKey)
jsonFields["error"] = createJSONError(fmt.Sprintf("Key '%s' not found", r.cfg.MessageKey))
}
return []byte(""), jsonFields
}

textString, ok := textValue.(string)
if !ok {
if r.cfg.AddErrorKey {
jsonFields[JsonErrorKey] = fmt.Sprintf("Value of key '%s' is not a string", r.cfg.MessageKey)
jsonFields["error"] = createJSONError(fmt.Sprintf("Value of key '%s' is not a string", r.cfg.MessageKey))
}
return []byte(""), jsonFields
}
Expand Down Expand Up @@ -86,3 +82,7 @@ func (r *JSON) Next() (Message, error) {
message.AddFields(common.MapStr{"json": fields})
return message, nil
}

func createJSONError(message string) common.MapStr {
return common.MapStr{"message": message, "type": "json"}
}
8 changes: 4 additions & 4 deletions filebeat/harvester/reader/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,28 +121,28 @@ func TestDecodeJSON(t *testing.T) {
Text: `null`,
Config: JSONConfig{MessageKey: "value", AddErrorKey: true},
ExpectedText: `null`,
ExpectedMap: common.MapStr{"json_error": "Error decoding JSON: <nil>"},
ExpectedMap: common.MapStr{"error": common.MapStr{"message": "Error decoding JSON: <nil>", "type": "json"}},
},
{
// Add key error helps debugging this
Text: `{"message": "test", "value": "`,
Config: JSONConfig{MessageKey: "value", AddErrorKey: true},
ExpectedText: `{"message": "test", "value": "`,
ExpectedMap: common.MapStr{"json_error": "Error decoding JSON: unexpected EOF"},
ExpectedMap: common.MapStr{"error": common.MapStr{"message": "Error decoding JSON: unexpected EOF", "type": "json"}},
},
{
// If the text key is not found, put an error
Text: `{"message": "test", "value": "1"}`,
Config: JSONConfig{MessageKey: "hello", AddErrorKey: true},
ExpectedText: ``,
ExpectedMap: common.MapStr{"message": "test", "value": "1", "json_error": "Key 'hello' not found"},
ExpectedMap: common.MapStr{"message": "test", "value": "1", "error": common.MapStr{"message": "Key 'hello' not found", "type": "json"}},
},
{
// If the text key is found, but not a string, put an error
Text: `{"message": "test", "value": 1}`,
Config: JSONConfig{MessageKey: "value", AddErrorKey: true},
ExpectedText: ``,
ExpectedMap: common.MapStr{"message": "test", "value": int64(1), "json_error": "Value of key 'value' is not a string"},
ExpectedMap: common.MapStr{"message": "test", "value": int64(1), "error": common.MapStr{"message": "Value of key 'value' is not a string", "type": "json"}},
},
{
// Without a text key, simple return the json and an empty text
Expand Down
12 changes: 6 additions & 6 deletions filebeat/tests/system/test_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,16 @@ def test_timestamp_in_message(self):
assert output[0]["@timestamp"] == "2016-04-05T18:47:18.444Z"

assert output[1]["@timestamp"] != "invalid"
assert output[1]["json_error"] == \
assert output[1]["error.message"] == \
"@timestamp not overwritten (parse error on invalid)"

assert output[2]["json_error"] == \
assert output[2]["error.message"] == \
"@timestamp not overwritten (not string)"

assert "json_error" not in output[3]
assert "error" not in output[3]
assert output[3]["@timestamp"] == "2016-04-05T18:47:18.444Z", output[3]["@timestamp"]

assert "json_error" not in output[4]
assert "error" not in output[4]
assert output[4]["@timestamp"] == "2016-04-05T18:47:18.000Z", output[4]["@timestamp"]

def test_type_in_message(self):
Expand Down Expand Up @@ -241,11 +241,11 @@ def test_type_in_message(self):
assert output[0]["type"] == "test"

assert "type" not in output[1]
assert output[1]["json_error"] == \
assert output[1]["error.message"] == \
"type not overwritten (not string)"

assert "type" not in output[2]
assert output[2]["json_error"] == \
assert output[2]["error.message"] == \
"type not overwritten (not string)"

def test_with_generic_filtering(self):
Expand Down
15 changes: 10 additions & 5 deletions libbeat/common/jsontransform/jsonhelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,36 @@ import (
"github.com/elastic/beats/libbeat/logp"
)

func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKeys bool, errorKey string) {
// WriteJSONKeys writes the json keys to the given event based on the overwriteKeys option
func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKeys bool) {
for k, v := range keys {
if overwriteKeys {
if k == "@timestamp" {
vstr, ok := v.(string)
if !ok {
logp.Err("JSON: Won't overwrite @timestamp because value is not string")
event[errorKey] = "@timestamp not overwritten (not string)"
event["error"] = createJSONError("@timestamp not overwritten (not string)")
continue
}

// @timestamp must be of format RFC3339
ts, err := time.Parse(time.RFC3339, vstr)
if err != nil {
logp.Err("JSON: Won't overwrite @timestamp because of parsing error: %v", err)
event[errorKey] = fmt.Sprintf("@timestamp not overwritten (parse error on %s)", vstr)
event["error"] = createJSONError(fmt.Sprintf("@timestamp not overwritten (parse error on %s)", vstr))
continue
}
event[k] = common.Time(ts)
} else if k == "type" {
vstr, ok := v.(string)
if !ok {
logp.Err("JSON: Won't overwrite type because value is not string")
event[errorKey] = "type not overwritten (not string)"
event["error"] = createJSONError("type not overwritten (not string)")
continue
}
if len(vstr) == 0 || vstr[0] == '_' {
logp.Err("JSON: Won't overwrite type because value is empty or starts with an underscore")
event[errorKey] = fmt.Sprintf("type not overwritten (invalid value [%s])", vstr)
event["error"] = createJSONError(fmt.Sprintf("type not overwritten (invalid value [%s])", vstr))
continue
}
event[k] = vstr
Expand All @@ -49,3 +50,7 @@ func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKe

}
}

func createJSONError(message string) common.MapStr {
return common.MapStr{"message": message, "type": "json"}
}
2 changes: 1 addition & 1 deletion libbeat/processors/actions/decode_json_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (f decodeJSONFields) Run(event common.MapStr) (common.MapStr, error) {
default:
errs = append(errs, errors.New("Error trying to add target to root.").Error())
case map[string]interface{}:
jsontransform.WriteJSONKeys(event, t, f.overwriteKeys, "json_error")
jsontransform.WriteJSONKeys(event, t, f.overwriteKeys)
}
}
} else {
Expand Down

0 comments on commit 97a549a

Please sign in to comment.