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

i5942: http/json module: Allow for modifying json field names with do… #5970

Merged
merged 1 commit into from
Jan 9, 2018

Conversation

christiangalsterer
Copy link
Contributor

  • Introduced new config parameter dedot.enabled, with default value false.
  • Made all necessary changes to implementation, added test case and documentation.
  • Uses a local implementation for actual replacement of file and not yet the implementation in common.DeDot introduced in Dedot keys for jolokia metricsets #5957. Waiting for this PR to get merged and will change to this implementation (already prepared, just needs to be enabled).

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

err = json.Unmarshal(actualJsonResponse, &actualJsonBody)
assert.Nil(t, err)

dedottedJsonResponse, err := ioutil.ReadFile(absPath + "/json_response_dedot.json")

Choose a reason for hiding this comment

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

var dedottedJsonResponse should be dedottedJSONResponse

assert.NotNil(t, absPath)
assert.Nil(t, err)

actualJsonResponse, err := ioutil.ReadFile(absPath + "/json_response_with_dots.json")

Choose a reason for hiding this comment

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

var actualJsonResponse should be actualJSONResponse


func TestEventMapper(t *testing.T) {
var actualJsonBody map[string]interface{}
var expectedJsonBody map[string]interface{}

Choose a reason for hiding this comment

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

var expectedJsonBody should be expectedJSONBody

)

func TestEventMapper(t *testing.T) {
var actualJsonBody map[string]interface{}

Choose a reason for hiding this comment

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

var actualJsonBody should be actualJSONBody

@christiangalsterer
Copy link
Contributor Author

christiangalsterer commented Jan 5, 2018

Included changes from #5957, ready for review from my side.

@@ -153,3 +162,16 @@ func (m *MetricSet) getHeaders(header http.Header) map[string]string {
}
return headers
}

func replaceDots(data interface{}) interface{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should have this recursive function also in libbeat later on.

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 actually had the very same idea as it is quite likely that also beats have json as payload where in all keys "dots" shall be replaced.

I can open a new PR for this if you OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like a good idea.

@ruflin
Copy link
Contributor

ruflin commented Jan 9, 2018

jenkins, test it

@ruflin ruflin merged commit 590ecd2 into elastic:master Jan 9, 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