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

Backend: Use int64 type instead of string for from/to date times #191

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

fridgepoet
Copy link
Member

@fridgepoet fridgepoet commented Jun 8, 2023

What this PR does / why we need it:
This is the equivalent fix that was made in the Elasticsearch data source: grafana/grafana#44027
Thank you for creating such a nice description!

When doing Data Histogram aggregation from and to dates are sent as strings. When selected aggregation field has date type in Elastic it's automatically converted by Elastic from a string to a number. However, if the aggregation field has a numeric type, such conversion doesn't happen.

This automatic type conversion happens inside Elastic because date can be represented as a formatted string or a timestamp in milliseconds or seconds. We always pass both values as timestamps in milliseconds anyway so there's no need to use strings, because it's not needed to be converted. This way fields with non-date type will also work fine for Date Histograms.

A similar fix was applied some time ago in the Elasticsearch frontend code #22173.

The equivalent documentation about date in OpenSearch is this: https://opensearch.org/docs/latest/field-types/supported-field-types/date/

Which issue(s) this PR fixes:

Fixes the very specific example in #176, but there will probably still be inconsistent results between alerts(backend) and queries for some time. We aim to resolve that by migrating the OpenSearch data source to a backend data source. This work is in progress.

@fridgepoet fridgepoet marked this pull request as ready for review June 8, 2023 13:48
@fridgepoet fridgepoet requested a review from a team as a code owner June 8, 2023 13:48
@fridgepoet fridgepoet requested review from iwysiu and sarahzinger and removed request for a team June 8, 2023 13:48
@fridgepoet fridgepoet merged commit 1c2819a into main Jun 8, 2023
@fridgepoet fridgepoet deleted the int-date branch June 8, 2023 16:19
@fridgepoet fridgepoet mentioned this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants