Fixed monitoring filebeat and metricbeat not connecting to Agent over GRPC

[michalpristas]

What does this PR do?

In PR #23736 there was a populate method removed which in case spec is not complete reloaded it from the spec file.

Spec was indeed in some cases incomplete it depends where the request came from. If it was normal one from emitter it was complete and beat was stable.

If it was from monitoring injector it was just a naked spec with only cmdName to run, this one was populated, in a removed populate func.
Having naked spec means no args are passed into the beat and beat does not know it is agent-managed, so it wont even try to connect back. It just runs with the default configuration until agent restarts it.

This change goes from generating naked spec file to loading it from spec definition at the time of injecting monitoring. which means only complete specs comes to the place where populate func was before and correct Args are passed to process.

Why is it important?

Fixes #23833

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.

[elasticmachine]

Pinging @elastic/ingest-management (Team:Ingest Management)

[elasticmachine]

Pinging @elastic/agent (Team:Agent)

[elasticmachine]

💚 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

• Build Cause: Pull request #23843 event

• • Start Time: 2021-02-04T09:22:49.471+0000

• Duration: 28 min 16 sec

• Commit: 59e651b

Test stats 🧪

Test	Results
Failed	0
Passed	5846
Skipped	16
Total	5862

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	5846
Skipped	16
Total	5862

[michalpristas]

/packaging

[michalpristas]

/package

[ph]

@michalpristas Could we add a unit to cover us from regression?

[blakerouse]

@michalpristas Thanks for sorting this out. My bad!

I agree with @ph a unit test would be great for this case, so the test will ensure any future changes will not break it.

[EricDavisX]

The unit test is certainly closer to the code and faster to find and review, and is never skipped I suppose. But we did have a test that covered it, which is nice.

What I find interesting is that the nightly test caught it, when it could (I think) have been caught in the PR CI test run of /package - I think we should restart the discussion on having those run automatically for all changes that include to Libbeat, Filebeat, Metricbeat and Agent - @mdelapenya @ph any thoughts on priority of that?

[mdelapenya]

The unit test is certainly closer to the code and faster to find and review, and is never skipped I suppose. But we did have a test that covered it, which is nice.

What I find interesting is that the nightly test caught it, when it could (I think) have been caught in the PR CI test run of /package - I think we should restart the discussion on having those run automatically for all changes that include to Libbeat, Filebeat, Metricbeat and Agent - @mdelapenya @ph any thoughts on priority of that?

I'd traced back what happened on CI here: #23833 (comment)

[ph]

@michalpristas will you followup with a unit test?

[ph]

@EricDavisX Maybe the trigger would be only libbeat and Agent, that should cover 90% of the use case?

[EricDavisX]

@EricDavisX Maybe the trigger would be only libbeat and Agent, that should cover 90% of the use case?

I would be fine with that - if it saves us a lot from the other Beats side, with most of the coverage we want