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

[Fleet] fix dynamic mapping missing time_series_metric #160417

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

juliaElastic
Copy link
Contributor

@juliaElastic juliaElastic commented Jun 23, 2023

Summary

Closes #155004

Found a simplification of adding time_series_metric to dynamic_templates.

I think tsdb enabled check is not needed similarly to #157047

To test:

  • Install package by upload or local registry istio-0.3.3.zip
  • Check that the created metrics-istio.istiod_metrics index template includes time_series_metric
"dynamic_templates": [
        {
          "istio.istiod.metrics.*.counter": {
            "path_match": "istio.istiod.metrics.*.counter",
            "mapping": {
              "time_series_metric": "counter",
              "type": "double"
            }
          }
        },
        {

@juliaElastic juliaElastic self-assigned this Jun 23, 2023
@juliaElastic juliaElastic requested a review from a team as a code owner June 23, 2023 14:39
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 23, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@juliaElastic juliaElastic added release_note:skip Skip the PR/issue when compiling release notes and removed Team:Fleet Team label for Observability Data Collection Fleet team labels Jun 23, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@@ -206,6 +206,7 @@ function _generateMappings(
case 'boolean':
dynProperties = {
type: field.object_type,
time_series_metric: field.metric_type,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found that metric_type can directly used here, so not needed to put time_series_metric on the field.

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 23, 2023
Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

Code LGTM 🚀

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #2 / Policy Details Malware Protection card "after all" hook for "user should be able to see related rules"
  • [job] [logs] Defend Workflows Cypress Tests #2 / Policy Details Malware Protection card "before all" hook for "user should be able to see related rules"

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 416 420 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 497 501 +4
total +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @juliaElastic

@juliaElastic juliaElastic merged commit 863f6bd into elastic:main Jun 23, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 23, 2023
)

## Summary

Closes elastic#155004

Found a simplification of adding `time_series_metric` to
`dynamic_templates`.

I think tsdb enabled check is not needed similarly to
elastic#157047

To test:
- Install package by upload or local registry
[istio-0.3.3.zip](https://github.com/elastic/kibana/files/11849494/istio-0.3.3.zip)
- Check that the created `metrics-istio.istiod_metric`s index template
includes `time_series_metric`
```
"dynamic_templates": [
        {
          "istio.istiod.metrics.*.counter": {
            "path_match": "istio.istiod.metrics.*.counter",
            "mapping": {
              "time_series_metric": "counter",
              "type": "double"
            }
          }
        },
        {
```

(cherry picked from commit 863f6bd)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jun 23, 2023
…) (#160444)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Fleet] fix dynamic mapping missing `time_series_metric`
(#160417)](#160417)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Bardi","email":"90178898+juliaElastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-06-23T17:51:12Z","message":"[Fleet]
fix dynamic mapping missing `time_series_metric` (#160417)\n\n##
Summary\r\n\r\nCloses
#155004 a
simplification of adding `time_series_metric`
to\r\n`dynamic_templates`.\r\n\r\nI think tsdb enabled check is not
needed similarly
to\r\nhttps://github.com//pull/157047\r\n\r\nTo test:\r\n-
Install package by upload or local
registry\r\n[istio-0.3.3.zip](https://github.com/elastic/kibana/files/11849494/istio-0.3.3.zip)\r\n-
Check that the created `metrics-istio.istiod_metric`s index
template\r\nincludes
`time_series_metric`\r\n```\r\n\"dynamic_templates\": [\r\n {\r\n
\"istio.istiod.metrics.*.counter\": {\r\n \"path_match\":
\"istio.istiod.metrics.*.counter\",\r\n \"mapping\": {\r\n
\"time_series_metric\": \"counter\",\r\n \"type\": \"double\"\r\n }\r\n
}\r\n },\r\n
{\r\n```","sha":"863f6bdb8bca99af02392e74ec75e37373bdcfc0","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v8.9.0","v8.10.0"],"number":160417,"url":"#160417
fix dynamic mapping missing `time_series_metric` (#160417)\n\n##
Summary\r\n\r\nCloses
#155004 a
simplification of adding `time_series_metric`
to\r\n`dynamic_templates`.\r\n\r\nI think tsdb enabled check is not
needed similarly
to\r\nhttps://github.com//pull/157047\r\n\r\nTo test:\r\n-
Install package by upload or local
registry\r\n[istio-0.3.3.zip](https://github.com/elastic/kibana/files/11849494/istio-0.3.3.zip)\r\n-
Check that the created `metrics-istio.istiod_metric`s index
template\r\nincludes
`time_series_metric`\r\n```\r\n\"dynamic_templates\": [\r\n {\r\n
\"istio.istiod.metrics.*.counter\": {\r\n \"path_match\":
\"istio.istiod.metrics.*.counter\",\r\n \"mapping\": {\r\n
\"time_series_metric\": \"counter\",\r\n \"type\": \"double\"\r\n }\r\n
}\r\n },\r\n
{\r\n```","sha":"863f6bdb8bca99af02392e74ec75e37373bdcfc0"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"#160417
fix dynamic mapping missing `time_series_metric` (#160417)\n\n##
Summary\r\n\r\nCloses
#155004 a
simplification of adding `time_series_metric`
to\r\n`dynamic_templates`.\r\n\r\nI think tsdb enabled check is not
needed similarly
to\r\nhttps://github.com//pull/157047\r\n\r\nTo test:\r\n-
Install package by upload or local
registry\r\n[istio-0.3.3.zip](https://github.com/elastic/kibana/files/11849494/istio-0.3.3.zip)\r\n-
Check that the created `metrics-istio.istiod_metric`s index
template\r\nincludes
`time_series_metric`\r\n```\r\n\"dynamic_templates\": [\r\n {\r\n
\"istio.istiod.metrics.*.counter\": {\r\n \"path_match\":
\"istio.istiod.metrics.*.counter\",\r\n \"mapping\": {\r\n
\"time_series_metric\": \"counter\",\r\n \"type\": \"double\"\r\n }\r\n
}\r\n },\r\n
{\r\n```","sha":"863f6bdb8bca99af02392e74ec75e37373bdcfc0"}}]}]
BACKPORT-->

Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com>
@ritalwar
Copy link

ritalwar commented Jul 4, 2023

Hi @juliaElastic, now that this pull request has been merged and we have pending TSDB package migrations that rely on it, we need to update the Kibana version. I would like to confirm if the fix will be included in version 8.9.0 or if we should consider alternative versions such as 8.8.1 or 8.8.2

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Jul 4, 2023

@ritalwar As discussed on slack, we can't backport to 8.8 branch as 8.8.2 is released and the branch is closed.
The fix will be included in 8.9.0 as it is backported there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.9.0 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] TSDB: need metric_type mapping for dynamic templates
7 participants