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

feat: allow date_nanos dates in timestamp selection #795

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

jehuty0shift
Copy link
Contributor

@jehuty0shift jehuty0shift commented Jun 14, 2024

Description

This change is enabling the selection of date_nanos date type for creating detectors in the plugin. The feature has been merged on the plugin side
opensearch-project/anomaly-detection#1238 and is being backported to 2.X :
opensearch-project/anomaly-detection#1249

The change add the date_nanos dataType and enable its selection as a date field.

I could test the change by cherry-picking this commit this code to 2.14 branch and testing OpenSearch Dashboard against a cluster with the patched anomaly-detection plugin (to pass validation on date_nanos).

The mapping of the timestamp field is the following

        "timestamp" : {
          "type" : "date_nanos",
          "format" : "epoch_millis||yyyy-MM-dd HH:mm:ss.SSS||yyyy-MM-dd HH:mm:ss.SSSSSS||yyyy-MM-dd HH:mm:ss.n"
        },

Attached are the screenshot showing the field appearing in the date fields listing and the final detector validated. The last screenshot shows an historical analysis working as intended.

pr-1
pr-2
pr-3

Issues Resolved

No issues have been opened on this project.

Check List

  • New functionality includes testing.
  • All tests pass
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@jehuty0shift
Copy link
Contributor Author

I couldn't run all tests on my laptop (out of RAM), I'm ready to contribute to this PR by writing some unit test if needed, but i would need an example to replicate (i am not at all proficient in javascript/typescript). I don't know if documentation must be updated to reflect this change since it doesn't indicate the limitation in the first place.

@kaituo
Copy link
Collaborator

kaituo commented Jun 19, 2024

I couldn't run all tests on my laptop (out of RAM), I'm ready to contribute to this PR by writing some unit test if needed, but i would need an example to replicate (i am not at all proficient in javascript/typescript). I don't know if documentation must be updated to reflect this change since it doesn't indicate the limitation in the first place.

You can use https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/main/public/pages/DefineDetector/components/Datasource/__tests__/IndexOption.test.tsx as an example. Once you finished, I can help run all tests in github CI.

@kaituo
Copy link
Collaborator

kaituo commented Jun 19, 2024

Please rebase from main as I fixed CI #798

Signed-off-by: Babacar Diasse <babacar.diasse@corp.ovh.com>
@kaituo
Copy link
Collaborator

kaituo commented Jun 20, 2024

Can you list what manual tests you have done in the Description part?

@jehuty0shift
Copy link
Contributor Author

I added some text and screenshot, let me know if you need something more.

@kaituo kaituo merged commit 89b6056 into opensearch-project:main Jun 20, 2024
9 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 20, 2024
Signed-off-by: Babacar Diasse <babacar.diasse@corp.ovh.com>
(cherry picked from commit 89b6056)
kaituo pushed a commit that referenced this pull request Jun 20, 2024
Signed-off-by: Babacar Diasse <babacar.diasse@corp.ovh.com>
(cherry picked from commit 89b6056)

Co-authored-by: jehuty0shift <jehuty0shift@users.noreply.github.com>
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.

2 participants