Skip to content
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

🌊 Streams: Handle stale classic streams #208221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jan 24, 2025

An invariant we have to handle somehow is if a user made additions to an unwired data stream via the streams interface, then the underlying data stream gets deleted.

This is "allowed", since the data stream is not managed by streams.

Currently, the UI breaks if this happens and shows error toasts when trying to load the doc count or when trying to change processing.

This PR makes this a regular case the API can handle:

  • The GET /api/streams/<id> endpoint does not throw, but still returns the existing definition. A new key data_stream_exists indicates whether we are in this situation
  • The UI clearly communicates to the user and doesn't try to do additional requests
Screenshot 2025-01-24 at 16 42 23

Trying to update ingest via the API will still result in an error.

Another weird behavior related to that was that if a dashboard is linked to a stale classic stream, no definition is ever saved and the stream disappears from the list when deleted, making the dashboard link inaccessible. This PR introduces ensureStream which is called by the dashboard APIs and makes sure the definition is there if dashboard links exist. As a side effect, this makes sure that a user can't add dashboard links to a stream they don't have access to - IMHO we should have done that from the start.

This does not touch wired streams - for those, the data stream getting deleted is a breach of contract. We should still handle it gracefully, but in this case I think we should go another route and offer a button in the UI to use the "resync" API to reconcile the state of the streams layer and Elasticsearch. I will look into this on a separate PR.

@flash1293 flash1293 added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:version Backport to applied version labels v8.18.0 Feature:Streams This is the label for the Streams Project labels Jan 24, 2025
@flash1293 flash1293 requested a review from a team as a code owner January 24, 2025 15:49
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/streams-schema 201 206 +5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
streamsApp 280.1KB 281.1KB +1012.0B
Unknown metric groups

API count

id before after diff
@kbn/streams-schema 201 206 +5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Streams This is the label for the Streams Project release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants