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.

Sign up for GitHub

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

Jump to bottom

fix number transformation from json files in heartbeat #13348

Merged
merged 3 commits into from
Sep 2, 2019
Merged

fix number transformation from json files in heartbeat #13348

merged 3 commits into from
Sep 2, 2019

Conversation

osherdp
Copy link
Contributor

@osherdp osherdp commented Aug 26, 2019

I noticed a bug when trying to use equals check at heartbeat.
I would like it to be merged and to get a fixed version soon.

I'm not a GO developer, so if you'd like to change what I wrote, feel free.

@osherdp osherdp requested a review from a team as a code owner August 26, 2019 19:50
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@osherdp
Copy link
Contributor Author

osherdp commented Aug 26, 2019

The bug is identical to the following:
#2038

@osherdp
Copy link
Contributor Author

osherdp commented Aug 27, 2019

@alvarolobato / @andrewvc do you have any spare time reviewing me?

@andrewvc
Copy link
Contributor

Sure, I'll take a look.

Jenkins, test this please

@andrewvc andrewvc added Team:obs-ds-hosted-services Label for the Observability Hosted Services team Heartbeat labels Aug 30, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime

@andrewvc andrewvc added the bug label Aug 30, 2019
@andrewvc
Copy link
Contributor

Looking good! Two things before we can merge this:

  1. Can you add a unit test to https://github.com/elastic/beats/blob/master/heartbeat/monitors/active/http/check_test.go ? While the integration test more comprehensive, this should be quick to add and may make it easier to spot the source of issues here. You could also just modify the existing unit test for testing JSON to add a numeric field.
  2. Can you add an entry to https://github.com/elastic/beats/blob/master/CHANGELOG.next.asciidoc under bugfixes for heartbeat?

Thanks for the great work! Once we have those in we can merge this.

@osherdp
Copy link
Contributor Author

osherdp commented Aug 31, 2019

@andrewvc Done!

@andrewvc
Copy link
Contributor

Jenkins, test this please

andrewvc
andrewvc approved these changes Sep 2, 2019
View reviewed changes
Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewvc andrewvc merged commit fb62c66 into elastic:master Sep 2, 2019
andrewvc pushed a commit to andrewvc/beats that referenced this pull request Sep 2, 2019
@osherdp @andrewvc
fix number transformation from json files in heartbeat (elastic#13348)
c930e98 
Prior to this patch heartbeat could not check JSON responses for numeric values correctly because they were incorrectly being cast to the wrong type for comparison.

(cherry picked from commit fb62c66)
andrewvc pushed a commit to andrewvc/beats that referenced this pull request Sep 2, 2019
@osherdp @andrewvc
fix number transformation from json files in heartbeat (elastic#13348)
122b83b 
Prior to this patch heartbeat could not check JSON responses for numeric values correctly because they were incorrectly being cast to the wrong type for comparison.

(cherry picked from commit fb62c66)
@andrewvc andrewvc added the v7.4.0 label Sep 2, 2019
@andrewvc
Copy link
Contributor

andrewvc commented Sep 2, 2019 •
edited
Loading

Thanks for the contribution! It will most likely go out in our 7.4.0 release. See the 7.4 backport here: #13465

andrewvc added a commit that referenced this pull request Sep 3, 2019
@andrewvc
fix number transformation from json files in heartbeat (#13348) (#13465)
da6b9f2 
Prior to this patch heartbeat could not check JSON responses for numeric values correctly because they were incorrectly being cast to the wrong type for comparison.

(cherry picked from commit fb62c66)
@urso urso added the v7.5.0 label Oct 22, 2019
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@andrewvc
fix number transformation from json files in heartbeat (elastic#13348) (
5accac9 
elastic#13465)

Prior to this patch heartbeat could not check JSON responses for numeric values correctly because they were incorrectly being cast to the wrong type for comparison.

(cherry picked from commit 90b7aa7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewvc andrewvc andrewvc approved these changes

Assignees
No one assigned
Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.4.0 v7.5.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@osherdp @elasticmachine @andrewvc @urso