-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Heartbeat] Use the proper 'base' data_stream as conf #28455
Conversation
Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead. Fixes elastic#28453
This pull request does not have a backport label. Could you fix it @andrewvc? 🙏
NOTE: |
Pinging @elastic/uptime (Team:Uptime) |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return nil, fmt.Errorf("could not determine base stream for config: %s", id) | ||
} | ||
|
||
err = res.Merge(common.MapStr{"id": optS.Id, "data_stream": optS.DataStream}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the use of this assigment here? seems like we are not using it.
remove debug statements below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I should just return the err (which should never happen really)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, it does return. If you have a return
with no args, it returns the variables initialized in the function signature as the return types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL.
Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead. To test this try using the config in the linked issue below as a heartbeat.yml Fixes #28453 (cherry picked from commit 2fcd0c4) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead. To test this try using the config in the linked issue below as a heartbeat.yml Fixes elastic#28453
@Mergifyio backport 7.15 |
✅ Backports have been created
|
Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead. To test this try using the config in the linked issue below as a heartbeat.yml Fixes #28453 (cherry picked from commit 2fcd0c4) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
@Mergifyio backport 8.0 |
✅ Backports have been created
|
…elastic#28896) Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead. To test this try using the config in the linked issue below as a heartbeat.yml Fixes elastic#28453 (cherry picked from commit bdde4c0) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
Previously we assumed the first config was the base stream, but lately fleet is sending things out of order, so we search for it instead.
To test this try using the config in the linked issue below as a heartbeat.yml
Fixes #28453
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.