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

Emit vars even if provider data is empty from the start #6598

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

swiatekm
Copy link
Contributor

What does this PR do?

We only signal from providers if the emitted data is different from the current state. This caused a bug where the data wasn't ever emitted if it was empty. The provider controller now distinguishes between no data (yet) and empty data. As a result, it's again possible to effectively disable all providers by using a local provider with no data.

This is a regression caused by #6114. It doesn't apply to the main and 8.x branches due to #6169 refactoring provider initialization in those.

Why is it important?

Fixes a regression where agent wouldn't generate any configuration if the only enabled context providers only emitted empty data. This is something users do if they want to effectively disable providers.

Checklist

  • I have read and understood the pull request guidelines of this project.
  • 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 added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool

Related issues

We only signal from providers if the emitted data is different from the
current state. This
caused a bug where the data wasn't ever emitted if it was empty. The
provider controller now distinguishes between no data (yet) and empty
data. As a result, it's again possible to effectively disable all
providers by using a local provider with no data.

This is a regression caused by
#6114. It doesn't apply to
the main and 8.x branches due to
#6169 refactoring provider
initialisation in those.
@swiatekm swiatekm requested a review from blakerouse January 24, 2025 16:31
@swiatekm swiatekm marked this pull request as ready for review January 24, 2025 16:42
@swiatekm swiatekm requested a review from a team as a code owner January 24, 2025 16:42
@swiatekm swiatekm requested review from kaanyalti and removed request for a team January 24, 2025 16:42
@swiatekm swiatekm added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team backport-8.16 Automated backport with mergify labels Jan 24, 2025
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Nice catch. Looks good.

@swiatekm
Copy link
Contributor Author

Nice catch. Looks good.

On a related note, should we make it possible to run agent with all providers disabled? I think #6169 effectively achieves that, but I'm not sure if intentionally or not.

@swiatekm swiatekm merged commit 8ea9439 into 8.17 Jan 27, 2025
17 checks passed
@swiatekm swiatekm deleted the fix/817-context-provider-init branch January 27, 2025 11:17
mergify bot pushed a commit that referenced this pull request Jan 27, 2025
We only signal from providers if the emitted data is different from the
current state. This
caused a bug where the data wasn't ever emitted if it was empty. The
provider controller now distinguishes between no data (yet) and empty
data. As a result, it's again possible to effectively disable all
providers by using a local provider with no data.

This is a regression caused by
#6114. It doesn't apply to
the main and 8.x branches due to
#6169 refactoring provider
initialisation in those.

(cherry picked from commit 8ea9439)
@blakerouse
Copy link
Contributor

@swiatekm it was intentional, a requirement of not having any variables.

swiatekm added a commit that referenced this pull request Jan 27, 2025
We only signal from providers if the emitted data is different from the
current state. This
caused a bug where the data wasn't ever emitted if it was empty. The
provider controller now distinguishes between no data (yet) and empty
data. As a result, it's again possible to effectively disable all
providers by using a local provider with no data.

This is a regression caused by
#6114. It doesn't apply to
the main and 8.x branches due to
#6169 refactoring provider
initialisation in those.

(cherry picked from commit 8ea9439)

Co-authored-by: Mikołaj Świątek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.16 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.

4 participants