Skip to content

Auditd package: revert back change to dynamic_dataset/namespace#6808

Closed
gsantoro wants to merge 2 commits intoelastic:mainfrom
gsantoro:feature/auditd_dynamic_dataset
Closed

Auditd package: revert back change to dynamic_dataset/namespace#6808
gsantoro wants to merge 2 commits intoelastic:mainfrom
gsantoro:feature/auditd_dynamic_dataset

Conversation

@gsantoro
Copy link
Copy Markdown
Contributor

@gsantoro gsantoro commented Jul 4, 2023

What does this PR do?

Revert back changes from issue elastic/kibana#157897.

Removing the following configs from the system package manifest

elasticsearch.dynamic_dataset: true
elasticsearch.dynamic_namespace: true

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

  • [ ]

How to test this PR locally

Related issues

Screenshots

@gsantoro gsantoro added enhancement New feature or request v8.9.0 labels Jul 4, 2023
@gsantoro gsantoro requested review from felixbarny and ruflin July 4, 2023 10:53
@gsantoro gsantoro self-assigned this Jul 4, 2023
@gsantoro gsantoro requested a review from a team as a code owner July 4, 2023 10:53
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jul 4, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-07-04T10:53:58.811+0000

  • Duration: 15 min 5 sec

Test stats 🧪

Test Results
Failed 0
Passed 9
Skipped 0
Total 9

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Copy Markdown

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (1/1) 💚
Classes 100.0% (1/1) 💚
Methods 100.0% (15/15) 💚 14.286
Lines 97.985% (2140/2184) 👍 4.214
Conditionals 100.0% (0/0) 💚

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

Can you please point me to an issue or comment somewhere that explains why these changes are being reverted. It wasn't obvious to me from looking at the linked issues.

If you do end up adding this back, could you please not use the dotted key format (relates to elastic/package-spec#538). i.e.

elasticsearch:
  dynamic_dataset: true
  dynamic_namespace: true

@gsantoro
Copy link
Copy Markdown
Contributor Author

gsantoro commented Jul 6, 2023

Actually, now that we discuss this further, I think we shouldn't revert these changes.

This integration is tailing the actual audits and not the logs of the auditd application. This datastream should eventually be an input package but for a similar reason why we decided to not rever the changes for kubernetes.container_logs, we shouldn't revert this change either.

@felixbarny and @ruflin do you agree to close this PR and not merge it?

@felixbarny
Copy link
Copy Markdown
Member

sgtm

@gsantoro
Copy link
Copy Markdown
Contributor Author

gsantoro commented Jul 6, 2023

I'm closing this PR without merging it. Reason at #6808 (comment).

@gsantoro gsantoro closed this Jul 6, 2023
@gsantoro gsantoro deleted the feature/auditd_dynamic_dataset branch July 6, 2023 11:57
@andrewkroh
Copy link
Copy Markdown
Member

andrewkroh commented Jul 6, 2023

This datastream should eventually be an input package

auditd does not seem like it should be an input package IMO. It's not a generic input type that you would be applying to different sources. It specifically reads the output of the Linux auditd daemon. This makes me wonder why we would need elasticsearch.dynamic_dataset and elasticsearch.dynamic_namespace in the first place. I think it would be best to continue with this and remove them.

andrewkroh added a commit that referenced this pull request Sep 20, 2023
auditd is not a generic input type that you would be applying to different sources.
It specifically reads the output of the Linux auditd daemon. It does not need to be able
to write to arbitrary logs-* data streams.

Relates: #6808 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants