Skip to content
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

Propagate input id from the Agent policy into Filebeat configuration #30386

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

ph
Copy link
Contributor

@ph ph commented Feb 14, 2022

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300

What does this PR do?

Why is it important?

Checklist

  • My code follows the style guidelines of this project
  • 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
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@ph ph added backport-v8.0.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team backport-v8.1.0 Automated backport with mergify backport-v8.2.0 Automated backport with mergify labels Feb 14, 2022
@ph ph requested review from kvch and a team February 14, 2022 22:12
@ph ph self-assigned this Feb 14, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Feb 14, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 15, 2022

💚 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: 2022-03-14T18:46:57.478+0000

  • Duration: 93 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 6349
Skipped 14
Total 6363

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

kvch
kvch previously approved these changes Feb 15, 2022
@kvch kvch dismissed their stale review February 15, 2022 10:30

Backwards compatibility

Copy link
Contributor

@kvch kvch left a comment

Choose a reason for hiding this comment

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

First, I accidentally approved the PR. The code looks good to me, and this should help with position tracking going forward.

My concern is given that IDs are used for finding state information in the registry file, changing an ID of an input means that Filebeat will not find the existing states. Thus, the files for existing inputs will be resent.

I do think passing down the IDs are needed otherwise we cannot support reading from the same file from multiple inputs. We just have to be careful how we roll it out.

@ph
Copy link
Contributor Author

ph commented Feb 15, 2022

@kvch Yes, this is a valid concern, I don't think this is something I can enforce by any means, I would prefer we fix it on the filestream. This is sadly the best I can do on my side. In older log input we were using the file inode as the main identification of the files, with the downside of possible inode reuse on the mounted drive. I don't what is different in the filestream implementation.

Concerning the ID from Fleet, I would expect the ID to be consistent, I would assume that when a user add an integration to an agent policy and it edits the configuration the ID will stay the same until the integration is removed from a policy and added again. @joshdover or @nchaulet can you clarify the stability of the id for input in fleet?

@ph
Copy link
Contributor Author

ph commented Feb 15, 2022

@kvch Would you consider the stability of the ID and passing the ID two different issues? In that case, you should test/review/approve this pull request.

Comment on lines 24 to +27
- copy_all_to_list:
to: streams
on_conflict: noop
except: ["streams", "id", "enabled", "processors"]
except: ["streams", "enabled", "processors"]
Copy link
Member

Choose a reason for hiding this comment

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

I imagine that there is a test that checks that all the fields, except these are passes to filebeat. Am I correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is exactly that. If you look at the golden files in this PR, you see how an agent policy is transformed into a filebeat policy.

@blakerouse
Copy link
Contributor

@ph @kvch I think the open question here is about backwards compatibility. What will Filebeat do? If Filebeat would take the inode as the main id first then merge the id passed into the configuration, that would allow an ID to be added to an input after the fact.

@ph
Copy link
Contributor Author

ph commented Feb 15, 2022

@blakerouse Yes, I think this is an interesting question, I don't know exactly the changes that went into the filestream input with the disambiguation.

@ph ph changed the title Propage input id from the Agent policy into Filebeat configuration Propagate input id from the Agent policy into Filebeat configuration Feb 15, 2022
@mergify
Copy link
Contributor

mergify bot commented Feb 15, 2022

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b propagate-id-from-agent-to-filebeat upstream/propagate-id-from-agent-to-filebeat
git merge upstream/main
git push upstream propagate-id-from-agent-to-filebeat

@kvch
Copy link
Contributor

kvch commented Feb 15, 2022

I think the open question here is about backwards compatibility. What will Filebeat do? If Filebeat would take the inode as the main id first then merge the id passed into the configuration, that would allow an ID to be added to an input after the fact.

At the moment there is no such support. But we can implement some heuristics for backwards compatibility. We are also considering forcing users to set IDs in Filebeat. If we decide to do it we have to migrate old state IDs to the new ones. I have to sleep on it. :D But it is something that should be resolved in Filebeat, not in Agent. That is for sure.

@kvch
Copy link
Contributor

kvch commented Feb 16, 2022

We decided to enforce IDs for filestream inputs. We will take care of migrating existing entries in the registry. So adding this change to Agent should be safe. Once that is merged, I will approve this PR.

@kvch
Copy link
Contributor

kvch commented Feb 16, 2022

Please add a changelog entry.

@ph
Copy link
Contributor Author

ph commented Feb 16, 2022

I just learned that id can be the same for different inputs, this is a bummer.

@ph
Copy link
Contributor Author

ph commented Feb 17, 2022

rebased waiting on ci.

@ph
Copy link
Contributor Author

ph commented Feb 22, 2022 via email

@ph ph changed the base branch from main to 8.1 March 10, 2022 14:06
@ph ph requested review from a team as code owners March 10, 2022 14:06
@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2022

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b propagate-id-from-agent-to-filebeat upstream/propagate-id-from-agent-to-filebeat
git merge upstream/8.1
git push upstream propagate-id-from-agent-to-filebeat

@ph ph changed the base branch from 8.1 to main March 10, 2022 14:07
@v1v v1v removed the request for review from a team March 10, 2022 14:39
@ph
Copy link
Contributor Author

ph commented Mar 10, 2022

done in elastic/elastic-agent#196 in the Elastic Agent repository for 8.2

@ph
Copy link
Contributor Author

ph commented Mar 14, 2022

@mergify.io update

ph added 2 commits March 14, 2022 14:46
This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: elastic#30300
@ph ph force-pushed the propagate-id-from-agent-to-filebeat branch from 5a7b924 to 4aa7e62 Compare March 14, 2022 18:46
@jlind23
Copy link
Collaborator

jlind23 commented Mar 16, 2022

/package

@ph
Copy link
Contributor Author

ph commented Mar 16, 2022

I am going to merge this, I need to investigate the failures, looking at the test this seems to be an issue with the way test are bootstrapped.

@ph ph added the backport-v7.17.0 Automated backport with mergify label Mar 16, 2022
@ph ph merged commit b318f6e into elastic:main Mar 16, 2022
mergify bot pushed a commit that referenced this pull request Mar 16, 2022
…30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)

# Conflicts:
#	x-pack/elastic-agent/pkg/agent/program/supported.go
mergify bot pushed a commit that referenced this pull request Mar 16, 2022
…30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)
mergify bot pushed a commit that referenced this pull request Mar 16, 2022
…30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)
ph added a commit that referenced this pull request Mar 17, 2022
… Filebeat configuration (#30862)

* Propagate input id from the Agent policy into Filebeat configuration (#30386)


This allows an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propagated down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will be sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)

Co-authored-by: Pier-Hugues Pellerin <phpellerin@gmail.com>
ph added a commit that referenced this pull request Mar 17, 2022
…30386) (#30863)

This allows an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propagated down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will be sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)

Co-authored-by: Pier-Hugues Pellerin <phpellerin@gmail.com>
ph added a commit that referenced this pull request Mar 17, 2022
…30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)
ph added a commit that referenced this pull request Mar 18, 2022
…Filebeat configuration (#30864)

* Propagate input id from the Agent policy into Filebeat configuration (#30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
(cherry picked from commit b318f6e)

* changelog

Co-authored-by: Pier-Hugues Pellerin <phpellerin@gmail.com>
kush-elastic pushed a commit to kush-elastic/beats that referenced this pull request May 2, 2022
…lastic#30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: elastic#30300
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…lastic#30386) (elastic#30863)

This allows an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propagated down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will be sent to
Filebeat.

Fixes: elastic#30300
(cherry picked from commit 24f6449)

Co-authored-by: Pier-Hugues Pellerin <phpellerin@gmail.com>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
…30386)

* Propage input id from the Agent policy into Filebeat configuration

This allow an ID defined on the Agent Policy for any inputs supported by
Filebeat to be propaged down to the Filebeat configuration.

Note: No validation is done on the field, if the field is present it will sent to
Filebeat.

Fixes: #30300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.17.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify backport-v8.1.0 Automated backport with mergify backport-v8.2.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Elastic Agent does not pass down input id to filestream
7 participants