[beatreceiver] Add e2e test to compare docs from redis and mqtt input#51565
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@filebeat/input/redis/testutil/testutil.go`:
- Around line 92-109: The emission loop in testutil’s goroutine uses
require.NoError for both conn.Close and conn.Do, which is unsafe because require
must not run in a background goroutine. Update the goroutine around conn.Get/Do
and ticker handling to send any error back to the main test via a channel or
shared error value, then perform the require/assertion in the parent test
function after the goroutine completes.
🪄 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: Enterprise
Run ID: fd9f2c9f-6add-4fef-b034-622547e13daf
📒 Files selected for processing (6)
filebeat/input/redis/redis_integration_test.gofilebeat/input/redis/testutil/testutil.gox-pack/filebeat/tests/integration/otel_cel_test.gox-pack/filebeat/tests/integration/otel_filebeat_input_test.gox-pack/filebeat/tests/integration/otel_test.gox-pack/otel/oteltestcol/collector.go
💤 Files with no reviewable changes (2)
- x-pack/filebeat/tests/integration/otel_cel_test.go
- x-pack/filebeat/tests/integration/otel_test.go
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@x-pack/filebeat/docker-compose.yml`:
- Line 12: Add a healthcheck to the new redis service in the docker-compose
setup, or change proxy_dep so it no longer depends on redis with
service_healthy. The unsatisfied dependency is introduced by the redis entry and
the proxy_dep service relationship, so update whichever of those is intended to
coordinate startup. Keep the compose dependency consistent by ensuring the
referenced service actually exposes a health status before using
service_healthy.
🪄 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: Enterprise
Run ID: a1ca60e5-6769-4e8e-b520-da7e30d5ee76
📒 Files selected for processing (1)
x-pack/filebeat/docker-compose.yml
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
x-pack/filebeat/docker-compose.yml (1)
13-13: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
mosquittois gated onservice_healthybut no healthcheck is defined.
proxy_depnow waits formosquittoto become healthy, but the newmosquittoservice only exposesbuildandports, and the suppliedtesting/environments/docker/mosquitto/Dockerfile:1-3snippet does not define an imageHEALTHCHECK. That makes this dependency unsatisfiable and can stall the integration stack before tests start.Also applies to: 119-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@x-pack/filebeat/docker-compose.yml` at line 13, The mosquitto dependency in the docker-compose setup is using service_healthy without any healthcheck to satisfy it. Add a healthcheck for the mosquitto service itself (or change the dependency in proxy_dep to a condition that matches the available startup signal) so the compose graph can resolve readiness; use the mosquitto service definition and proxy_dep dependency entries to locate the affected config, and ensure the testing/environments/docker/mosquitto Dockerfile or compose service provides the needed health status.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@filebeat/input/mqtt/testutil/testutil.go`:
- Around line 65-95: The publish assertions in PublishMessage and EmitMessages
are running inside a background goroutine, so failures won’t reliably fail the
test from the main goroutine. Refactor EmitMessages (and the related publisher
goroutine usage in mqtt_integration_test.go) to return publish errors or send
them back to the test goroutine, then perform the require/assert checks there;
keep the publishing loop in the goroutine but move the require.True/NoError/Fail
calls out of it.
---
Duplicate comments:
In `@x-pack/filebeat/docker-compose.yml`:
- Line 13: The mosquitto dependency in the docker-compose setup is using
service_healthy without any healthcheck to satisfy it. Add a healthcheck for the
mosquitto service itself (or change the dependency in proxy_dep to a condition
that matches the available startup signal) so the compose graph can resolve
readiness; use the mosquitto service definition and proxy_dep dependency entries
to locate the affected config, and ensure the
testing/environments/docker/mosquitto Dockerfile or compose service provides the
needed health status.
🪄 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: Enterprise
Run ID: 3bcf3255-d7d6-4404-af37-eda7bb67f60b
📒 Files selected for processing (4)
filebeat/input/mqtt/mqtt_integration_test.gofilebeat/input/mqtt/testutil/testutil.gox-pack/filebeat/docker-compose.ymlx-pack/filebeat/tests/integration/otel_filebeat_input_test.go
belimawr
left a comment
There was a problem hiding this comment.
This PR is quite large, the refactoring/moving tests around could have been done in a different PR to make reviewing easier. Could you split it into two PRs?
Done #51699 |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
TL;DRBoth failed x-pack Filebeat integration jobs are failing at Go test compile time because Remediation
Investigation detailsRoot CauseThis is a code/test compile failure introduced by adding a second package-level helper with the same name. The PR-head file func deleteDataStreamsFromES(t *testing.T, es *elasticsearch.Client, dataStreams []string) {
_, err := es.Indices.DeleteDataStream(dataStreams)
require.NoError(t, err, "failed to delete data streams")
}But Evidence
VerificationNot run locally: the checkout available to this workflow is What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Proposed commit message
This PR adds e2e test that compares document ingested by redis and mqtt input in standalone and otel mode.
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
None
How to test this PR locally
Related issues