[Heartbeat] Fix broken data_stream assignment by andrewvc · Pull Request #27535 · elastic/beats

andrewvc · 2021-08-20T15:28:49Z

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

My code follows the style guidelines of this project
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

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.

elasticmachine · 2021-08-20T15:28:57Z

Pinging @elastic/uptime (Team:Uptime)

elasticmachine · 2021-08-20T15:46:30Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-08-25T16:01:11.246+0000
Duration: 152 min 29 sec
Commit: 96e006d

Test stats 🧪

Test	Results
Failed	0
Passed	53711
Skipped	5324
Total	59035

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	53711
Skipped	5324
Total	59035

vigneshshanmugam

LGTM

vigneshshanmugam · 2021-08-20T15:45:36Z

heartbeat/monitors/factory.go

 		fields := clientCfg.Processing.Fields.Clone()
 		fields.Put("event.dataset", dataset)

-		if settings.DataStream != nil {


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?

I just moved the logic to the add_data_stream processor. So, it's still running.

vigneshshanmugam · 2021-08-20T15:48:02Z

libbeat/processors/add_data_stream/add_data_stream.go

+	if event.Fields == nil {
+		event.Fields = common.MapStr{}
+	}
+	event.PutValue("data_stream", eventDataStream)


Gotcha so we just moved the logic here 👍🏽

dominiqueclarke

This LGTM for screenshot and network data streams.

andrewvc · 2021-08-23T17:21:44Z

@Mergifyio update

mergify · 2021-08-23T17:21:49Z

Command update: success

Branch has been successfully updated

andrewvc · 2021-08-25T03:04:32Z

This should now fix event.dataset and the setting of other fields to be more consistent and clear in its logic in heartbeat.

This is now 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.

andrewvc · 2021-08-25T16:00:13Z

@Mergifyio update

mergify · 2021-08-25T16:00:42Z

Command update: success

Branch has been successfully updated

vigneshshanmugam

LGTM with new changes. Much cleaner indeed!

andrewvc · 2021-08-25T19:32:03Z

@Mergifyio backport 7.x

andrewvc · 2021-08-25T19:32:08Z

@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)

mergify · 2021-08-25T19:32:47Z

Command backport 7.x: success

Backports have been created

#27592 [Heartbeat] Fix broken data_stream assignment (backport #27535) has been created for branch 7.x

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)

mergify · 2021-08-25T19:33:16Z

Command backport 7.15: success

Backports have been created

#27593 [Heartbeat] Fix broken data_stream assignment (backport #27535) has been created for branch 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) 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.

andrewvc added bug Heartbeat v7.15.0 labels Aug 20, 2021

andrewvc requested review from justinkambic and vigneshshanmugam August 20, 2021 15:28

andrewvc self-assigned this Aug 20, 2021

andrewvc requested a review from a team as a code owner August 20, 2021 15:28

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 20, 2021

andrewvc added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label Aug 20, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 20, 2021

Fmt

0069de4

vigneshshanmugam approved these changes Aug 20, 2021

View reviewed changes

dominiqueclarke approved these changes Aug 20, 2021

View reviewed changes

andrewvc added 5 commits August 20, 2021 14:40

handle event dataset

591a9cc

Merge remote-tracking branch 'origin/master' into fix-ds-assign

291989a

Update enrich

757553a

Update dataset behavior to match expectations

bcb0592

Fix var name

782f2d0

mergify bot and others added 7 commits August 23, 2021 17:21

Merge branch 'master' into fix-ds-assign

a8cd147

Fix up logic

e36f0cf

Merge remote-tracking branch 'andrewvc/fix-ds-assign' into fix-ds-assign

d43e011

Scratch

f06c435

Refactor monitor dataset assignment to be consistent and make sense

2ae7154

Update python tests

d8f9258

Add changelog

a255573

Fix id invocation

7dc2b81

Merge branch 'master' into fix-ds-assign

96e006d

vigneshshanmugam approved these changes Aug 25, 2021

View reviewed changes

andrewvc merged commit 12df9f7 into elastic:master Aug 25, 2021

andrewvc deleted the fix-ds-assign branch August 25, 2021 19:31

mergify bot mentioned this pull request Aug 25, 2021

[Heartbeat] Fix broken data_stream assignment (backport #27535) #27592

Merged

mergify bot mentioned this pull request Aug 25, 2021

[Heartbeat] Fix broken data_stream assignment (backport #27535) #27593

Merged

andrewvc mentioned this pull request Aug 26, 2021

[Heartbeat] Disabled monitors in static monitor list cause crash #27599

Closed

dosera mentioned this pull request Dec 20, 2021

[Uptime] No monitors listed after upgrading beat to 7.16.2 elastic/kibana#121627

Closed

herrBez mentioned this pull request Apr 18, 2024

Prebuilt ML job for Heartbeat looks for no-more-existent dataset "uptime" elastic/kibana#181105

Closed

Conversation

andrewvc commented Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

How to test this PR locally

Uh oh!

elasticmachine commented Aug 20, 2021

Uh oh!

elasticmachine commented Aug 20, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

vigneshshanmugam left a comment

Choose a reason for hiding this comment

Uh oh!

vigneshshanmugam Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

andrewvc Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

vigneshshanmugam Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

andrewvc Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

dominiqueclarke left a comment

Choose a reason for hiding this comment

Uh oh!

andrewvc commented Aug 23, 2021

Uh oh!

mergify bot commented Aug 23, 2021

Uh oh!

andrewvc commented Aug 25, 2021

Uh oh!

andrewvc commented Aug 25, 2021

Uh oh!

mergify bot commented Aug 25, 2021

Uh oh!

vigneshshanmugam left a comment

Choose a reason for hiding this comment

Uh oh!

andrewvc commented Aug 25, 2021

Uh oh!

andrewvc commented Aug 25, 2021

Uh oh!

mergify bot commented Aug 25, 2021

Uh oh!

mergify bot commented Aug 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrewvc commented Aug 20, 2021 •

edited

Loading

elasticmachine commented Aug 20, 2021 •

edited by jenkins-beats-ci bot

Loading