-
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] Fix broken data_stream assignment #27535
Conversation
Fixes elastic#27478 This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation.
Pinging @elastic/uptime (Team:Uptime) |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
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
@@ -119,10 +119,6 @@ func newCommonPublishConfigs(info beat.Info, cfg *common.Config) (pipetool.Confi | |||
fields := clientCfg.Processing.Fields.Clone() | |||
fields.Put("event.dataset", dataset) | |||
|
|||
if settings.DataStream != nil { |
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.
Think i added this as a fix for Heartbeat not recognizing the data stream when used inside agent - #26774.
I hope it still works without this change?
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.
I just moved the logic to the add_data_stream
processor. So, it's still running.
if event.Fields == nil { | ||
event.Fields = common.MapStr{} | ||
} | ||
event.PutValue("data_stream", eventDataStream) |
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.
Gotcha so we just moved the logic here 👍🏽
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.
Yep
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.
@Mergifyio update |
Command
|
This should now fix This is now a breaking change, setting |
@Mergifyio update |
Command
|
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 with new changes. Much cleaner indeed!
@Mergifyio backport 7.x |
@Mergifyio backport 7.15 |
This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation. This is a breaking change, setting event.dataset consistently to the monitor type rather than the old value of uptime. Since this field is unlikely to have been used by anyone the impact is low, and it will bring us inline with the ECS spec requiring this field to match event.dataset. (cherry picked from commit 12df9f7)
Command
|
This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation. This is a breaking change, setting event.dataset consistently to the monitor type rather than the old value of uptime. Since this field is unlikely to have been used by anyone the impact is low, and it will bring us inline with the ECS spec requiring this field to match event.dataset. (cherry picked from commit 12df9f7)
Command
|
This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation. This is a breaking change, setting event.dataset consistently to the monitor type rather than the old value of uptime. Since this field is unlikely to have been used by anyone the impact is low, and it will bring us inline with the ECS spec requiring this field to match event.dataset. (cherry picked from commit 12df9f7) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation. This is a breaking change, setting event.dataset consistently to the monitor type rather than the old value of uptime. Since this field is unlikely to have been used by anyone the impact is low, and it will bring us inline with the ECS spec requiring this field to match event.dataset. (cherry picked from commit 12df9f7) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com>
This PR fixes the logic behind assigning the data_steam.dataset field. Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation. This is a breaking change, setting event.dataset consistently to the monitor type rather than the old value of uptime. Since this field is unlikely to have been used by anyone the impact is low, and it will bring us inline with the ECS spec requiring this field to match event.dataset.
Fixes #27478
This PR fixes the logic behind assigning the data_steam.dataset field.
Previously this was static per monitor type, only the index would change, but not the field. This makes the processor more comprehensive handling not just index naming but field generation.
No release note because it doesn't fix any public bugs.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally