-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Fixing data stream support in random sampling #137271
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
Fixing data stream support in random sampling #137271
Conversation
|
Pinging @elastic/es-data-management (Team:Data Management) |
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.
Pull Request Overview
This pull request adds support for data streams in the sampling service. The changes enable sampling configurations to be applied to data streams in addition to regular indices, and ensure that sampling configurations are properly cleaned up when data streams are deleted.
Key changes:
- New utility method to validate that a request targets either a data stream or a single existing index
- Automatic cleanup of sampling configurations when data streams are deleted
- Comprehensive test coverage for data stream operations including creation, deletion, and sampling
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SamplingService.java | Added validation method for data streams/indices and cleanup logic for deleted data streams |
| TransportPutSampleConfigurationAction.java | Updated to use new validation method supporting data streams |
| TransportGetSampleStatsAction.java | Updated to use new validation method supporting data streams |
| TransportGetSampleAction.java | Updated to use new validation method supporting data streams |
| TransportDeleteSampleConfigurationAction.java | Updated to use new validation method supporting data streams |
| GetSampleStatsAction.java | Added includeDataStreams() override to enable data stream support |
| SamplingServiceIT.java | Added integration test for data stream deletion cleanup |
| 30_with_data_streams.yml (get_sample_stats) | Added comprehensive YAML test suite for sample stats with data streams |
| 30_with_data_streams.yml (get_sample) | Added comprehensive YAML test suite for getting samples from data streams |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
seanzatzdev
left a comment
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
…7301) I failed to merge in main before merging elastic#137271 so I didn't pick up the changes in elastic#137290. This accounts for them.
There were some parts of random sampling that would break when using data streams instead of ordinary indices. This fixes those, and adds some tests.