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

[UII] Fill in empty values for constant_keyword fields from existing mappings #188145

Merged
merged 6 commits into from
Jul 12, 2024

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Jul 11, 2024

Summary

Resolves #178528.

Some packages declare constant_keyword type fields without an explicit value. This causes ES to fill in the value in the mappings using the first ingested value.

When upgrading this type of package & field after the value has already been populated in this way, the mappings update fail due to pushing a null value into an existing value, triggering unnecessary rollovers.

This PR fixes that by filling in the empty values from the existing mappings.

Test

  1. On an empty cluster, turn on debug logs
  2. Set up Fleet Server policy and Fleet Server agent
  3. Force install old version of Elastic Agent integration, v1.19.2:
POST kbn:/api/fleet/epm/packages/elastic_agent/1.19.2
{
  "force": true
}
  1. Create a new empty policy, deselect system and agent monitoring (otherwise the integration will be upgraded, we do not want this yet)
  2. Manually add Elastic Agent integration v1.19.2 to the new policy
  3. Edit the policy to enable logs and metrics monitoring
  4. Enroll agent into the policy, confirm that monitoring logs and metrics are being ingested and that a value exists for event.dataset mapping for the logs:
GET logs-elastic_agent*/_mappings
            "dataset": {
              "type": "constant_keyword",
              "value": "elastic_agent"
            }
  1. Upgrade Elastic Agent integration to v1.20.0 (note we are not upgrading to the newest versions, 2.0+, because these are expected to trigger rollovers for some data streams):
POST kbn:/api/fleet/epm/packages/elastic_agent/1.20.0
{
  "force": true
}
  1. Confirm in Kibana logs that no rollovers triggered during the upgrade
  2. Confirm that there is still only 1 backing index for monitoring logs:
GET logs-elastic_agent*

Checklist

Delete any items that are not applicable to this PR.

…om the mappings of the existing backing index
@jen-huang jen-huang added release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team backport:prev-minor Backport to the previous minor version (i.e. one version back from main) v8.16.0 labels Jul 11, 2024
@jen-huang jen-huang self-assigned this Jul 11, 2024
@jen-huang jen-huang requested a review from a team as a code owner July 11, 2024 18:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

mappings: MappingTypeMapping,
currentPath: string[] = []
) => {
for (const [key, potentialField] of Object.entries(mappings)) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any value to check specifically for .properties instead of going through everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I updated this function

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.

one small question, code LGTM 🚀

@jen-huang jen-huang enabled auto-merge (squash) July 11, 2024 21:34
@jen-huang jen-huang merged commit b7c96f4 into elastic:main Jul 12, 2024
21 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @jen-huang

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 12, 2024
…g mappings (elastic#188145)

## Summary

Resolves elastic#178528.

Some packages declare `constant_keyword` type fields without an explicit
value. This causes ES to fill in the value in the mappings using the
first ingested value.

When upgrading this type of package & field after the value has already
been populated in this way, the mappings update fail due to pushing a
`null` value into an existing value, triggering unnecessary rollovers.

This PR fixes that by filling in the empty values from the existing
mappings.

## Test
1. On an empty cluster, turn on debug logs
2. Set up Fleet Server policy and Fleet Server agent
3. Force install old version of Elastic Agent integration, v1.19.2:
```
POST kbn:/api/fleet/epm/packages/elastic_agent/1.19.2
{
  "force": true
}
```
4. Create a new empty policy, **deselect system and agent monitoring**
(otherwise the integration will be upgraded, we do not want this yet)
5. Manually add Elastic Agent integration v1.19.2 to the new policy
6. Edit the policy to enable logs and metrics monitoring
7. Enroll agent into the policy, confirm that monitoring logs and
metrics are being ingested and that a value exists for `event.dataset`
mapping for the logs:
```
GET logs-elastic_agent*/_mappings
```
```
            "dataset": {
              "type": "constant_keyword",
              "value": "elastic_agent"
            }
```
9. Upgrade Elastic Agent integration to v1.20.0 (note we are not
upgrading to the newest versions, 2.0+, because these **are** expected
to trigger rollovers for some data streams):
```
POST kbn:/api/fleet/epm/packages/elastic_agent/1.20.0
{
  "force": true
}
```
10. Confirm in Kibana logs that no rollovers triggered during the
upgrade
11. Confirm that there is still only 1 backing index for monitoring
logs:
```
GET logs-elastic_agent*
```

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit b7c96f4)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.15

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 Jul 12, 2024
…elds from existing mappings (#188145) (#188170)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[UII] Fill in empty values for `constant_keyword` fields
from existing mappings
(#188145)](#188145)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Jen
Huang","email":"its.jenetic@gmail.com"},"sourceCommit":{"committedDate":"2024-07-12T03:05:03Z","message":"[UII]
Fill in empty values for `constant_keyword` fields from existing
mappings (#188145)\n\n## Summary\r\n\r\nResolves
#178528 packages
declare `constant_keyword` type fields without an explicit\r\nvalue.
This causes ES to fill in the value in the mappings using the\r\nfirst
ingested value.\r\n\r\nWhen upgrading this type of package & field after
the value has already\r\nbeen populated in this way, the mappings update
fail due to pushing a\r\n`null` value into an existing value, triggering
unnecessary rollovers.\r\n\r\nThis PR fixes that by filling in the empty
values from the existing\r\nmappings.\r\n\r\n## Test\r\n1. On an empty
cluster, turn on debug logs\r\n2. Set up Fleet Server policy and Fleet
Server agent\r\n3. Force install old version of Elastic Agent
integration, v1.19.2:\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.19.2\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n4. Create a new empty policy, **deselect system and
agent monitoring**\r\n(otherwise the integration will be upgraded, we do
not want this yet)\r\n5. Manually add Elastic Agent integration v1.19.2
to the new policy\r\n6. Edit the policy to enable logs and metrics
monitoring\r\n7. Enroll agent into the policy, confirm that monitoring
logs and\r\nmetrics are being ingested and that a value exists for
`event.dataset`\r\nmapping for the logs:\r\n```\r\nGET
logs-elastic_agent*/_mappings\r\n```\r\n```\r\n \"dataset\": {\r\n
\"type\": \"constant_keyword\",\r\n \"value\": \"elastic_agent\"\r\n
}\r\n```\r\n9. Upgrade Elastic Agent integration to v1.20.0 (note we are
not\r\nupgrading to the newest versions, 2.0+, because these **are**
expected\r\nto trigger rollovers for some data streams):\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.20.0\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n10. Confirm in Kibana logs that no rollovers
triggered during the\r\nupgrade\r\n11. Confirm that there is still only
1 backing index for monitoring\r\nlogs:\r\n```\r\nGET
logs-elastic_agent*\r\n```\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b7c96f4c09e88b820664bbd0bb996844dd50a0e6","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","backport:prev-minor","v8.16.0"],"title":"[UII]
Fill in empty values for `constant_keyword` fields from existing
mappings","number":188145,"url":"#188145
Fill in empty values for `constant_keyword` fields from existing
mappings (#188145)\n\n## Summary\r\n\r\nResolves
#178528 packages
declare `constant_keyword` type fields without an explicit\r\nvalue.
This causes ES to fill in the value in the mappings using the\r\nfirst
ingested value.\r\n\r\nWhen upgrading this type of package & field after
the value has already\r\nbeen populated in this way, the mappings update
fail due to pushing a\r\n`null` value into an existing value, triggering
unnecessary rollovers.\r\n\r\nThis PR fixes that by filling in the empty
values from the existing\r\nmappings.\r\n\r\n## Test\r\n1. On an empty
cluster, turn on debug logs\r\n2. Set up Fleet Server policy and Fleet
Server agent\r\n3. Force install old version of Elastic Agent
integration, v1.19.2:\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.19.2\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n4. Create a new empty policy, **deselect system and
agent monitoring**\r\n(otherwise the integration will be upgraded, we do
not want this yet)\r\n5. Manually add Elastic Agent integration v1.19.2
to the new policy\r\n6. Edit the policy to enable logs and metrics
monitoring\r\n7. Enroll agent into the policy, confirm that monitoring
logs and\r\nmetrics are being ingested and that a value exists for
`event.dataset`\r\nmapping for the logs:\r\n```\r\nGET
logs-elastic_agent*/_mappings\r\n```\r\n```\r\n \"dataset\": {\r\n
\"type\": \"constant_keyword\",\r\n \"value\": \"elastic_agent\"\r\n
}\r\n```\r\n9. Upgrade Elastic Agent integration to v1.20.0 (note we are
not\r\nupgrading to the newest versions, 2.0+, because these **are**
expected\r\nto trigger rollovers for some data streams):\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.20.0\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n10. Confirm in Kibana logs that no rollovers
triggered during the\r\nupgrade\r\n11. Confirm that there is still only
1 backing index for monitoring\r\nlogs:\r\n```\r\nGET
logs-elastic_agent*\r\n```\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b7c96f4c09e88b820664bbd0bb996844dd50a0e6"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"#188145
Fill in empty values for `constant_keyword` fields from existing
mappings (#188145)\n\n## Summary\r\n\r\nResolves
#178528 packages
declare `constant_keyword` type fields without an explicit\r\nvalue.
This causes ES to fill in the value in the mappings using the\r\nfirst
ingested value.\r\n\r\nWhen upgrading this type of package & field after
the value has already\r\nbeen populated in this way, the mappings update
fail due to pushing a\r\n`null` value into an existing value, triggering
unnecessary rollovers.\r\n\r\nThis PR fixes that by filling in the empty
values from the existing\r\nmappings.\r\n\r\n## Test\r\n1. On an empty
cluster, turn on debug logs\r\n2. Set up Fleet Server policy and Fleet
Server agent\r\n3. Force install old version of Elastic Agent
integration, v1.19.2:\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.19.2\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n4. Create a new empty policy, **deselect system and
agent monitoring**\r\n(otherwise the integration will be upgraded, we do
not want this yet)\r\n5. Manually add Elastic Agent integration v1.19.2
to the new policy\r\n6. Edit the policy to enable logs and metrics
monitoring\r\n7. Enroll agent into the policy, confirm that monitoring
logs and\r\nmetrics are being ingested and that a value exists for
`event.dataset`\r\nmapping for the logs:\r\n```\r\nGET
logs-elastic_agent*/_mappings\r\n```\r\n```\r\n \"dataset\": {\r\n
\"type\": \"constant_keyword\",\r\n \"value\": \"elastic_agent\"\r\n
}\r\n```\r\n9. Upgrade Elastic Agent integration to v1.20.0 (note we are
not\r\nupgrading to the newest versions, 2.0+, because these **are**
expected\r\nto trigger rollovers for some data streams):\r\n```\r\nPOST
kbn:/api/fleet/epm/packages/elastic_agent/1.20.0\r\n{\r\n \"force\":
true\r\n}\r\n```\r\n10. Confirm in Kibana logs that no rollovers
triggered during the\r\nupgrade\r\n11. Confirm that there is still only
1 backing index for monitoring\r\nlogs:\r\n```\r\nGET
logs-elastic_agent*\r\n```\r\n\r\n### Checklist\r\n\r\nDelete any items
that are not applicable to this PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"b7c96f4c09e88b820664bbd0bb996844dd50a0e6"}}]}]
BACKPORT-->

Co-authored-by: Jen Huang <its.jenetic@gmail.com>
@jen-huang jen-huang deleted the constant-keyword-rollover branch July 12, 2024 04:52
jen-huang added a commit that referenced this pull request Sep 14, 2024
## Summary

Follow up to #188145. In some edge cases
(elastic/sdh-beats#5156), users could override
the index template used by integration data streams. It is possible to
create an index template without mappings, this causes
`fillConstantKeywordValues` to receive an undefined object when
upgrading the integration, and the upgrade then fails.

This PR makes the backfill operation here more fail-safe.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 14, 2024
## Summary

Follow up to elastic#188145. In some edge cases
(elastic/sdh-beats#5156), users could override
the index template used by integration data streams. It is possible to
create an index template without mappings, this causes
`fillConstantKeywordValues` to receive an undefined object when
upgrading the integration, and the upgrade then fails.

This PR makes the backfill operation here more fail-safe.

(cherry picked from commit b5abc4e)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 14, 2024
## Summary

Follow up to elastic#188145. In some edge cases
(elastic/sdh-beats#5156), users could override
the index template used by integration data streams. It is possible to
create an index template without mappings, this causes
`fillConstantKeywordValues` to receive an undefined object when
upgrading the integration, and the upgrade then fails.

This PR makes the backfill operation here more fail-safe.

(cherry picked from commit b5abc4e)
kibanamachine added a commit that referenced this pull request Sep 14, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[UII] Make constant keyword backfill optional
(#192921)](#192921)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Jen
Huang","email":"its.jenetic@gmail.com"},"sourceCommit":{"committedDate":"2024-09-14T02:34:46Z","message":"[UII]
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","backport:prev-major"],"title":"[UII]
Make constant keyword backfill
optional","number":192921,"url":"#192921
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#192921
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3"}}]}]
BACKPORT-->

Co-authored-by: Jen Huang <its.jenetic@gmail.com>
kibanamachine added a commit that referenced this pull request Sep 14, 2024
# Backport

This will backport the following commits from `main` to `8.15`:
- [[UII] Make constant keyword backfill optional
(#192921)](#192921)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Jen
Huang","email":"its.jenetic@gmail.com"},"sourceCommit":{"committedDate":"2024-09-14T02:34:46Z","message":"[UII]
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","backport:prev-major"],"title":"[UII]
Make constant keyword backfill
optional","number":192921,"url":"#192921
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#192921
Make constant keyword backfill optional (#192921)\n\n##
Summary\r\n\r\nFollow up to #188145. In some edge
cases\r\n(elastic/sdh-beats#5156), users could
override\r\nthe index template used by integration data streams. It is
possible to\r\ncreate an index template without mappings, this
causes\r\n`fillConstantKeywordValues` to receive an undefined object
when\r\nupgrading the integration, and the upgrade then
fails.\r\n\r\nThis PR makes the backfill operation here more
fail-safe.","sha":"b5abc4ec7e308815b0f338f4c836a9caf3ee48a3"}}]}]
BACKPORT-->

Co-authored-by: Jen Huang <its.jenetic@gmail.com>
markov00 pushed a commit to markov00/kibana that referenced this pull request Sep 18, 2024
## Summary

Follow up to elastic#188145. In some edge cases
(elastic/sdh-beats#5156), users could override
the index template used by integration data streams. It is possible to
create an index template without mappings, this causes
`fillConstantKeywordValues` to receive an undefined object when
upgrading the integration, and the upgrade then fails.

This PR makes the backfill operation here more fail-safe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v8.15.0 v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] constant_keyword mapping should provide a value
5 participants