x-pack/filebeat/input/entityanalytics: add entcollect adapter layer#49871
Merged
Conversation
efd6
added
enhancement
Filebeat
botelastic
Bot
added
needs_team
Contributor
🤖 GitHub commentsJust comment with:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a minimal-state mode to the entityanalytics input and updates its Plugin signature to accept a shared statestore.States. Configuration gains ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@x-pack/filebeat/input/entityanalytics/minimal_test.go`:
- Line 96: The test creates a logger with logp.NewLogger (log variable) which
the linter forbids; change those usages to use the global logger logp.L() or
refactor the test functions to accept a *logp.Logger parameter and pass it in
from the caller. Update each occurrence referenced (the local variable named log
in minimal_test.go at the spots matching the original new-logger calls) to
either assign log := logp.L() or alter the test signatures to take a
*logp.Logger and use that parameter instead, and apply the same change at the
other occurrences reported (lines around 178, 251, 264).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b4b42cd-8028-4385-829e-d6fb50135901
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
NOTICE.txtchangelog/fragments/1775095457-entity-analytics-entcollect-adapter.yamldev-tools/notice/overrides.jsongo.modx-pack/filebeat/input/default-inputs/inputs_aix.gox-pack/filebeat/input/default-inputs/inputs_darwin.gox-pack/filebeat/input/default-inputs/inputs_other.gox-pack/filebeat/input/default-inputs/inputs_windows.gox-pack/filebeat/input/entityanalytics/conf.gox-pack/filebeat/input/entityanalytics/conf_test.gox-pack/filebeat/input/entityanalytics/input.gox-pack/filebeat/input/entityanalytics/input_test.gox-pack/filebeat/input/entityanalytics/internal/kvstore/entcollect.gox-pack/filebeat/input/entityanalytics/internal/kvstore/entcollect_test.gox-pack/filebeat/input/entityanalytics/internal/kvstore/publish.gox-pack/filebeat/input/entityanalytics/internal/kvstore/publish_test.gox-pack/filebeat/input/entityanalytics/minimal.gox-pack/filebeat/input/entityanalytics/minimal_test.gox-pack/filebeat/input/entityanalytics/provider/minimal.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@x-pack/filebeat/input/entityanalytics/conf.go`:
- Around line 24-29: The validation branch that checks c.UseMinimalState calls
provider.HasMinimalStateProvider and immediately returns ErrProviderUnknown
because the minimal-state provider registry is empty; fix by either registering
at least one minimal-state provider in the provider registry (so
HasMinimalStateProvider returns true for supported c.Provider) or remove/guard
the UseMinimalState path until providers are added — update the registry
initialization where providers are registered and/or adjust the validation in
the function containing UseMinimalState to skip the HasMinimalStateProvider
check when no minimal providers exist, ensuring ErrProviderUnknown is not
returned for valid configs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d348c477-63d0-4b79-99c5-d837d2e1c1a0
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
NOTICE.txtchangelog/fragments/1775095457-entity-analytics-entcollect-adapter.yamldev-tools/notice/overrides.jsongo.modx-pack/filebeat/input/default-inputs/inputs_aix.gox-pack/filebeat/input/default-inputs/inputs_darwin.gox-pack/filebeat/input/default-inputs/inputs_other.gox-pack/filebeat/input/default-inputs/inputs_windows.gox-pack/filebeat/input/entityanalytics/conf.gox-pack/filebeat/input/entityanalytics/conf_test.gox-pack/filebeat/input/entityanalytics/input.gox-pack/filebeat/input/entityanalytics/input_test.gox-pack/filebeat/input/entityanalytics/internal/kvstore/entcollect.gox-pack/filebeat/input/entityanalytics/internal/kvstore/entcollect_test.gox-pack/filebeat/input/entityanalytics/internal/kvstore/publish.gox-pack/filebeat/input/entityanalytics/internal/kvstore/publish_test.gox-pack/filebeat/input/entityanalytics/minimal.gox-pack/filebeat/input/entityanalytics/minimal_test.gox-pack/filebeat/input/entityanalytics/provider/minimal.go
✅ Files skipped from review due to trivial changes (9)
- x-pack/filebeat/input/default-inputs/inputs_windows.go
- changelog/fragments/1775095457-entity-analytics-entcollect-adapter.yaml
- x-pack/filebeat/input/entityanalytics/conf_test.go
- go.mod
- x-pack/filebeat/input/entityanalytics/provider/minimal.go
- x-pack/filebeat/input/entityanalytics/minimal.go
- x-pack/filebeat/input/entityanalytics/internal/kvstore/entcollect_test.go
- x-pack/filebeat/input/entityanalytics/minimal_test.go
- x-pack/filebeat/input/entityanalytics/input_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- x-pack/filebeat/input/default-inputs/inputs_aix.go
- x-pack/filebeat/input/default-inputs/inputs_other.go
- x-pack/filebeat/input/default-inputs/inputs_darwin.go
- NOTICE.txt
- x-pack/filebeat/input/entityanalytics/internal/kvstore/publish_test.go
- x-pack/filebeat/input/entityanalytics/input.go
orestisfl
reviewed
Apr 8, 2026
| tmpDir := t.TempDir() | ||
| dbFile := filepath.Join(tmpDir, "test.db") | ||
|
|
||
| log := logp.L() |
Contributor
There was a problem hiding this comment.
The linter is correct, we want to avoid introducing any new global loggers including tests. Please use logptest.NewTestingLogger.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@x-pack/otel/extension/elasticsearchstorage/entcollect.go`:
- Around line 48-57: The current entcollectStore.Delete implementation is
TOCTOU-prone because it calls s.base.Has(key) then s.base.Remove(key); remove
the pre-check and call s.base.Remove(key) directly, and map the specific
sentinel es.ErrKeyUnknown returned by s.base.Remove to a nil (treat “not found”
as success) while returning any other error unchanged; reference
entcollectStore.Delete, s.base.Remove, and es.ErrKeyUnknown when making this
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbdbe5e5-41c1-4867-b3d1-aac8c1c9050c
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
NOTICE.txtchangelog/fragments/1775685154-add-entcollect-store-adapter.yamlgo.modx-pack/otel/extension/elasticsearchstorage/entcollect.go
✅ Files skipped from review due to trivial changes (2)
- changelog/fragments/1775685154-add-entcollect-store-adapter.yaml
- NOTICE.txt
andrewkroh
reviewed
Apr 10, 2026
andrewkroh
approved these changes
Apr 15, 2026
pierrehilbert
added
the
Team:Elastic-Agent-Data-Plane
Contributor
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
TL;DRThe failing Buildkite job is a test failure in Remediation
Investigation detailsRoot CauseThe failure is not a compile/build break; it is runtime test teardown on Windows:
Relevant code paths:
Evidence
Verification
So this needs verification in the Windows CI environment where the failure occurs. Follow-upIf this still flakes after isolating per-subtest paths, add an assertion/poll in test teardown for state DB unlock (Windows-specific timing), then rerun the same Buildkite pipeline. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | noneWhat is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
mauri870
reviewed
Apr 24, 2026
| func TestEntcollectStore_SetGet(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| store := testSetupStore(t, "TestEntcollectStore_SetGet.db") |
Member
There was a problem hiding this comment.
Suggested change
| store := testSetupStore(t, "TestEntcollectStore_SetGet.db") | |
| store := testSetupStore(t, t.Name()+".db") |
mauri870
reviewed
Apr 24, 2026
| func TestEntcollectStore_GetMissing(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| store := testSetupStore(t, "TestEntcollectStore_GetMissing.db") |
Member
There was a problem hiding this comment.
Suggested change
| store := testSetupStore(t, "TestEntcollectStore_GetMissing.db") | |
| store := testSetupStore(t, t.Name()+".db") |
mauri870
reviewed
Apr 24, 2026
| func TestEntcollectStore_Delete(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| store := testSetupStore(t, "TestEntcollectStore_Delete.db") |
Member
There was a problem hiding this comment.
Suggested change
| store := testSetupStore(t, "TestEntcollectStore_Delete.db") | |
| store := testSetupStore(t, t.Name()+".db") |
mauri870
reviewed
Apr 24, 2026
| }, | ||
| } | ||
|
|
||
| if err := pub(context.Background(), doc); err != nil { |
Member
There was a problem hiding this comment.
Suggested change
| if err := pub(context.Background(), doc); err != nil { | |
| if err := pub(t.Context(), doc); err != nil { |
mauri870
approved these changes
Apr 24, 2026
mauri870
left a comment
mauri870
left a comment
Member
There was a problem hiding this comment.
LGTM. Left some nit comments regarding test names and context.
Wire the entcollect shared library into the entity-analytics input behind a new use_minimal_state config flag. When the flag is false (the default), the existing kvstore-based provider path is unchanged. The adapter layer consists of: - Plugin signature extended with statestore.States, matching the CEL/httpjson pattern and positioning for ES-backed state later. - EntcollectStore wrapping kvstore.Transaction as entcollect.Store with error translation and a per-provider bucket namespace. - Publisher closure converting entcollect.Document to beat.Event with TxTracker.Add before each Publish for ACK-before-commit. - MinimalStateProviderFactory registry for entcollect providers (empty for now; sessions 2-5 populate it). - minimalStateInput implementing the generic sync loop: timer, buffer, ACK-then-commit, and buffer discard on error. - slog bridge from logp.Logger via zapslog.
The OTel entity-analytics receiver (in opentelemetry-collector- components) needs to obtain an entcollect.Store from the elasticsearchstorage extension without importing beats types. Add a Store method on elasticStorage that satisfies entcollect.Registry, returning an adapter that translates between backend.Store and entcollect.Store (error sentinels, method names, Each callback shape). The receiver will resolve the extension via host.GetExtensions() and type-assert to entcollect.Registry. Both sides depend only on the stdlib-only entcollect module; neither imports the other. The entcollect dependency uses a local replace directive until the module gets a tagged release. Updates elastic#49160
efd6
added a commit
to efd6/beats
that referenced
this pull request
May 1, 2026
…inimal-state registry Register the entcollect Jamf provider under the "jamf" name so that the entity-analytics input can use it in minimal-state mode (use_minimal_state: true). The registration mirrors the pattern established by the beats shim merged in elastic#49871. A local UCF-tagged struct (localConf) is defined inside minimalProvider to bridge the ucfg config dialect (config:"" tags) to ecjamf.Config, which uses json:"" tags for cross-runtime portability. Fields are mapped explicitly so that a mismatch is a compile error rather than a silent zero. TestMinimalConfigRoundTrip guards against future drift: if ecjamf.Config gains a new field the field-count assertion fails, forcing localConf and the test to be updated together. Assisted-By: Cursor (claude-opus-4-6)
efd6
added a commit
that referenced
this pull request
May 3, 2026
…inimal-state registry (#50445) Register the entcollect Jamf provider under the "jamf" name so that the entity-analytics input can use it in minimal-state mode (use_minimal_state: true). The registration mirrors the pattern established by the beats shim merged in #49871. A local UCF-tagged struct (localConf) is defined inside minimalProvider to bridge the ucfg config dialect (config:"" tags) to ecjamf.Config, which uses json:"" tags for cross-runtime portability. Fields are mapped explicitly so that a mismatch is a compile error rather than a silent zero. TestMinimalConfigRoundTrip guards against future drift: if ecjamf.Config gains a new field the field-count assertion fails, forcing localConf and the test to be updated together. Assisted-By: Cursor (claude-opus-4-6)
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 5, 2026
…lastic#49871) Wire the entcollect shared library into the entity-analytics input behind a new use_minimal_state config flag. When the flag is false (the default), the existing kvstore-based provider path is unchanged. The Beats adapter layer consists of: - Plugin signature extended with statestore.States, matching the CEL/httpjson pattern and positioning for ES-backed state later. - EntcollectStore wrapping kvstore.Transaction as entcollect.Store with error translation and a per-provider bucket namespace. - Publisher closure converting entcollect.Document to beat.Event with TxTracker.Add before each Publish for ACK-before-commit. - MinimalStateProviderFactory registry for entcollect providers (empty for now; later sessions populate it). - minimalStateInput implementing the generic sync loop: timer, buffer, ACK-then-commit, and buffer discard on error. - slog bridge from logp.Logger via zapslog. The OTel adapter adds an entcollect.Registry implementation to the elasticsearchstorage extension. The receiver (in opentelemetry- collector-components) will resolve the extension via host.GetExtensions() and type-assert to entcollect.Registry. Both sides depend only on the stdlib-only entcollect module; neither imports the other. The adapter translates between backend.Store and entcollect.Store (error sentinels, method names, Each callback shape).
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 5, 2026
…inimal-state registry (elastic#50445) Register the entcollect Jamf provider under the "jamf" name so that the entity-analytics input can use it in minimal-state mode (use_minimal_state: true). The registration mirrors the pattern established by the beats shim merged in elastic#49871. A local UCF-tagged struct (localConf) is defined inside minimalProvider to bridge the ucfg config dialect (config:"" tags) to ecjamf.Config, which uses json:"" tags for cross-runtime portability. Fields are mapped explicitly so that a mismatch is a compile error rather than a silent zero. TestMinimalConfigRoundTrip guards against future drift: if ecjamf.Config gains a new field the field-count assertion fails, forcing localConf and the test to be updated together. Assisted-By: Cursor (claude-opus-4-6)
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 6, 2026
…lastic#49871) Wire the entcollect shared library into the entity-analytics input behind a new use_minimal_state config flag. When the flag is false (the default), the existing kvstore-based provider path is unchanged. The Beats adapter layer consists of: - Plugin signature extended with statestore.States, matching the CEL/httpjson pattern and positioning for ES-backed state later. - EntcollectStore wrapping kvstore.Transaction as entcollect.Store with error translation and a per-provider bucket namespace. - Publisher closure converting entcollect.Document to beat.Event with TxTracker.Add before each Publish for ACK-before-commit. - MinimalStateProviderFactory registry for entcollect providers (empty for now; later sessions populate it). - minimalStateInput implementing the generic sync loop: timer, buffer, ACK-then-commit, and buffer discard on error. - slog bridge from logp.Logger via zapslog. The OTel adapter adds an entcollect.Registry implementation to the elasticsearchstorage extension. The receiver (in opentelemetry- collector-components) will resolve the extension via host.GetExtensions() and type-assert to entcollect.Registry. Both sides depend only on the stdlib-only entcollect module; neither imports the other. The adapter translates between backend.Store and entcollect.Store (error sentinels, method names, Each callback shape).
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 6, 2026
…inimal-state registry (elastic#50445) Register the entcollect Jamf provider under the "jamf" name so that the entity-analytics input can use it in minimal-state mode (use_minimal_state: true). The registration mirrors the pattern established by the beats shim merged in elastic#49871. A local UCF-tagged struct (localConf) is defined inside minimalProvider to bridge the ucfg config dialect (config:"" tags) to ecjamf.Config, which uses json:"" tags for cross-runtime portability. Fields are mapped explicitly so that a mismatch is a compile error rather than a silent zero. TestMinimalConfigRoundTrip guards against future drift: if ecjamf.Config gains a new field the field-count assertion fails, forcing localConf and the test to be updated together. Assisted-By: Cursor (claude-opus-4-6)
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 6, 2026
…lastic#49871) Wire the entcollect shared library into the entity-analytics input behind a new use_minimal_state config flag. When the flag is false (the default), the existing kvstore-based provider path is unchanged. The Beats adapter layer consists of: - Plugin signature extended with statestore.States, matching the CEL/httpjson pattern and positioning for ES-backed state later. - EntcollectStore wrapping kvstore.Transaction as entcollect.Store with error translation and a per-provider bucket namespace. - Publisher closure converting entcollect.Document to beat.Event with TxTracker.Add before each Publish for ACK-before-commit. - MinimalStateProviderFactory registry for entcollect providers (empty for now; later sessions populate it). - minimalStateInput implementing the generic sync loop: timer, buffer, ACK-then-commit, and buffer discard on error. - slog bridge from logp.Logger via zapslog. The OTel adapter adds an entcollect.Registry implementation to the elasticsearchstorage extension. The receiver (in opentelemetry- collector-components) will resolve the extension via host.GetExtensions() and type-assert to entcollect.Registry. Both sides depend only on the stdlib-only entcollect module; neither imports the other. The adapter translates between backend.Store and entcollect.Store (error sentinels, method names, Each callback shape).
brian-mckinney
pushed a commit
to brian-mckinney/beats
that referenced
this pull request
May 6, 2026
…inimal-state registry (elastic#50445) Register the entcollect Jamf provider under the "jamf" name so that the entity-analytics input can use it in minimal-state mode (use_minimal_state: true). The registration mirrors the pattern established by the beats shim merged in elastic#49871. A local UCF-tagged struct (localConf) is defined inside minimalProvider to bridge the ucfg config dialect (config:"" tags) to ecjamf.Config, which uses json:"" tags for cross-runtime portability. Fields are mapped explicitly so that a mismatch is a compile error rather than a silent zero. TestMinimalConfigRoundTrip guards against future drift: if ecjamf.Config gains a new field the field-count assertion fails, forcing localConf and the test to be updated together. Assisted-By: Cursor (claude-opus-4-6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed commit message
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
How to test this PR locally
Related issues
Use cases
Screenshots
Logs