Skip to content

[system] Add missing conditional checks to render preserve_original_event tag when toggled on #9426

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

Merged
merged 5 commits into from
Apr 23, 2024
Merged

Conversation

devamanv
Copy link
Contributor

@devamanv devamanv commented Mar 22, 2024

Proposed commit message

The PR contains changes to make sure the preserve_original_event tag is rendered without having to add that manually. Complete details have been captured in the related issue. Affected datastreams are application, security, and system.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

Following are some of the sanity checks that must be passed at all times:

no preserve_original_event and no tags ==> no tags are rendered
no preserve_original_event but tags ==> only tags should be rendered
preserve_original_event but no tags ==> only preserve_original_event is rendered under the tags section
preserve_original_event and tags ==> render all tags including preserve_original_event under the tags section 

How to test this PR locally

  • Spin up the Elastic stack and point your elastic-package to the running Kibana instance
  • Build the package locally and install the package using elastic-package install --zip system-1.54.1.zip
  • Upgrade the integration to the latest version i.e. 1.54.1, there shouldn't be any errors. If you find any errors while upgrading, please report it by adding a comment to this PR.
  • Add the integration to an agent policy and notice the tags sections application, security, and system datastream. The preserve_original_event should be rendered when toggled on, and all others tags should render if added in the advanced options at the time of adding the integration.

Related issues

Screenshots

image
imageimage

@elasticmachine
Copy link

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

Copy link

Quality Gate passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No Coverage information No data about Coverage
No Duplication information No data about Duplication

See analysis details on SonarQube

@botelastic
Copy link

botelastic bot commented Apr 21, 2024

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added Stalled and removed Stalled labels Apr 21, 2024
Comment on lines +21 to +31
{{#each tags as |tag|}}
- {{tag}}
{{/each}}
{{#if preserve_original_event}}
- preserve_original_event
{{/if}}
{{else}}
{{#if preserve_original_event}}
tags:
- preserve_original_event
{{/if}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested this rendering logic on a handlebar playground

@devamanv devamanv marked this pull request as ready for review April 23, 2024 04:56
@devamanv devamanv requested review from a team as code owners April 23, 2024 04:56
@devamanv devamanv added the bugfix Pull request that fixes a bug issue label Apr 23, 2024
Copy link
Contributor

@intxgo intxgo left a comment

Choose a reason for hiding this comment

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

LGTM, this code does what's been described in the linked issue

@shmsr shmsr changed the title [system] Add missing conditional checks to render preserve_original_event tag when toggled on [system] Add missing conditional checks to render preserve_original_event tag when toggled on Apr 23, 2024
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @devamanv

@devamanv devamanv merged commit 73da1da into elastic:main Apr 23, 2024
@devamanv devamanv deleted the system-winlog-tags branch April 23, 2024 18:09
@elasticmachine
Copy link

Package system - 1.55.1 containing this change is available at https://epr.elastic.co/search?package=system

qcorporation pushed a commit that referenced this pull request Feb 3, 2025
qcorporation pushed a commit that referenced this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request that fixes a bug issue Integration:system System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants