Skip to content

[Lens][as-code] fix metric trendlines#254093

Closed
drewdaemon wants to merge 12 commits into
elastic:mainfrom
drewdaemon:251620/fix-trendlines
Closed

[Lens][as-code] fix metric trendlines#254093
drewdaemon wants to merge 12 commits into
elastic:mainfrom
drewdaemon:251620/fix-trendlines

Conversation

@drewdaemon
Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon commented Feb 19, 2026

Summary

Fix #251620

Screen.Recording.2026-02-19.at.3.21.14.PM.mov

Checklist

@drewdaemon drewdaemon added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Feb 20, 2026
@drewdaemon drewdaemon marked this pull request as ready for review February 20, 2026 19:58
@drewdaemon drewdaemon requested a review from a team as a code owner February 20, 2026 19:58
@elastic elastic deleted a comment from elasticmachine Feb 20, 2026
@drewdaemon
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@andrimal andrimal left a comment

Choose a reason for hiding this comment

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

Tested locally and works great! 🚀

Left only one comment.

operation: 'date_histogram',
include_empty_rows: true,
suggested_interval: 'auto',
use_original_time_range: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed that currently without the api transformations, when setting a trendline from UI the date histograms are created without ignoreTimeRange.

I don't think this creates a different functionality in this case, but may be for parity we should also omit it? What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we should have parity, thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 18391fe!

@drewdaemon drewdaemon requested a review from andrimal February 24, 2026 20:30
@drewdaemon drewdaemon enabled auto-merge (squash) February 24, 2026 20:30
}),
schema.object({
type: schema.literal('trend'),
time_field: schema.string({ meta: { description: 'Time field for trend chart' } }),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is simple and effective, but I have doubts this can correctly scale (it doesn't consider for example how we want to integrate ESQL in the near future).

The concept of field is limited to DSL and doesn't play with the current ESQL setup we use.

Since this is actually configured behind the scene in Lens datasource as a date_hisogram + same metric layer configuration and is done based on the current data_view configuration, do you think we can instead of doing that at the transformation side, we can "hydrate" the datasources at the rendering/runtime side?
In this way we don't need anything special, we just need to consider that the background_chart is of type trend and we inject that datasource cnofiguration.

When we are going to implement ESQL + the possibility to configure that trendline in a better way (different agg, dfferent histogram etc) we can add a parameter like the operation that allows us to construct and provide a better definition.

WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As you suggested offline, I'll try out using the full date histogram schema instead of the short-hand field.

"background_chart": {
  "type": "trend",
  "x": bucketDateHistogramOperationSchema
}

This is a schema we will have to support in ES|QL anyways, so we aren't making things harder in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I tried this out, but I'm not sure it's an improvement. It does re-use something we already have to support, but it is much more verbose and still mentions the word, "field."

Screenshot 2026-02-25 at 1 20 09 PM

WDYT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's just add the operation + field that is the way we determine the type of operation.
Everything else can be hidden (we don't have a real contract on how we handle the trendline behind the scene so is fine to avoid that)

@drewdaemon drewdaemon disabled auto-merge February 25, 2026 16:35
@drewdaemon
Copy link
Copy Markdown
Contributor Author

There's a desire to plan for trendline schema consistency across the Lens classic visualizations (DSL) and ES|QL-based visualizations (which will support trendlines at some point).

Therefore, this PR is blocked on further direction from the team.

@markov00
Copy link
Copy Markdown
Contributor

@drewdaemon @andrimal I've tried the runtime hydration mechanism and seems to work fine Please double check this PR #258104

@drewdaemon
Copy link
Copy Markdown
Contributor Author

closing in favor of #258104 !

@drewdaemon drewdaemon closed this Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting blocked release_note:skip Skip the PR/issue when compiling release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Lens as code] Metric trendline not visible

4 participants