[Core] Upgrade Assistant warns the global SO HTTP APIs are deprecated

[TinaHeiligers]

PR #196081 adds support for surfacing deprecated APIs in the upgrade assistant.

We should surface the deprecated global saved objects HTTP APIs in Upgrade Assistant as an early warning of removal at a later stage. To show these warnings, deprecated has to change from a boolean to an object.
The warning will complement how the deprecations are logged.

We expect to update alternative APIs as they become available.

Deprecated Saved objects APIs

/api/saved_objects/{type}/{id}
/api/saved_objects/resolve/{type}/{id}
/api/saved_objects/{type}/{id?}
/api/saved_objects/{type}/{id}
/api/saved_objects/_find
/api/saved_objects/{type}/{id}
/api/saved_objects/_bulk_get
/api/saved_objects/_bulk_create
/api/saved_objects/_bulk_resolve
/api/saved_objects/_bulk_update
/api/saved_objects/_bulk_delete

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[TinaHeiligers]

I've had to add docLinks as a dependency for route registration and add a few new docLinks.

[TinaHeiligers]

@Bamieh we might need to add deprecation types for APIs that will be removed or migrated in the future. ATM, the deprecation for SO APIs renders as "route xyz is removed".

From PR #197936:

The response entry is:

{
            "apiId": "unversioned|get|/api/saved_objects/{type}/{id}",
            "title": "The \"GET /api/saved_objects/{type}/{id}\" route is removed",
            "level": "warning",
            "message": [
                "The API \"GET /api/saved_objects/{type}/{id}\" has been called 3 times. The last call was on Sunday, October 27, 2024 3:05 PM -07:00."
            ],
            "documentationUrl": "https://www.elastic.co/guide/en/kibana/master/saved-objects-api.html#saved-objects-api",
            "correctiveActions": {
                "manualSteps": [
                    "Identify the origin of these API calls.",
                    "This API no longer exists and no replacement is available. Delete any requests you have that use this API.",
                    "Check that you are no longer using the old API in any requests, and mark this issue as resolved. It will no longer appear in the Upgrade Assistant unless another call using this API is detected."
                ],
                "mark_as_resolved_api": {
                    "routePath": "/api/saved_objects/{type}/{id}",
                    "routeMethod": "get",
                    "apiTotalCalls": 3,
                    "totalMarkedAsResolved": 0,
                    "timestamp": "2024-10-27T22:28:26.688Z"
                }
            },
            "deprecationType": "api",
            "domainId": "core.api_deprecations"
        }

[Bamieh]

I dont think we'd need a new type it is just the copy text that might be misleading to developers. However I think for people attempting to upgrade I do agree with @florent-leborgne that using is removed might make more sense since it is talking about a future upgrade the user is upgrading towards that the API has already been removed in.

Happy to revisit the copy to chance it back again to will be removed but i dont think a new type is needed for this. its too neunced and confusing.

Here is some parts of the discussion that bubbled to github: #196081 (comment)

@florent-leborgne please feel free to jump in :)

[florent-leborgne]

@Bamieh @TinaHeiligers From what I understood in the initial conversation, there are 4 major cases:

• Moved: In the target version, the API has moved to another API that users must use instead.
• Removed: In the target version, the API no longer exists and has no replacement.
• Deprecated: In the target version, the API is deprecated. It will be removed in a future version, later. It's not removed yet in the target version, just deprecated.
• Version bump: In the target version, the API has a newer version that users should use instead.

So I think the question is not about the tense that we use but rather about making sure that we don't mix up the removed and deprecated cases. Please correct me if I'm wrong :)

[jloleysens]

Looking at the linked comment:

In 8.16, this API is deprecated. It will be removed in a future version.

Perhaps this does point to something we are currently lacking... The way I see it we could:

1. Create a new deprecation type type: deprecated that can only ever be of type warning to capture "removed at some future date"
2. Change the type: removed copy based on severity. severity can be either warning or critical. If severity is critical we assume integrations will break once upgraded if severity is warning we assume that integrations should change, but don't need to right now, can soften then copy to "will be removed"

IMO (2) seems nicer than having a new deprecation type, but also invites us to do the same thing for other types based on severity. We would possibly have (3x2=)6 new copy entries. One for each type and based on severity. We should also document that in the severity setting. My q here is: is it safe to assume that we've covered all cases correctly with this revision based on severity? Could there be a warning severity for type: removed where the copy should just be "has been removed" instead of "will be removed"?

WDYT @Bamieh @TinaHeiligers @florent-leborgne ?

[Bamieh]

The relationship is not super direct between severity and will be or has been. I do understand your point that critical severity is critical because the API has been removed while the warning indicates that it is still there. However this will require developers to have intimate knowledge with the interface to do this distinction and it'll also require constant update to the route deprecation details for different versions. I dont think we'll have this level of adoption and care from developers.

I would go with the simplest mental model which is just changing the copy:

1. Update the tense to will be removed since the deprecation will be removed in the future weather we know when or we dont.
2. I am debating between adding a clarification message to the users about the critical and warning but i am leaning towards keeping it vague since it'll carry different meanings for different routes what warning/critical boils down to. For example it might be critical severity because this API is part of a critical process the user needs to follow for alerts even though there is no specific removal date on the horizon.

[jloleysens]

The relationship is not super direct between severity and will be or has been

Yeah, I agree it's a bit fuzzy to me too so ++ to keeping it simple with changing the copy and not altering the API.

[jloleysens]

@florent-leborgne if you agree, should we go ahead and change the copy for the removal case to future tense or does this compromise consistency too much?

[florent-leborgne]

We can but I'm not sure if this really helps the customer know what they need do now (which is different if the removal happens in the target version or later in a future version).

If you change is removed to will be removed, we should also change the 2nd step accordingly by replacing This API no longer exists and no replacement is available. with This API will be removed and won't be replaced.

[TinaHeiligers]

We can but I'm not sure if this really helps the customer know what they need do now (which is different if the removal happens in the target version or later in a future version).

I've been thinking through this more and keep landing with adding at least 1 type: "deprecated", with optional text that extends the copy: "This API is deprecated. (optional-text}"

[florent-leborgne]

That is where I landed as well, if that is do-able I'll +1 on the idea

[TinaHeiligers]

That is where I landed as well, if that is do-able I'll +1 on the idea

I'll only get back to this next week. I'm on serverless onCall until then

[Bamieh]

So to summarize if we add a deprecated generic deprecation type to the APIs:

1. the removed type will be is removed indicating that it has been removed in a future version so you need to stop usting it now and resolve the issue (probably forcing severity: critical here makes sense as well)
2. the deprecated type will show is deprecated which indicates that the API is deprecated, it might be removed or migrated, or got a version bump in the future. for now it is a warning that this will be changed but in your next upgrade the api will still work (severity: warning in most cases)

[TinaHeiligers]

the deprecated type will show is deprecated which indicates that the API is deprecated, it might be removed or migrated, or got a version bump in the future

I'm hoping to apply this in #197936

[TinaHeiligers]

@Bamieh This issue was closed with #198800 but the Saved Objects APIs still need their deprecated field changed, which is what I'm doing in #197936.