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

Move json_error to error.message and error.type #4167

Merged
merged 1 commit into from
May 10, 2017

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented May 2, 2017

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.

@@ -49,3 +49,7 @@ func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKe

}
}

func CreateJSONError(message string) common.MapStr {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be exported? Seems simple enough to replicate if we need it elsewhere, similar to the debug function calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I exported it because I also use it in the json reader. But I can also just copy / paste the code to the json reader.

Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. See tiny comment and please add PR number to changelog.

@@ -8,35 +8,35 @@ import (
"github.com/elastic/beats/libbeat/logp"
)

func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKeys bool, errorKey string) {
func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKeys bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golint] reported by reviewdog 🐶
exported function WriteJSONKeys should have comment or be unexported

@@ -49,3 +49,7 @@ func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKe

}
}

func CreateJSONError(message string) common.MapStr {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golint] reported by reviewdog 🐶
exported function CreateJSONError should have comment or be unexported

@ruflin
Copy link
Contributor Author

ruflin commented May 2, 2017

PR added, function duplicated and golint errors fixed.

@ruflin
Copy link
Contributor Author

ruflin commented May 2, 2017

jenkins, retest it

@ruflin ruflin force-pushed the error.json branch 2 times, most recently from e480942 to 840e1e7 Compare May 4, 2017 07:10
@@ -8,35 +8,36 @@ import (
"github.com/elastic/beats/libbeat/logp"
)

func WriteJSONKeys(event common.MapStr, keys map[string]interface{}, overwriteKeys bool, errorKey string) {
// WritJSONKeys writes the json keys to the given event based on the overwriteKeys option
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[golint] reported by reviewdog 🐶
comment on exported function WriteJSONKeys should be of the form "WriteJSONKeys ..."

@ruflin ruflin force-pushed the error.json branch 4 times, most recently from a578ca5 to 183e793 Compare May 9, 2017 13:13
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.
@tsg tsg merged commit 97a549a into elastic:master May 10, 2017
@tsg tsg mentioned this pull request Jul 24, 2017
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants