[Streams] Refactor schemas#218308
Merged
dgieselaar merged 25 commits intoelastic:mainfrom May 1, 2025
Merged
Conversation
dgieselaar
force-pushed
the
streams-refactor-schemas
branch
from
a0d24ca to
5126a88
Compare
dgieselaar
added
backport:version
botelastic
bot
added
the
Team:obs-ux-infra_services - DEPRECATED
Contributor
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/streams --include-path /api/fleet --include-path /api/dashboards --include-path /api/saved_objects/_import --include-path /api/saved_objects/_export --include-path /api/alerting/maintenance_window --update'
maximpn
approved these changes
Apr 28, 2025
Contributor
maximpn
left a comment
maximpn
left a comment
There was a problem hiding this comment.
@dgieselaar Thanks for adding an explanation comment 🙏
Addressing the other comments is up to you.
| function getFlattenedKeys(obj: unknown, parentKey = '', keys: Set<string> = new Set()) { | ||
| if (isPlainObject(obj)) { | ||
| Object.entries(obj as Record<string, any>).forEach(([key, value]) => { | ||
| getFlattenedKeys(value, parentKey ? `${parentKey}.${key}` : key, keys); |
Contributor
There was a problem hiding this comment.
Using parentKey as an array with push/pop as has shown above improves runtime speed up to 5% and reduces memory consumption especially on large objects. GC doesn't have to clean up intermediate parent keys not used in the final result.
I can't say it gives too much profit but just FYI.
flash1293
approved these changes
Apr 29, 2025
Contributor
flash1293
left a comment
flash1293
left a comment
There was a problem hiding this comment.
Discussed offline and the usage of Typescript namespaces looks good to me. Everything still seems to work and it's much more ordered than before.
Did we check why the OAS jsons change so much? Not saying there is a problem, it's probably fine
Contributor
|
@dgieselaar one more thing from #218308 (comment) I think we didn't discuss:
|
Contributor
Author
|
@flash1293 DeepStrict is used for the models (e.g., |
Contributor
|
@dgieselaar Then it might be worth another look in terms of performance for arbitrary nested structures which we unfortunately need to deal with. Can happen outside of this PR, but I imagine there will be issues. |
Contributor
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
Total ESLint disabled count
History
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Streams models to use a namespaced structure, updates import paths, and removes legacy fixture files to streamline the codebase. Additionally, the refactor introduces a proxy-based DeepStrict implementation for improved runtime validation and enhances error handling for AggregateError.
Reviewed Changes
Copilot reviewed 158 out of 158 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/helpers/get_condition_fields.ts | Renamed function from getFields to getConditionFields and updated import path from models to conditions. |
| src/helpers/get_advanced_parameters.ts | Removed the unused helper getInheritedFieldsFromAncestors to simplify parameter processing. |
| src/helpers/condition_to_query_dsl.ts | Updated import path from models to conditions for consistency. |
| src/fixtures/* | Removed multiple legacy fixture files to clean up unused configurations. |
| src/fields/index.ts | Adjusted the import path for recursiveRecord to the new shared path. |
| src/content/index.ts | Transitioned the wiredStream export to a content pack schema for improved type safety. |
| src/conditions/index.ts | Updated import for type guards to reflect the new shared location. |
| src/api/significant_events/api.ts | Updated import of StreamQueryKql to leverage the reorganized queries namespace. |
| src/index.ts | Overhauled exports to introduce the new namespace structure for Streams models and related types. |
| kbn-zod-helpers/src/deep_strict.ts | Refactored DeepStrict to use a proxy wrapper with a new parseStrict function and flattened key comparison. |
| kbn-storage-adapter/index.ts | Updated type constraints to incorporate a MissingKeysError type. |
| kbn-server-route-repository/src/register_routes.ts | Removed redundant Zod schema checks and added detailed AggregateError handling in API error responses. |
| streams_synthtrace_client.ts | Updated the stream upsert request type to use the new Streams.all.UpsertRequest. |
| logging/conversions/message.ts & message.test.ts | Modified error message formatting to specially handle AggregateError instances and added corresponding tests. |
Comments suppressed due to low confidence (2)
src/platform/packages/shared/kbn-zod-helpers/src/deep_strict.ts:36
- The usage of 'input.data' in parseStrict assumes that the input object always has a 'data' property. Confirm that the structure of input is guaranteed or add appropriate checks.
const allInputKeys = Array.from(getFlattenedKeys(input.data));
src/platform/packages/shared/kbn-server-route-repository/src/register_routes.ts:149
- Verify that the error.errors array reliably contains Error objects with a 'message' property to prevent potential runtime errors during the mapping process.
if (error instanceof AggregateError) {
sorenlouv
reviewed
May 1, 2025
Comment on lines
+127
to
+138
| expect(result).toEqual( | ||
| [ | ||
| 'AggregateError: aggregate failed. Caused by:', | ||
| ' > Error: first error', | ||
| ' > at foo', | ||
| ' > Error: second error', | ||
| ' > at bar', | ||
| ' at baz', | ||
| ' at qux', | ||
| ].join('\n') | ||
| ); | ||
| }); |
Member
There was a problem hiding this comment.
nit: Generally not a fan of inline snapshots but this seems like a good use case. Will make it easier to update them if needed.
sorenlouv
approved these changes
May 1, 2025
Contributor
|
Starting backport for target branches: 8.19 https://github.com/elastic/kibana/actions/runs/14773510390 |
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
dgieselaar
added a commit
to dgieselaar/kibana
that referenced
this pull request
May 1, 2025
Refactors Streams models to use namespaces. `Streams` contains both the
TypeScript interfaces and the validation schemas. In general, there are
four variations of each Stream type:
- `Definition` - the definition that we largely work with in the Streams
client and in the browser.
- `Source` - how the definition is stored in ES. This is a pass-through
at the minute, but separating this from the definition allows us to do
migrations on read and write.
- `GetResponse` - what is returned from `GET /api/streams/{name}`. This
includes the definition, but also other assets that are not part of the
definition, but rather a function of the definition, like Elasticsearch
index templates etc, and attached Kibana assets like Dashboards and
Queries.
- `UpsertRequest` - what is required in `PUT /api/streams/{name}`. This
includes the stream definition, but, importantly, without a name - the
name is not allowed in the request endpoint. Similarly, this allows the
user to update assets like Dashboards and Queries.
Schemas are defined as Validations, which are left/right schemas - that
is, the type on the right is always a refinement of the type on the
left. This allows for more safe transitions than going from any object
to the specific type. Additionally, it includes helpers such as `is`,
`as`, `parse` and `asserts`, which help with both runtime validation and
type refinement.
## Additional changes
- Added special formatting for `AggregateError` in MessageConversion
- Simplified DeepStrict
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit a1f2691)
# Conflicts:
# oas_docs/bundle.json
# oas_docs/output/kibana.yaml
# x-pack/test/api_integration/deployment_agnostic/apis/observability/streams/helpers/requests.ts
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
dgieselaar
added a commit
that referenced
this pull request
May 1, 2025
# Backport This will backport the following commits from `main` to `8.19`: - [[Streams] Refactor schemas (#218308)](#218308) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Dario Gieselaar","email":"dario.gieselaar@elastic.co"},"sourceCommit":{"committedDate":"2025-05-01T09:53:55Z","message":"[Streams] Refactor schemas (#218308)\n\nRefactors Streams models to use namespaces. `Streams` contains both the\nTypeScript interfaces and the validation schemas. In general, there are\nfour variations of each Stream type:\n\n- `Definition` - the definition that we largely work with in the Streams\nclient and in the browser.\n- `Source` - how the definition is stored in ES. This is a pass-through\nat the minute, but separating this from the definition allows us to do\nmigrations on read and write.\n- `GetResponse` - what is returned from `GET /api/streams/{name}`. This\nincludes the definition, but also other assets that are not part of the\ndefinition, but rather a function of the definition, like Elasticsearch\nindex templates etc, and attached Kibana assets like Dashboards and\nQueries.\n- `UpsertRequest` - what is required in `PUT /api/streams/{name}`. This\nincludes the stream definition, but, importantly, without a name - the\nname is not allowed in the request endpoint. Similarly, this allows the\nuser to update assets like Dashboards and Queries.\n\nSchemas are defined as Validations, which are left/right schemas - that\nis, the type on the right is always a refinement of the type on the\nleft. This allows for more safe transitions than going from any object\nto the specific type. Additionally, it includes helpers such as `is`,\n`as`, `parse` and `asserts`, which help with both runtime validation and\ntype refinement.\n\n## Additional changes\n\n- Added special formatting for `AggregateError` in MessageConversion\n- Simplified DeepStrict\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"a1f269158748b77a64e1a9dda14cdeeca856b5cd","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:obs-ux-infra_services","backport:version","Feature:Streams","v9.1.0","v8.19.0"],"title":"[Streams] Refactor schemas","number":218308,"url":"https://github.com/elastic/kibana/pull/218308","mergeCommit":{"message":"[Streams] Refactor schemas (#218308)\n\nRefactors Streams models to use namespaces. `Streams` contains both the\nTypeScript interfaces and the validation schemas. In general, there are\nfour variations of each Stream type:\n\n- `Definition` - the definition that we largely work with in the Streams\nclient and in the browser.\n- `Source` - how the definition is stored in ES. This is a pass-through\nat the minute, but separating this from the definition allows us to do\nmigrations on read and write.\n- `GetResponse` - what is returned from `GET /api/streams/{name}`. This\nincludes the definition, but also other assets that are not part of the\ndefinition, but rather a function of the definition, like Elasticsearch\nindex templates etc, and attached Kibana assets like Dashboards and\nQueries.\n- `UpsertRequest` - what is required in `PUT /api/streams/{name}`. This\nincludes the stream definition, but, importantly, without a name - the\nname is not allowed in the request endpoint. Similarly, this allows the\nuser to update assets like Dashboards and Queries.\n\nSchemas are defined as Validations, which are left/right schemas - that\nis, the type on the right is always a refinement of the type on the\nleft. This allows for more safe transitions than going from any object\nto the specific type. Additionally, it includes helpers such as `is`,\n`as`, `parse` and `asserts`, which help with both runtime validation and\ntype refinement.\n\n## Additional changes\n\n- Added special formatting for `AggregateError` in MessageConversion\n- Simplified DeepStrict\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"a1f269158748b77a64e1a9dda14cdeeca856b5cd"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/218308","number":218308,"mergeCommit":{"message":"[Streams] Refactor schemas (#218308)\n\nRefactors Streams models to use namespaces. `Streams` contains both the\nTypeScript interfaces and the validation schemas. In general, there are\nfour variations of each Stream type:\n\n- `Definition` - the definition that we largely work with in the Streams\nclient and in the browser.\n- `Source` - how the definition is stored in ES. This is a pass-through\nat the minute, but separating this from the definition allows us to do\nmigrations on read and write.\n- `GetResponse` - what is returned from `GET /api/streams/{name}`. This\nincludes the definition, but also other assets that are not part of the\ndefinition, but rather a function of the definition, like Elasticsearch\nindex templates etc, and attached Kibana assets like Dashboards and\nQueries.\n- `UpsertRequest` - what is required in `PUT /api/streams/{name}`. This\nincludes the stream definition, but, importantly, without a name - the\nname is not allowed in the request endpoint. Similarly, this allows the\nuser to update assets like Dashboards and Queries.\n\nSchemas are defined as Validations, which are left/right schemas - that\nis, the type on the right is always a refinement of the type on the\nleft. This allows for more safe transitions than going from any object\nto the specific type. Additionally, it includes helpers such as `is`,\n`as`, `parse` and `asserts`, which help with both runtime validation and\ntype refinement.\n\n## Additional changes\n\n- Added special formatting for `AggregateError` in MessageConversion\n- Simplified DeepStrict\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"a1f269158748b77a64e1a9dda14cdeeca856b5cd"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
akowalska622
pushed a commit
to akowalska622/kibana
that referenced
this pull request
May 29, 2025
Refactors Streams models to use namespaces. `Streams` contains both the
TypeScript interfaces and the validation schemas. In general, there are
four variations of each Stream type:
- `Definition` - the definition that we largely work with in the Streams
client and in the browser.
- `Source` - how the definition is stored in ES. This is a pass-through
at the minute, but separating this from the definition allows us to do
migrations on read and write.
- `GetResponse` - what is returned from `GET /api/streams/{name}`. This
includes the definition, but also other assets that are not part of the
definition, but rather a function of the definition, like Elasticsearch
index templates etc, and attached Kibana assets like Dashboards and
Queries.
- `UpsertRequest` - what is required in `PUT /api/streams/{name}`. This
includes the stream definition, but, importantly, without a name - the
name is not allowed in the request endpoint. Similarly, this allows the
user to update assets like Dashboards and Queries.
Schemas are defined as Validations, which are left/right schemas - that
is, the type on the right is always a refinement of the type on the
left. This allows for more safe transitions than going from any object
to the specific type. Additionally, it includes helpers such as `is`,
`as`, `parse` and `asserts`, which help with both runtime validation and
type refinement.
## Additional changes
- Added special formatting for `AggregateError` in MessageConversion
- Simplified DeepStrict
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors Streams models to use namespaces.
Streamscontains both the TypeScript interfaces and the validation schemas. In general, there are four variations of each Stream type:Definition- the definition that we largely work with in the Streams client and in the browser.Source- how the definition is stored in ES. This is a pass-through at the minute, but separating this from the definition allows us to do migrations on read and write.GetResponse- what is returned fromGET /api/streams/{name}. This includes the definition, but also other assets that are not part of the definition, but rather a function of the definition, like Elasticsearch index templates etc, and attached Kibana assets like Dashboards and Queries.UpsertRequest- what is required inPUT /api/streams/{name}. This includes the stream definition, but, importantly, without a name - the name is not allowed in the request endpoint. Similarly, this allows the user to update assets like Dashboards and Queries.Schemas are defined as Validations, which are left/right schemas - that is, the type on the right is always a refinement of the type on the left. This allows for more safe transitions than going from any object to the specific type. Additionally, it includes helpers such as
is,as,parseandasserts, which help with both runtime validation and type refinement.Additional changes
AggregateErrorin MessageConversion