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: Custom integration creation / integration update fails on changing subobjects property on mapping #183496

Closed
flash1293 opened this issue May 15, 2024 · 15 comments · Fixed by #186991
Assignees
Labels
Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@flash1293
Copy link
Contributor

flash1293 commented May 15, 2024

When creating a custom integration (

createCustomIntegrationHandler
) via /api/fleet/epm/custom_integrations for a datastream that already contains log datastreams created via the root logs template, the creation fails because the integration tries to set subobjects: false for all streams:

As the existing datastreams don't have this set and it's not possible to change this setting after the fact, the creation fails with:

[2024-05-14T11:50:29.324+02:00][INFO ][plugins.fleet] Attempt to update the mappings for the logs-abcd.xyz-default (write_index_only)
[2024-05-14T11:50:29.333+02:00][ERROR][plugins.fleet] Mappings update for logs-abcd.xyz-default failed due to unexpected error: ResponseError: mapper_exception
Root causes:
mapper_exception: the [subobjects] parameter can't be updated for the object mapping [_doc]

The same happens when trying to upgrade an integration that has a data stream which didn't have subobjects defined in the old version but sets subobjects: false after the upgrade.

Steps to reproduce:

  • Create a new dataset:
POST logs-myintegration.test-default/_doc
{
   "message": "abc"
}
  • Go to /app/observabilityOnboarding/customLogs/?category=logs and create a new integration like thisL
Screenshot 2024-05-15 at 12 43 18

clicking "Continue" fails:
Screenshot 2024-05-15 at 12 44 23

I can think of the following fixes:

  • Do not set subobjects: false for custom integrations for now
  • Make it configurable
  • Check whether there is matching data already and only set it if there is none
  • Make it default in the central logs template (probably a dangerous change as it's so far reaching)

This is relevant for near-term plans around offering automated mapping issue mitigation flows.

cc @ruflin @achyutjhunjhunwala

@flash1293 flash1293 added the Team:Fleet Team label for Observability Data Collection Fleet team label May 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@kpollich
Copy link
Member

Hey @flash1293 thanks for the issue. The custom integrations code paths within Fleet were actually contributed by o11y (namely @Kerry350), so I'm a little wary of making changes here. To me, the Do not set subobjects: false for custom integrations for now option seems easiest to me, but I worry I'm not seeing the whole picture around custom integrations, so maybe Kerry can answer this better than me, or route us to someone who can.

Ref initial custom integrations scope: #159991

@flash1293
Copy link
Contributor Author

Actually I added the subobjects: false recently - we want to make that the default anyway (see elastic/elasticsearch#106812), and I thought it would be a safe thing to do for the custom integrations - turns out it's not.

@ruflin
Copy link
Contributor

ruflin commented May 16, 2024

As the existing datastreams don't have this set and it's not possible to change this setting after the fact, the creation fails with

Is just the update of the existing mapping failing or also if you set it on a template and then trigger the rollover?

@flash1293
Copy link
Contributor Author

@ruflin

Is just the update of the existing mapping failing or also if you set it on a template and then trigger the rollover?

Only the existing mapping update for the current write index here:

I tested the rollover case and it works as expected. So that would indeed be another option - to not attempt to update subobjects for the current write index, but instead just roll over.

@ruflin
Copy link
Contributor

ruflin commented May 17, 2024

I tested the rollover case and it works as expected.

This is a common behaviour when upgrading Integration packages AFAIK. At first it tries to update the mapping and if we get an error, the template is updated and then a rollover is triggered as there are also other changes which require a rollover, like change to TSDS. The reason rollover is not always triggered is because this could lead to too many indices quickly.

@flash1293
Copy link
Contributor Author

Ah, that's a good point @ruflin - we should definitely handle this case gracefully as it will also happen if an integration is switching over to subobjects: false. @kpollich what do you think about detecting this case and handling it gracefully?

@flash1293 flash1293 changed the title Fleet: Custom integration creation fails on existing default data streams Fleet: Custom integration creation / integration update fails on changing subobjects property on mapping May 17, 2024
@flash1293
Copy link
Contributor Author

Updated the title to reflect that the scope is broader.

@kpollich
Copy link
Member

what do you think about detecting this case and handling it gracefully?

Yes I think we should handle this via rollover as we do for standard integrations.

@kpollich
Copy link
Member

Pulling this into the team's next sprint as a P1 bug. It should be relatively straightforward, and we should get some non-o11y folks into the custom integrations code regardless since we are codeowners on it 🙂

@flash1293
Copy link
Contributor Author

@kpollich It's been a while this bug is open, is there anything to clear up about how to fix it? It's blocking some integration work.

@kpollich
Copy link
Member

It's been a while this bug is open, is there anything to clear up about how to fix it? It's blocking some integration work.

I don't think there are any blockers here, it's just been tough to fit in with other priorities. Will try to make sure this is done as part of the current sprint. I'll bump the priority here.

For context - the Fleet team only oversaw the "custom integrations" work that was contributed to our API. We haven't actively maintained or contributed to these APIs to date, so this will be new ground for the team.

@criamico
Copy link
Contributor

@flash1293 I tried to reproduce this bug locally following the steps in the description but I didn't get any error.

Followed steps:

POST logs-testintegration.test-default/_doc
{
   "message": "abc"
}

Screenshot 2024-06-26 at 14 56 44

Screenshot 2024-06-26 at 14 57 35

Screenshot 2024-06-26 at 14 57 49

Screenshot 2024-06-26 at 14 58 28

Is there any other steps that I'm missing? I tried twice but couldn't reproduce both times.

@flash1293
Copy link
Contributor Author

@criamico In the second screenshot, the "dataset" name should be test, not testintegration.

In the data stream logs-testintegration.test-default you created, testintegration is the integration name, test is the dataset name and default is the namespace.

@criamico
Copy link
Contributor

Thanks @flash1293, I have a PR with a tentative fix: #186991 Could you please take a look?

criamico added a commit that referenced this issue Jul 1, 2024
…apper_exception error (#186991)

Fixes #183496
Fixes #180062

## Summary
Custom integration creation / integration update currently fails on
changing subobjects property on mapping with `mapper_exception` error.
This PR handles this case by triggering a rollover when the error is
received.

## Testing 
- In dev tools, enter the following
 ```
  POST logs-testintegration.test-default/_doc
  {
     "message": "abc"
  }
```
- Navigate to `/app/observabilityOnboarding/customLogs/?category=logs` and fill it with the following:

![Screenshot 2024-06-26 at 15 19 19](https://github.com/elastic/kibana/assets/16084106/a8a0016b-79aa-41b9-9b9b-8a4198a43feb)
In the screenshot the error is visible but in the current branch the request should succeed and install `testintegration` correctly.


### Checklist

- [ ] [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

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
5 participants