[Streams] Only mark _changes as true when values are meaningful by flash1293 · Pull Request #250806 · elastic/kibana

flash1293 · 2026-01-28T19:02:41Z

🍒 Summary

Fixes unnecessary validation and Elasticsearch actions (like rollovers) when creating streams with empty/default property values.

Closes #241300

🛠️ Changes

Updated WiredStream.doHandleUpsertChange() to only mark properties as changed when values are meaningful:
- ownFields: only true when fields object is non-empty
- routing: only true when routing rules array is non-empty
- processing: only true when processing.steps array is non-empty
- lifecycle: only true when lifecycle is not inherit (default)
- settings: only true when settings object is non-empty
- failure_store: only true when failure_store is not inherit (default)
Updated ClassicStream.doHandleUpsertChange() with the same pattern:
- processing: only true when processing.steps array is non-empty
- lifecycle: only true when lifecycle is not inherit (default)
- settings: only true when settings object is non-empty
- field_overrides: only true when field_overrides object is non-empty
- failure_store: only true when failure_store is not inherit (default)
Added comprehensive unit tests for both WiredStream and ClassicStream covering:
- New stream creation with empty vs non-empty values
- Existing stream updates with actual vs no changes
- Edge cases for all property types

🎙️ Prompts

Investigate the root cause of unnecessary rollovers when creating streams
Implement conditional change tracking for empty/default values
Add unit tests covering all change tracking scenarios
Validate with integration tests

🤖 This pull request was assisted by Cursor

Fixes elastic#241300 When creating a new stream with empty/default values for properties like `fields`, `processing.steps`, `routing`, etc., the change tracking logic was unconditionally marking all properties as changed. This caused unnecessary validation and ES actions (like rollovers) to execute. This fix updates the `doHandleUpsertChange` method in both WiredStream and ClassicStream to only mark a property as changed when: - For new streams: the value is non-empty/non-default - For existing streams: the value actually differs from the previous state Properties affected: - WiredStream: ownFields, routing, processing, lifecycle, settings, failure_store - ClassicStream: processing, lifecycle, settings, field_overrides, failure_store Co-authored-by: Cursor <cursoragent@cursor.com>

flash1293 · 2026-01-30T14:46:04Z

/ralph the change looks good, but the code you wrote is very repetitive. Identify a way to make it a bit more DRY for the pattern of the check (if it's a create, check whether there is something, if it's a change, check whether something got changed).

Extract repetitive pattern for determining if a stream property changed into a reusable helper function. This makes the code more DRY by abstracting the logic: for existing streams, check if values changed; for new streams, check if value is meaningful/non-empty.

flash1293 · 2026-01-30T15:48:23Z

Refactored the change detection logic to be more DRY by introducing a computeChange helper function.

Changes:

Added computeChange(isExistingStream, hasMeaningfulValue, hasChanged) helper to helpers.ts
Updated classic_stream.ts to use the helper for all 5 change flags (processing, lifecycle, settings, field_overrides, failure_store)
Updated wired_stream.ts to use the helper for all 6 change flags (ownFields, routing, failure_store, processing, lifecycle, settings)

The helper abstracts the common pattern:

For existing streams: returns true if values changed (calls the hasChanged callback)
For new streams: returns true if the value is meaningful/non-empty

Written by cursor, wait for @flash1293 to chime in

elasticmachine · 2026-02-02T17:38:46Z

Pinging @elastic/obs-onboarding-team (Team:obs-onboarding)

flash1293 · 2026-02-03T11:23:03Z

This is ready for human review

Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # x-pack/platform/plugins/shared/streams/server/lib/streams/state_management/streams/classic_stream.ts # x-pack/platform/plugins/shared/streams/server/lib/streams/state_management/streams/wired_stream.ts

CoenWarmer · 2026-02-17T09:50:46Z

