Skip to content
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

[Metrics UI] All the times on the Elastic Charts are using UTC instead of the browser timezone #122683

Closed
simianhacker opened this issue Jan 11, 2022 · 5 comments · Fixed by #123139
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@simianhacker
Copy link
Member

Kibana version:

7.16.2+

Elasticsearch version:

7.16.2+

Original install method (e.g. download page, yum, from source, etc.):

Any

Describe the bug:

All the Elastic Charts in the Metrics UI are displaying UTC on the X-Axis. This issue was introduced in elastic/elastic-charts#1383 and appearded in 7.16.2. Anything that is using the niceTimeFormatter and relying on the default behavior is affected by this bug. The solution is to add the browser's timezone param to the BarChart component.

x-pack/plugins/infra/public/alerting/common/criterion_preview_chart/criterion_preview_chart.tsx:      return niceTimeFormatter([xMin, xMax]);
x-pack/plugins/infra/public/alerting/inventory/components/expression_chart.tsx:    return niceTimeFormatter([firstTimestamp, lastTimestamp]);
x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx:    return niceTimeFormatter([firstTimestamp, lastTimestamp]);
x-pack/plugins/infra/public/pages/metrics/inventory_view/components/node_details/tabs/metrics/metrics.tsx:    return niceTimeFormatter([firstTimestamp, lastTimestamp]);
x-pack/plugins/infra/public/pages/metrics/inventory_view/components/node_details/tabs/processes/process_row_charts.tsx:    return niceTimeFormatter([firstTimestamp, lastTimestamp]);
x-pack/plugins/infra/public/pages/metrics/inventory_view/components/timeline/timeline.tsx:    return niceTimeFormatter([firstTimestamp, lastTimestamp]);
x-pack/plugins/infra/public/pages/metrics/metric_detail/components/chart_section_vis.tsx:    () => (metric != null ? niceTimeFormatter(getMaxMinTimestamp(metric)) : undefined),
x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/chart.tsx:      ? niceTimeFormatter([firstRow.timestamp, lastRow.timestamp])

Steps to reproduce:

  1. Ingest Metricbeat data for your local system
  2. Visit the Metrics UI and look at all the charts

Expected behavior:

The time should be displayed in local time.

@simianhacker simianhacker added bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Jan 11, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@jasonrhodes
Copy link
Member

@monfera @elastic/elastic-charts can someone weigh in here? If the above ticket description is wholly accurate, this would be a pretty wide-reaching bug, I'd imagine?

@jasonrhodes
Copy link
Member

Question: If we specify a timezone value to the chart, which should it be? There is a Kibana advanced setting for this but I don't know if we use that in observability apps already. @elastic/infra-monitoring-ui does anybody know if we have a precedent for that?

@smith
Copy link
Contributor

smith commented Jan 11, 2022

does anybody know if we have a precedent for that?

APM gets the timezone from uiSettings and passes it into the series. I think it's done for each chart (most charts use the TimeSeriesChart but they are all just using Elastic Charts)

const timeZone = getTimeZone(core.uiSettings);
return (
<>
<EuiTitle size="xs">
<span>{title}</span>
</EuiTitle>
<ChartContainer
hasData={!!distribution}
height={256}
status={fetchStatus}
id="errorDistribution"
>
<Chart>
<Settings
xDomain={{ min, max }}
tooltip={{ stickTo: 'top' }}
showLegend
showLegendExtra
legendPosition={Position.Bottom}
/>
<Axis
id="x-axis"
position={Position.Bottom}
showOverlappingTicks
tickFormat={xFormatter}
/>
<Axis
id="y-axis"
position={Position.Left}
ticks={2}
gridLine={{ visible: true }}
/>
{timeseries.map((serie) => {
return (
<BarSeries
timeZone={timeZone}

@markov00
Copy link
Member

markov00 commented Jan 12, 2022

The proposed solution is the best choice to do to be fully sure and under control of the timezone used.
It's definitely a change introduced without considering it a breaking change. We can make that change and have the 'local' time as the default one used, but I strongly suggest to connect the timezone to the UI settings so that the user can control that parameter by themself.

Another quick comment: this is not caused directly by the niceTimeFormatter but from the internals default changes of the chart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants