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

#6066: Extract json deDot feature from metricbeat/http module to libbeat/common #6067

Merged
merged 2 commits into from
Jan 17, 2018

Conversation

christiangalsterer
Copy link
Contributor

No description provided.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?


// DeDot a json struct by replacing all . with _
// This helps when sending data to Elasticsearch to prevent object and key collisions.
func DeDotJson(json interface{}) interface{} {

Choose a reason for hiding this comment

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

func DeDotJson should be DeDotJSON

@@ -256,3 +256,24 @@ func (f Float) MarshalJSON() ([]byte, error) {
func DeDot(s string) string {
return strings.Replace(s, ".", "_", -1)
}

// DeDot a json struct by replacing all . with _

Choose a reason for hiding this comment

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

comment on exported function DeDotJson should be of the form "DeDotJson ..."

@christiangalsterer christiangalsterer changed the title #6066: Extract json deDot feature from metricbeat/http module to libb… #6066: Extract json deDot feature from metricbeat/http module to libbeat/common Jan 13, 2018
@christiangalsterer
Copy link
Contributor Author

@ruflin
Hi Nicolas,

as discussed here the PR for extracting the de dot feature for json into libbeats.
Please let me you of any feedback/changes.

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. Left a minor comment about the naming of the testdata directory.

@@ -3,8 +3,8 @@
"key_with_dot_l2": 1,
"key_with_multiple_dots_l2": 2,
"key_without_dot_l2": {
"key_with_dot_l2": 3,
"key_with_multiple_dots_l2": 4
"key_with_dot_l3": 3,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call the directory testdata which is more idiomatic for Golang. The _meta data directory is specific to our modules where we automatically generate some information, we don't need it here I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the test data to testdata folder

…to libbeat/common

- Moved test data to testdata folder
@ruflin
Copy link
Contributor

ruflin commented Jan 16, 2018

jenkins, test it

1 similar comment
@ruflin
Copy link
Contributor

ruflin commented Jan 16, 2018

jenkins, test it

@ruflin ruflin merged commit 96429fa into elastic:master Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants