Skip to content

[Streams 🌊] Remove enablement check in PUT /api/streams/{id} for classic streams #212289

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

Merged
merged 9 commits into from
Feb 27, 2025

Conversation

simianhacker
Copy link
Member

@simianhacker simianhacker commented Feb 24, 2025

Summary

The goal of this PR is to enable all the workflows for "Classic" without enabling "Wired" streams. This PR changes the isStreamsEnabled check for PUT /api/streams/{id} to allow for PUT requests for an UnwiredStreamDefinition. This change will allow users to directly navigate to /app/streams and use it to manage "classic" streams. User's would still be required to call POST /api/streams/_enable to work with "wired" streams.

This also includes a fix for the i18n paths that were missed when moving from Observability to Platform.

@simianhacker simianhacker added backport:version Backport to applied version labels v9.1.0 v8.19.0 Feature:Streams This is the label for the Streams Project labels Feb 24, 2025
@simianhacker simianhacker marked this pull request as ready for review February 24, 2025 18:52
@simianhacker simianhacker requested a review from a team as a code owner February 24, 2025 18:52
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything that should work does work, but via the _ingest endpoint it's still possible to write wired streams without explicitly calling the _enable endpoint:

PUT kbn:/api/streams/logs/_ingest
{
    "ingest": {
    "lifecycle": {
      "ilm": {
        "policy": "logs@lifecycle"
      }
    },
    "routing": [],
    "processing": [],
    "wired": {
      "fields": {
        "@timestamp": {
          "type": "date"
        },
        "message": {
          "type": "match_only_text"
        },
        "host.name": {
          "type": "keyword"
        },
        "log.level": {
          "type": "keyword"
        },
        "stream.name": {
          "type": "keyword"
        }
      }
   
    }
  } 
}

This isn't a new thing and I think it's not a big problem - after all enabling isn't anything else but creating that root stream.

Only thing we might need to change is what _disable is doing - right now this wipes the whole thing (including our hidden tracking indices). It's not important for the first release but I guess people would want to disable only the root logs stream without also deleting all the stuff they configured for classic streams as well.

We can also target that once we get to the first release of wired streams, but feel free to account for that now (e.g. we could have explicit _disable for everything and _disable_wired for just wired streams

@simianhacker
Copy link
Member Author

@flash1293 I added the checks to _group and _ingest... give it another look. I'll defer on the _disable since that seem more appropriate for a seperate PR.

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looks good to me after one small nit is addressed.

@@ -75,6 +75,13 @@ const upsertIngestRoute = createServerRoute({
request,
});

if (
!(await streamsClient.isStreamsEnabled()) &&
isWiredStreamDefinition({ name: params.path.name, ...params.body })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flipping these conditions around will save us an async call to streamsClient.isStreamsEnabled in the happy path

@simianhacker simianhacker enabled auto-merge (squash) February 26, 2025 23:22
@simianhacker simianhacker merged commit b797617 into elastic:main Feb 27, 2025
10 checks passed
@kibanamachine
Copy link
Contributor

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
streamsAppWrapper 4.7KB 4.7KB -30.0B

History

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 212289

Questions ?

Please refer to the Backport tool documentation

JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Feb 27, 2025
…assic streams (elastic#212289)

## Summary

The goal of this PR is to enable all the workflows for "Classic" without
enabling "Wired" streams. This PR changes the `isStreamsEnabled` check
for `PUT /api/streams/{id}` to allow for `PUT` requests for an
`UnwiredStreamDefinition`. This change will allow users to directly
navigate to `/app/streams` and use it to manage "classic" streams.
User's would still be required to call `POST /api/streams/_enable` to
work with "wired" streams.

This also includes a fix for the `i18n` paths that were missed when
moving from Observability to Platform.
@simianhacker
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

simianhacker added a commit to simianhacker/kibana that referenced this pull request Feb 28, 2025
…assic streams (elastic#212289)

## Summary

The goal of this PR is to enable all the workflows for "Classic" without
enabling "Wired" streams. This PR changes the `isStreamsEnabled` check
for `PUT /api/streams/{id}` to allow for `PUT` requests for an
`UnwiredStreamDefinition`. This change will allow users to directly
navigate to `/app/streams` and use it to manage "classic" streams.
User's would still be required to call `POST /api/streams/_enable` to
work with "wired" streams.

This also includes a fix for the `i18n` paths that were missed when
moving from Observability to Platform.

(cherry picked from commit b797617)

# Conflicts:
#	x-pack/.i18nrc.json
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2025
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

simianhacker added a commit that referenced this pull request Mar 3, 2025
…for classic streams (#212289) (#212779)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Streams 🌊] Remove enablement check in `PUT /api/streams/{id}` for
classic streams
(#212289)](#212289)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Chris
Cowan","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-02-27T01:35:53Z","message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:version","Feature:Streams","v9.1.0","v8.19.0"],"title":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams","number":212289,"url":"https://github.com/elastic/kibana/pull/212289","mergeCommit":{"message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212289","number":212289,"mergeCommit":{"message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2025
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Mar 4, 2025
…for classic streams (elastic#212289) (elastic#212779)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Streams 🌊] Remove enablement check in `PUT /api/streams/{id}` for
classic streams
(elastic#212289)](elastic#212289)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Chris
Cowan","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-02-27T01:35:53Z","message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (elastic#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:version","Feature:Streams","v9.1.0","v8.19.0"],"title":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams","number":212289,"url":"https://github.com/elastic/kibana/pull/212289","mergeCommit":{"message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (elastic#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212289","number":212289,"mergeCommit":{"message":"[Streams
🌊] Remove enablement check in `PUT /api/streams/{id}` for classic
streams (elastic#212289)\n\n## Summary\n\nThe goal of this PR is to enable all
the workflows for \"Classic\" without\nenabling \"Wired\" streams. This
PR changes the `isStreamsEnabled` check\nfor `PUT /api/streams/{id}` to
allow for `PUT` requests for an\n`UnwiredStreamDefinition`. This change
will allow users to directly\nnavigate to `/app/streams` and use it to
manage \"classic\" streams.\nUser's would still be required to call
`POST /api/streams/_enable` to\nwork with \"wired\" streams.\n\nThis
also includes a fix for the `i18n` paths that were missed when\nmoving
from Observability to
Platform.","sha":"b7976175e59a2ca7adb286f33a53d1ce3b9db20c"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elastic Machine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
…assic streams (elastic#212289)

## Summary

The goal of this PR is to enable all the workflows for "Classic" without
enabling "Wired" streams. This PR changes the `isStreamsEnabled` check
for `PUT /api/streams/{id}` to allow for `PUT` requests for an
`UnwiredStreamDefinition`. This change will allow users to directly
navigate to `/app/streams` and use it to manage "classic" streams.
User's would still be required to call `POST /api/streams/_enable` to
work with "wired" streams.

This also includes a fix for the `i18n` paths that were missed when
moving from Observability to Platform.
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:enhancement v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants