[Infra] Add endpoints to manage Custom Dashboards

[mykolaharmash]

Closes #176069

Summary

This adds the logic to register a new Saved Object type to store custom dashboards for Asset Details and adds endpoints to fetch and save custom dashboards.

Changes highlights:

• Renamed the enableInfrastructureHostsCustomDashboards to enableInfrastructureAssetCustomDashboards to make it more generic and support additional asset types in the future
• Added a new Saved Object type
• Moved initialization of all Infra endpoints to plugin's start. This one one of the points on the BE tech debt ticket. Having endpoint initialization in start makes it more convenient to access start dependencies which almost all endpoints require.
• Added savedObjectClient and uiSettingsClient to the custom request context (also one of the ideas for endpoints improvement). Right now infra endpoints use custom libs object with all dependencies required for routes, the idea is to rely on the request context instead because it automatically available for every route handler and by default includes some useful things like scoped service clients.
• Added a wrapper handleRouteErrors to avoid error handling duplication which we now have in a few routes. In the future we could do something similar right within registerRoutes framework function, but this would require a bit of refactoring.

Hot to Test

1. Toggle the UI setting off in Advanced Settings
   
2. Go to the Dev Tools and try the endpoints, both should respond with 403

GET kbn:api/infra/custom-dashboards/host

POST kbn:api/infra/custom-dashboards
{
  "assetType": "host",
  "dashboardIdList": ["0", "1"]
}

3. Toggle the UI setting on
4. Try the endpoints again, now they should work as expected

[mykolaharmash]

/ci

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

[mykolaharmash]

/ci

[mykolaharmash]

/ci

[elasticmachine]

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

[elasticmachine]

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

[cauemarcondes]

I think the name handleRouteErrors is a bit odd. At first, I thought this would be handled only when errors happened. But that is not the case, right? It wraps up the handle and handles when errors happen.

[cauemarcondes]

Does it make sense to implement it here instead?

[mykolaharmash]

I think the name handleRouteErrors is a bit odd. At first, I thought this would be handled only when errors happened. But that is not the case, right? It wraps up the handle and handles when errors happen.

Could you explain a bit more? Not sure I get your point.

Does it make sense to implement it here instead?

Yeah, I mentioned this in the PR description. It would definitely make sense but just a bit out of scope for this change I think.

In the future we could do something similar right within registerRoutes framework function, but this would require a bit of refactoring.

[cauemarcondes]

Added a wrapper handleRouteErrors to avoid error handling duplication which we now have in a few routes. In the future we could do something similar right within registerRoutes framework function, but this would require a bit of refactoring.

Can you elaborate on why this would need refactoring?

[mykolaharmash]

Mainly because some routes already have a bespoke error handling and ideally we'd need to remove those and re-test the routes.

[cauemarcondes]

If you can create a follow-up issue for this tech debt would be nice, so we don't lose track of it.

[mykolaharmash]

Created the issue

[yuliacech]

Advanced settings constant change LGTM 👍

[kpatticha]

You mentioned

"Having endpoint initialization in start makes it more convenient to access start dependencies which almost all endpoints require."

What kind of start dependencies do most of the endpoints require?

[mykolaharmash]

Depends on the endpoint, but there is a getStartServices that we put into libs during start phase and pass down to route initializers. Here are instances of where this method is used. Creating this loophole from the start phase in form of getStartServices() feels a bit weird to me. But I'm happy to discuss, maybe there is a better approach 🙌

[mykolaharmash]

Mistyped there, libs are formed during the setup phase, not start

[jloleysens]

Left a question regarding the access level of the new server routes. Otherwise changes LGTM!

[jloleysens]

FMI: will .encode strip unknown fields?

[mykolaharmash]

Looked at the io-ts code, seems like encode is purely a type guard encode: <A>(a: A): A => a, so it won't let you pass unknown fields.

[jloleysens]

OK, in that case I would recommend cherry picking fields in responses like you are doing here, but same for below:

      const attrs = customDashboards.saved_objects[0].attributes;
      return response.ok({
        body: InfraGetCustomDashboardsResponseBodyRT.encode(
          { a: attr.a, b: attr.b }
        ),
      });

This ensures we are only sending known values over the wire.

[mykolaharmash]

Thanks! I added the access: internal to the endpoints and adjusted the response object.

[kpatticha]

In the end, this might be the feature we'd like to deliver to the users, I just want to highlight that with this approach the custom dashboards are linked tightly with the asset type and it's not abstract enough from the technical point of view.

I think we assume that the user's custom dashboards are built based and focused only on the asset type metrics. What if they want to link a revenue dashboard to all their Kubernetes and hosts?

@roshan-elastic is the above a use case for the custom dashboard in infra or do the users mostly want metrics dashboards we're not prodiving in our created pre-build dashboards??

[crespocarlos]

I think we assume that the user's custom dashboards are built based and focused only on the asset type metrics. What if they want to link a revenue dashboard to all their Kubernetes and hosts?

I guess for the GET endpoint this is fine, or I'm missing something? This endpoint will retrieve only the dashboards linked to the asset type.

It's an interesting use case, though, but the linking feature would have to support linking a dashboard to multiple asset types, and we'd have to change the assetType to be an array both in the Saved Objects schema and in the POST request payload type. Making assetType an array would indeed make this future-proof (In fact, we'd need to have one saved object per dashboardSavedObjectId and assetType being an array to meet that requirement)

[mykolaharmash]

Hey @kpatticha @crespocarlos, just had a chat with @roshan-elastic, he confirmed that we want to stick to dashboards linked to asset types, and there is a feature planned to allow users to disable filtering by a specific asset for a dashboard, so this would enable generic dashboards to work even when you look at some asset details view. This would require an additional field on the saved object, but this will be a non-braking change and we can add it with a migration.

It seems that we're good and nothing needs to be changed in this PR, so I'll go ahead and merge it.

[kpatticha]

nit: Could be just me but I find it easier to read the code when the schema is defined here
ex

Suggested change

	create: schema.object(createSchema),
	create: schema.object({
	dashboardSavedObjectIdList:
	schema.arrayOf(schema.string()),
	assetType: schema.string(),
	kuery: schema.maybe(schema.string()),
	}),

[mykolaharmash]

True, that would be more readable. The reason I've put it into a separate object is to have an explicit type Record<keyof InfraCustomDashboard, Type<any>> to make sure schema fields stay the same as the corresponding InfraCustomDashboard type. Tbh, I'd trade off some readability for more type-safety, or maybe there is a way to achieve both that I'm missing?

[bmorelli25]

Docs LGTM

[jloleysens]

Pending making the route internal and sending only known values over the wire this LGTM! Great work!

[jloleysens]

OK, in that case I would recommend cherry picking fields in responses like you are doing here, but same for below:

      const attrs = customDashboards.saved_objects[0].attributes;
      return response.ok({
        body: InfraGetCustomDashboardsResponseBodyRT.encode(
          { a: attr.a, b: attr.b }
        ),
      });

This ensures we are only sending known values over the wire.

[cauemarcondes]

LGTM

[mykolaharmash]

@elasticmachine merge upstream

[mykolaharmash]

@elasticmachine merge upstream

[maryam-saeidi]

I get bad request for the second query, with both enabled and disabled setting:

[mykolaharmash]

I get bad request for the second query, with both enabled and disabled setting:

Sorry @maryam-saeidi, the field name was updated in the process of code review but I forgot to update the description, the new field name is dashboardIdList instead of dashboardSavedObjectIdList

The description is now up to day.

[mykolaharmash]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 0dd6e82

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
infra	29	34	+5

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
infra	8	6	-2

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/saved-objects-service.html#_mappings

id	before	after	diff
infra-custom-dashboards	-	4	+4

Unknown metric groups

API count

id	before	after	diff
infra	32	37	+5

History

• 💛 Build #194119 was flaky 1acb44a
• 💚 Build #193825 succeeded 172881d
• 💔 Build #193760 failed b01e41a
• 💔 Build #193640 failed ab2de78
• 💔 Build #193351 failed 881c5b9c66e1e7cd00b1bf6d3431f3134a4c1c57
• 💔 Build #193319 failed 517846a

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

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.13

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

Questions ?

Please refer to the Backport tool documentation