-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Discover] Add context awareness telemetry tests for Observability profiles #201310
base: main
Are you sure you want to change the base?
Conversation
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#7468[✅] x-pack/test_serverless/functional/test_suites/observability/config.ts: 25/25 tests passed. |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
`--plugin-path=${resolve( | ||
__dirname, | ||
'../../../test/analytics/plugins/analytics_ftr_helpers' | ||
)}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand what this plugin is doing?
Asking because if this is something-test-only where the behavior is different to what we have in a real serverless environment, then tests based on this would fail when running against MKI. In this case, this config entry should move to a separate config (see e.g. here how it's done for the example plugins), which doesn't run against MKI.
If it simulates MKI behavior locally, then it can stay. Would probably be good to add a comment explaining it in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elastic/kibana-core could confirm for sure since they maintain the service, but I'm pretty sure this does alter the behaviour because it adds a custom analytics shipper to capture EBT events in tests. I added a separate config for the telemetry tests here: 67a5954.
I also wasn't really sure how to handle the custom services to make sure they aren't available where they won't work, so I tried to do something similar to what was done for API integration tests: 38b9306. Let me know if you think there's a better way to do this and I can update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.buildkite/ftr_oblt_serverless_configs.yml
💛 Build succeeded, but was flaky
Failed CI StepsTest FailuresMetrics [docs]
History
cc @davismcphee |
@@ -1131,6 +1131,7 @@ x-pack/test_serverless/api_integration/test_suites/common/platform_security @ela | |||
/x-pack/test_serverless/functional/test_suites/common/examples/unified_field_list_examples @elastic/kibana-data-discovery | |||
/x-pack/test_serverless/functional/test_suites/common/management/data_views @elastic/kibana-data-discovery | |||
src/plugins/discover/public/context_awareness/profile_providers/security @elastic/kibana-data-discovery @elastic/security-threat-hunting-investigations | |||
src/plugins/discover/public/context_awareness/profile_providers/observability @elastic/kibana-data-discovery @elastic/obs-ux-logs-team |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tonyghiani I noticed this wasn't updated when we added the observability
folder but I figure it makes sense to share ownership here.
`--plugin-path=${resolve( | ||
__dirname, | ||
'../../../test/analytics/plugins/analytics_ftr_helpers' | ||
)}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elastic/kibana-core could confirm for sure since they maintain the service, but I'm pretty sure this does alter the behaviour because it adds a custom analytics shipper to capture EBT events in tests. I added a separate config for the telemetry tests here: 67a5954.
I also wasn't really sure how to handle the custom services to make sure they aren't available where they won't work, so I tried to do something similar to what was done for API integration tests: 38b9306. Let me know if you think there's a better way to do this and I can update.
Summary
As suggested in #199255 (comment), I've copied and modified the existing Discover context awareness telemetry tests to work for Observability profiles. This helps test that solution root profiles are picked up as expected, as well as giving us some serverless coverage.
@elastic/appex-qa It didn't seem like there were any config specific services for serverless tests yet, so I added the EBT to services to the main serverless config. If there's a better way to do this, please let me know and I can update it.
Checklist
release_note:breaking
label should be applied in these situations.release_note:*
label is applied per the guidelines