-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
APM switch to Elastic Agent fails with cryptic error #121934
Comments
Pinging @elastic/apm-ui (Team:apm) |
@ogupte Did you manage to reproduce this? Have we seen other case of this? If not, I would suggest to close this assuming it was an error that was resolved in Fleet since then or similar (@joshdover ) |
@ruflin I still have the same issue. We are on v7.16.2 on Elastic Cloud service. |
@nugroho-expereo Thanks for the heads up. Will definitively keep it open then. |
@nugroho-expereo or @simitt do you know what your APM alias node field was set to before trying to run this upgrade? |
@joshdover No I didn't set anything before running the switch. How do I know if alias node field is set? FYI We setup the APM since v7.11 and upgraded to 7.12, 7.13, 7.14, 7.15, 7.16 using Elasticsearch service console. |
@joshdover I wasn't able to reproduce, so cannot offer further advice unfortunately. |
I'm also unable to reproduce this error in EC. I'm curious to inspect your saved APM schema object via We don't do any YAML conversions in this code in APM UI, but I'm guessing the YAML error originately from the compiled input as converts to policy yaml in Fleet? |
After working thru a known bad schema, i was finally able to consistently reproduce the issue. The setup
One of the fields in the saved schema object is
The problemIt seems that the YAML conversion sees the It should be noted that the The fixThe fix to wrap quotes in the migration schema object can be done in either APM Server before it saves the object to Kibana or in Kibana transforming the invalid fields to valid ones as it creates the new package policy. |
Interesting find, thanks for the very detailed investigation and write-up @ogupte.
This makes me think we should solve this on the Fleet side by wrapping all input string values in quotes when rendering the handlebar templates, before we parse the documents with YAML. I can't think of a reason not to do this. @ogupte is there an extended stack trace that would show us where in the Fleet code this is happening? My guess is it's part of the |
+1 on fixing this directly in Fleet; |
unfortunately not, The YAML exception must be wrapped, thrown out of band, or otherwise obscured. :( |
here's the full stack trace from the logs for reference, it doesn't seem very helpful:
|
Pinging @elastic/fleet (Team:Fleet) |
@ogupte We'll handle fixing this in Fleet and backporting to 7.17.x. Would you mind handling documenting your workaround for fixing the issue wherever it would be appropriate? |
Some initial findings:
While #93585 has a good explanation of why this problem presents itself when converting between YAML to JSON back to YAML, I don't agree with the conclusion that putting the onus on package developers and users makes sense. We should be able to handle at least the most common scenarios (like a string that starts with a It appears the initial implementation of this in #91418 was too heavy handed and could be re-implemented in a safer way. Instead of quoting any strings that contain any special characters at any position, we could start by quoting only strings that start with |
On the Fleet side we do not really know how variable are used and some packaged are using string concatenation (like Kafka here for example https://epr.elastic.co/package/kafka/1.1.0/data_stream/log/agent/stream/log.yml.hbs ) It's why we choose to put the responsibility here on the package developper side to quote their values.
|
@nchaulet is correct, Fleet UI really can't assume much at all about how variables are being used in package templates. The only thing I can think of that Fleet could explore is disabling some of the more advanced YAML features, but this would need to be coordinated with Elastic Agent's YAML parsing as well and it doesn't appear to be a supported feature in the YAML library we're using today in Fleet. My recommendation would be to add double quotes around the @simitt Does that make sense to you? |
There is a related discussion around the difficulties of I wonder if this kind of helper can help us in this kind of case too? @nchaulet @joshdover WDYT? |
Hello @ogupte ! Thank you for your write up as well as the fix to this. I currently have a customer with this similar issue but unsure how to best address it with the high level workaround you've provided. Since my customer already has their services up, I think the latter approach you mentioned This is in regards to https://github.com/elastic/sdh-apm/issues/528 |
+1 good find, seems worth exploring for a long-term fix. I've opened #127268 I think in the meantime, we should go ahead and wrap this setting with quotes in the apm package as explained in #121934 (comment) in order to fix this issue for 7.17.x and 8.1.x. @simitt friendly ping on this, do you think we can make this change in the APM package for these patch releases? |
apologies, I missed that message; yes we can do that - elastic/apm-server#7508 |
Removing my assignment from this issue. The plan here is to:
|
I think we can just consider this a duplicate of elastic/apm-server#7508 for now. Going to close this one. |
Looks like this was already fixed by this PR #128704 |
Stack version: 7.16.2
Describe the issue:
A user reported that an error occurs when navigating to
Kibana/APM/Settings/Schema
and trying to initiate the switch to APM Integration.From the Kibana logs:
Steps to reproduce:
It is not clear how to reproduce this, but the logs contains lots of data so this might be enough for finding the root cause for the issue.
Errors in browser console (if relevant):
The text was updated successfully, but these errors were encountered: