Autodiscover: pass paths through builders instead of global paths#49537
Autodiscover: pass paths through builders instead of global paths#49537orestisfl merged 13 commits intoelastic:mainfrom
Conversation
🤖 GitHub commentsJust comment with:
|
Pass *paths.Path from NewAutodiscover through BuildProvider and each provider's AutodiscoverBuilder into NewBuilders, replacing the previous nil/global approach. Also fixes a bug where the package name `paths` was passed instead of the `path` parameter variable.
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis change threads a 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
|
My only concern is if we have tested this in case of an upgrade from a previous version where the hints.paths dont exist. |
We synced with Orestis, my comment was not accurate as this paths parameter refers to internal hints builder configuration. |
TL;DRBoth failed jobs are blocked by the same transient infrastructure error while provisioning the ECH test deployment: Remediation
Investigation detailsRoot CauseThe failures occur before beat test logic runs, during Terraform provisioning of the ECH deployment at Evidence
Verification
Follow-upIf this reproduces after retries, correlate those Note 🔒 Integrity filtering filtered 1 itemIntegrity filtering activated and filtered the following item during workflow execution.
What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
…9537) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30)
…9537) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30)
|
@Mergifyio backport 8.19 |
✅ Backports have been createdDetails
Cherry-pick of 31d1c30 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
…9537) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30) # Conflicts: # filebeat/autodiscover/builder/hints/logs_test.go # libbeat/autodiscover/autodiscover_test.go
…9537) (#50023) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30) Co-authored-by: Orestis Floros <orestis.floros@elastic.co>
…9537) (#50024) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30) Co-authored-by: Orestis Floros <orestis.floros@elastic.co>
…tead of global paths (#50030) * Autodiscover: pass paths through builders instead of global paths (#49537) Thread an explicit `*paths.Path` parameter through the autodiscover subsystem (`NewAutodiscover` → `BuildProvider` → `AutodiscoverBuilder` → `NewBuilders` → `BuildBuilder` → `BuilderConstructor`) instead of relying on the global `paths.Paths` variable. Also makes `NewLogHints` unexported (`newLogHints`) since it is only used via the autodiscover registry. (cherry picked from commit 31d1c30) # Conflicts: # filebeat/autodiscover/builder/hints/logs_test.go # libbeat/autodiscover/autodiscover_test.go * Fix conflicts --------- Co-authored-by: Orestis Floros <orestis.floros@elastic.co>
Proposed commit message
Thread an explicit
*paths.Pathparameter through the autodiscover subsystem (NewAutodiscover→BuildProvider→AutodiscoverBuilder→NewBuilders→BuildBuilder→BuilderConstructor) instead of relying on the globalpaths.Pathsvariable.Also makes
NewLogHintsunexported (newLogHints) since it is only used via the autodiscover registry.Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesstresstest.shscript to run them under stress conditions and race detector to verify their stability.I have added an entry in./changelog/fragmentsusing the changelog tool.How to test this PR locally
Create a
filebeat.ymlwith hints-based autodiscover:Run filebeat and start a container with hint labels:
./filebeat -e -c filebeat.yml # In another terminal: docker run --label co.elastic.logs/module=nginx --rm nginxRelated issues