+    const isExistingStream = !!startingStateStreamDefinition;
+
+    this._changes.ownFields = computeChange(
+      isExistingStream,


Perhaps we can consider passing an object with descriptive keys to computeChange so it's more immediately apparent what these arguments represent. Wdyt?

CoenWarmer

Code looks good, left one comment to consider

flash1293 · 2026-02-17T10:01:02Z

Fair point - /ralph can you change the interface of computeChange to an options object so it's clear what parameter means what?

Changed the interface of computeChange from positional parameters to an options object with descriptive keys (isExistingStream, hasMeaningfulValue, hasChanged) for improved readability at call sites. Co-authored-by: Cursor <cursoragent@cursor.com>

flash1293 · 2026-02-17T12:24:43Z

Changed the interface of computeChange from positional parameters to an options object with descriptive keys.

Changes:

Added ComputeChangeOptions interface with documented properties (isExistingStream, hasMeaningfulValue, hasChanged)
Updated function signature to use destructured options object
Updated all 11 call sites in classic_stream.ts (5) and wired_stream.ts (6)
All 35 existing tests pass

Example call site before:

computeChange(isExistingStream, hasValue, () => !_.isEqual(a, b))

Example call site after:

computeChange({
  isExistingStream,
  hasMeaningfulValue: hasValue,
  hasChanged: () => !_.isEqual(a, b),
})

Written by cursor, wait for @flash1293 to chime in

flash1293 · 2026-02-17T12:26:17Z

The refactor looks good to me, any other thoughts @CoenWarmer ?

flash1293 · 2026-02-18T13:24:18Z

@CoenWarmer should be good for another look

CoenWarmer · 2026-02-18T21:00:51Z

LGTM

Update test files to use proper type annotations instead of `as any` casts to comply with the no-explicit-any ESLint rule that was added to the streams plugins. Co-authored-by: Cursor <cursoragent@cursor.com>

elasticmachine · 2026-02-19T11:16:53Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: e862ed0

Failed CI Steps

Scout: [ security / entity_store ] plugin

Test Failures

[job] [logs] Scout: [ security / entity_store ] plugin / local-serverless-security_complete - Entity Store Logs Extraction - Should extract properly generate euid for host

Metrics [docs]

✅ unchanged

History

💔 Build #396160 failed 1dbdd88
💔 Build #396125 failed 4772489
💔 Build #396105 failed ca5f4f8
💔 Build #395952 failed 398abc6
💚 Build #395180 succeeded 4bf1841
💚 Build #394434 succeeded d410aca

…tic#250806) ## 🍒 Summary Fixes unnecessary validation and Elasticsearch actions (like rollovers) when creating streams with empty/default property values. Closes elastic#241300 ## 🛠️ Changes - Updated `WiredStream.doHandleUpsertChange()` to only mark properties as changed when values are meaningful: - `ownFields`: only true when `fields` object is non-empty - `routing`: only true when routing rules array is non-empty - `processing`: only true when `processing.steps` array is non-empty - `lifecycle`: only true when lifecycle is not `inherit` (default) - `settings`: only true when `settings` object is non-empty - `failure_store`: only true when failure_store is not `inherit` (default) - Updated `ClassicStream.doHandleUpsertChange()` with the same pattern: - `processing`: only true when `processing.steps` array is non-empty - `lifecycle`: only true when lifecycle is not `inherit` (default) - `settings`: only true when `settings` object is non-empty - `field_overrides`: only true when `field_overrides` object is non-empty - `failure_store`: only true when failure_store is not `inherit` (default) - Added comprehensive unit tests for both `WiredStream` and `ClassicStream` covering: - New stream creation with empty vs non-empty values - Existing stream updates with actual vs no changes - Edge cases for all property types ## 🎙️ Prompts - Investigate the root cause of unnecessary rollovers when creating streams - Implement conditional change tracking for empty/default values - Add unit tests covering all change tracking scenarios - Validate with integration tests 🤖 This pull request was assisted by Cursor --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

flash1293 added Team:obs-onboarding Observability Onboarding Team Feature:Streams This is the label for the Streams Project v9.4.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Feb 2, 2026

flash1293 marked this pull request as ready for review February 2, 2026 17:38

flash1293 requested review from a team as code owners February 2, 2026 17:38

flash1293 and others added 4 commits February 2, 2026 18:42

Merge branch 'main' into ralph/issue-42

8b6496a

Changes from yarn openapi:bundle

20f7d92

Merge branch 'main' into ralph/issue-42

f3c6646

Merge branch 'main' into ralph/issue-42

1363a09

flash1293 added 5 commits February 5, 2026 20:17

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

c45584f

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

32cc439

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

831e413

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

cafe2da

flash1293 mentioned this pull request Feb 11, 2026

Unskip Streams routing preview filter test (9.2) and reduce skipped-suite noise in Observability onboarding FTR #252336

Closed

flash1293 added 3 commits February 11, 2026 21:08

Merge branch 'main' into ralph/issue-42

1fb0243

Merge upstream/main into ralph/issue-42

d410aca

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

4bf1841

CoenWarmer reviewed Feb 17, 2026

View reviewed changes

flash1293 added 3 commits February 17, 2026 15:48

Merge branch 'main' into ralph/issue-42

ca5f4f8

Merge branch 'main' into ralph/issue-42

4772489

Merge branch 'main' into ralph/issue-42

1dbdd88

CoenWarmer approved these changes Feb 18, 2026

View reviewed changes

flash1293 and others added 2 commits February 19, 2026 10:37

Merge remote-tracking branch 'upstream/main' into ralph/issue-42

c0c0471

fix(streams): resolve no-explicit-any ESLint errors after merge

e862ed0

Update test files to use proper type annotations instead of `as any` casts to comply with the no-explicit-any ESLint rule that was added to the streams plugins. Co-authored-by: Cursor <cursoragent@cursor.com>

flash1293 enabled auto-merge (squash) February 19, 2026 11:10

flash1293 merged commit 8ee57cd into elastic:main Feb 19, 2026
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Streams] Only mark _changes as true when values are meaningful#250806

[Streams] Only mark _changes as true when values are meaningful#250806
flash1293 merged 20 commits intoelastic:mainfrom
flash1293:ralph/issue-42

flash1293 commented Jan 28, 2026

Uh oh!

flash1293 commented Jan 30, 2026

Uh oh!

flash1293 commented Jan 30, 2026

Uh oh!

elasticmachine commented Feb 2, 2026

Uh oh!

flash1293 commented Feb 3, 2026

Uh oh!

CoenWarmer Feb 17, 2026

Uh oh!

CoenWarmer left a comment

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 18, 2026

Uh oh!

CoenWarmer commented Feb 18, 2026

Uh oh!

Uh oh!

elasticmachine commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

flash1293 commented Jan 28, 2026

🍒 Summary

🛠️ Changes

🎙️ Prompts

Uh oh!

flash1293 commented Jan 30, 2026

Uh oh!

flash1293 commented Jan 30, 2026

Uh oh!

elasticmachine commented Feb 2, 2026

Uh oh!

flash1293 commented Feb 3, 2026

Uh oh!

CoenWarmer Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

CoenWarmer left a comment

Choose a reason for hiding this comment

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 17, 2026

Uh oh!

flash1293 commented Feb 18, 2026

Uh oh!

CoenWarmer commented Feb 18, 2026

Uh oh!

Uh oh!

elasticmachine commented Feb 19, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants