[Fleet UI] Improve fleet ui error message

[cauemarcondes]

While implementing the feature #109516, I had to make some changes to the APM fields located at https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/fleet/get_apm_package_policy_definition.ts#L60. But when I was trying to edit the APM integration, fleet threw this error:

VarDef is undefined is very generic and hard to identify which field is causing the problem.

I would like to improve this error message by adding the exact field that caused the problem. I debugged the fleet code and it would be a matter of adding an extra condition here https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/common/services/validate_package_policy.ts#L136 that validates if varDef is defined, otherwise throw an error with the name of the field that caused the problem.

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[kpollich]

Thanks for raising this! I can take a look at this as we're actively working on and testing this package policy validation code for 7.15/16.

[kpollich]

Hey @cauemarcondes I'm taking a look at this now. Could you provide an example of a change you made to that apmConfigMapping object that triggered a validation error here? I think I've a fix but I want to confirm based on your use case. Thanks!

[cauemarcondes]

I believe if you change the key of one of the fields mapped here it will probably break https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/fleet/get_apm_package_policy_definition.ts#L60.

[kpollich]

I believe if you change the key of one of the fields mapped here it will probably break https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/server/lib/fleet/get_apm_package_policy_definition.ts#L60.

Thanks, @cauemarcondes. I'm having trouble triggering this error message off of master right now. I've tried altering keys and names in that apmConfigMapping object and creating/editing APM policies with no luck. Here's an example where I edited the first two values in that map:

  'apm-server.host-NEW': {
    name: 'host-NEW',
    type: 'text',
  },
  'apm-server.url-NEW': {
    name: 'url0-NEW',
    type: 'text',
    getValue: ({ cloudPluginSetup }) => cloudPluginSetup?.apm?.url,
  },

Maybe I am misunderstanding the context in which this error occurs? Let me know if you any further suggestions to ensure I can reproduce the initial error message here. I'm wondering if https://github.com/elastic/kibana/pull/109887/files#diff-e3331479a7af18c1c3f934373c854d6946f0c590498e9d649d4af03144cf9b44R163 may have inadvertently squashed this.

[cauemarcondes]

Let me see if I can put together a PR with the error, so you can test it.

[cauemarcondes]

@kpollich this PR breaks: #110341. You must use apmpackage 0.4.0

[kpollich]

@cauemarcondes - Thanks so much for this reproducible example. I have one more question if it's not too much trouble. How can I upgrade to version 0.4.0 of the APM package in my local dev environment? It seems I'm only able to see version 0.3.0.

I see we declare the package version here, but is there another step to take in order to actually install this version from the package registry?

kibana/x-pack/plugins/apm/server/lib/fleet/get_apm_package_policy_definition.ts

Lines 16 to 39 in 6d874ca

	export function getApmPackagePolicyDefinition(
	options: GetApmPackagePolicyDefinitionOptions
	) {
	return {
	name: 'Elastic APM',
	namespace: 'default',
	enabled: true,
	policy_id: POLICY_ELASTIC_AGENT_ON_CLOUD,
	output_id: '',
	inputs: [
	{
	type: 'apm',
	enabled: true,
	streams: [],
	vars: getApmPackageInputVars(options),
	},
	],
	package: {
	name: APM_PACKAGE_NAME,
	version: '0.4.0',
	title: 'Elastic APM',
	},
	};
	}

Thanks again for all your help here, and please accept my apologies for my lack of familiarity with APM in this area 🙂

[cauemarcondes]

@kpollich can you try following these steps? How to test these changes

and then I should start your local kibana pointing to the local package registry:

xpack.fleet.registryUrl: 'http://localhost:8080'

[kpollich]

Thanks a ton, @cauemarcondes. I was able to follow those steps and get version 0.4.0 of the APM package installed from local instance of the registry.

I've copied the change from your PR, e.g.

  'apm-server.rum.allow_service_names': {
    name: 'rum_allow_service_names',
    type: 'text',
  }

and unfortunately I'm not able to reproduce any error messages off of the latest from master. Attaching a screen recording demonstrating this. Apologies if the text is small, but I've got the package definition change in my editor on the left, and I'm creating/editing a new policy for APM on the right. In the browser console I'm logging any validation errors and everything is coming back clean.

GitHub doesn't natively support .webm and this recording is too big as a GIF, so here's an external link: https://streamable.com/k9yixk

[cauemarcondes]

hmmm that's odd. Do you have a PR where I can try?

[kpollich]

Sure. Here's a branch - it should be the same as your PR linked earlier: #110493

[kpollich]

Here's something that might be useful. I was able to reproduce errors when attempting to upgrade a policy from APM 0.3.0 to 0.4.0 through our new automated upgrade process. I produced these errors:

{
  rum_allow_service_names: ["No variable definition for rum_allow_service_names found "]
  rum_event_rate_limit: ["No variable definition for rum_event_rate_limit found "]
  rum_event_rate_lru_size: ["No variable definition for rum_event_rate_lru_size found "]
}

Throwing an error in this case results in an upgrade form that's "stuck", however, because the user can't actually edit the underlying policy to exclude these fields.

If we just discard variables for which no varDef exists, however, we risk squashing valuable debugging information for package developers though. Maybe it makes sense to log something out to debug in this case but not report a validation error to the user?

[kpollich]

I implemented the above debug change to the validation process in #110505. I think this addresses the initial issue raised by the PR, and also fixes the automatic upgrade process for APM 0.3.0 -> 0.4.0.

[joshdover]

It seems this specific error condition has been eliminated, but we may want some better high-level error messaging such as Unexpected error during package policy validation: xxx

[jen-huang]

Going to close this one as #110505 is merged. @joshdover can you open an issue for improving the error message/handling in general?