[Streams] Add client-side field name validation for wired streams#254152
[Streams] Add client-side field name validation for wired streams#254152flash1293 merged 4 commits intoelastic:mainfrom
Conversation
Adds validation to the "Add field" flyout in the schema editor for wired streams, matching the server-side validation behavior: - Field names must be namespaced (start with attributes., body.structured., scope.attributes., or resource.attributes.) OR be in the keepFields list - Field names cannot be OTel reserved fields (body, attributes, scope, resource, span.id, message, trace.id, log.level) This provides immediate feedback to users when entering invalid field names, rather than waiting for a server error. Changes: - Export otelReservedFields and isOtelReservedField from @kbn/streams-schema - Add validation rules to FieldNameSelector in add_field_flyout.tsx - Add unit tests for validation behavior (14 tests) - Add Scout UI integration test for field name validation Closes: elastic/streams-program#472 Co-authored-by: Cursor <cursoragent@cursor.com>
Self-ReviewOverall AssessmentThe implementation correctly adds client-side field name validation for wired streams, matching the server-side validation behavior. What's Good
Concerns / Areas for Improvement
Verification
Self-review by Cursor |
|
/ralph Please address the "otelReservedFields duplication" point of the review, I agree that this isn't great |
Addresses review feedback about otelReservedFields duplication. The list in kbn-streams-schema must stay in sync with baseMappings keys in logs_layer.ts. Added a test that validates this at test-time to prevent drift. Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up: Address otelReservedFields duplication concernI've addressed the "otelReservedFields duplication" point from the self-review by adding a sync test. What was done
Why this approach
Alternative consideredCould have extracted the reserved fields list directly from
Tests verified
Self-review by Cursor |
|
/ralph The "Add field" button in the flyout should be disabled if the validation fails. |
Change the "Add field" button in the add field flyout to be disabled whenever the form is invalid, not just after a failed submission. This provides better user feedback by immediately indicating when the field name or type validation is not satisfied. Also added tests for the button disabled state behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up: Disable Add field button when validation failsAddressed the request to disable the "Add field" button when validation fails. Changes
Testing
|
|
Pinging @elastic/streams-program-team (Team:streams-program) |
mohamedhamed-ahmed
left a comment
There was a problem hiding this comment.
LGTM! Thank you
|
/ci-ralph |
|
/ralph check the test failures of build 398525 and fix them Build URL: https://buildkite.com/elastic/kibana-pull-request/builds/398525 Use |
Update the 'validates field names for wired streams' test to verify the Add field button is disabled when validation fails, instead of trying to click it. The previous approach conflicted with the recent change that disables the button when form validation fails. Co-authored-by: Cursor <cursoragent@cursor.com>
Test fix for Scout UI failureFixed the failing Scout test Root causeAfter the change to disable the "Add field" button when form validation fails, the test at line 217 was trying to click a disabled button to verify the flyout stays open. Playwright blocks clicks on disabled elements by default, causing a timeout. FixUpdated the test to directly assert that the button is disabled ( The rest of the test flow remains unchanged - it still verifies that after entering a valid field name and type, the button becomes enabled and clicking it successfully adds the field. |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Async chunks
History
|
Summary
Adds client-side validation to the "Add field" flyout in the streams schema editor for wired streams, matching the server-side validation behavior:
attributes.,body.structured.,scope.attributes., orresource.attributes.) OR be in the keepFields listbody,attributes,scope,resource,span.id,message,trace.id,log.level)This provides immediate feedback to users when entering invalid field names, rather than waiting for a server error.
Changes
otelReservedFieldsandisOtelReservedFieldfrom@kbn/streams-schemaFieldNameSelectorinadd_field_flyout.tsxTest plan
foo- should show error about namespaced ECS/OTel schemabody- should show error about OTel reserved fieldattributes.foo- should be accepted (valid namespace)Closes: https://github.com/elastic/streams-program/issues/472
Made with Cursor