[Synthetics] Perform params API HTTP migration #160575

justinkambic · 2023-06-26T21:43:40Z

Summary

Converts API for global params to avoid referencing SO fields on client and separate interfaces for use on client/server when interacting with SO API.

~~Code is WIP as I still need to convert the getter route.~~

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

apmmachine · 2023-06-26T21:43:58Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

justinkambic · 2023-06-28T14:15:07Z

x-pack/test/api_integration/apis/synthetics/add_edit_params.ts

 import { SYNTHETICS_API_URLS } from '@kbn/synthetics-plugin/common/constants';
 import expect from '@kbn/expect';
 import { syntheticsParamType } from '@kbn/synthetics-plugin/common/types/saved_objects';
 import { FtrProviderContext } from '../../ftr_provider_context';
 import { PrivateLocationTestService } from './services/private_location_test_service';

+function assertHas(actual: unknown, expected: object) {


I couldn't find a good out-of-the-box assertion that did this. I want to say expect(actual).to.contain.all.keys.and.values.of(expected). If there's a cleaner way that lets me remove this helper please LMK.

EDIT: the reason for this is that the id value is dynamic. These assertions seemed to fail using the eql sort-of-equal, which I had previously believed could do this fuzzy comparison.

...plugins/synthetics/public/apps/synthetics/components/settings/global_params/delete_param.tsx

x-pack/plugins/synthetics/public/apps/synthetics/components/settings/hooks/use_params_list.ts

elasticmachine · 2023-06-28T14:21:16Z

Pinging @elastic/uptime (Team:uptime)

…oduce separate `Attributes` type for use on server.

…params-api-http-migration

kibana-ci · 2023-07-04T17:12:36Z

💚 Build Succeeded

Buildkite Build
Commit: 5a30256

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`synthetics`	907.3KB	907.2KB	-45.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	14	16	+2
`securitySolution`	410	414	+4
`synthetics`	72	70	-2
total			+4

References to deprecated APIs

id	before	after	diff
`synthetics`	19	12	-7

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	15	17	+2
`securitySolution`	489	493	+4
`synthetics`	79	77	-2
total			+4

History

💔 Build #139764 failed 419aa26
💚 Build #138865 succeeded 4533c4a
💛 Build #138806 was flaky 33c5cafb84c1101f755253303d5760d906f95c12
💔 Build #138707 failed baba894a87665f8442433d97b6880f66ea31eddb
💔 Build #138658 failed 9c5540f17d988afb9a8186951607cf81f8503609

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

cc @justinkambic

* main: (354 commits) [Synthetics] Overview page fix last refresh value display (elastic#161086) [Synthetics] Remove TLS alert option for ICMP monitor (elastic#161173) fixing the path of manifets for hints autodiscover (elastic#161075) [Fleet] Fix permissions in integrations Assets page (elastic#161233) Update publicBaseUrl warning id (elastic#161204) [ML] Fix Anomaly Explorer URL for alerting context with non-default space (elastic#160899) [Enterprise Search]Add 404 error handling for mappings and documents endpoints (elastic#161203) [Logs Shared] Move LogStream and LogView into new shared plugin (elastic#161151) [Security Solutions] Fix CellActions component should hide ShowTopN action for nested fields (elastic#159645) [SecuritySolutions] Remove filter actions from Cases alerts table and fix show_top_n action (elastic#161150) [Infrastructure UI] Add strict payload validation to inventory_views endpoint (elastic#160852) [api-docs] 2023-07-05 Daily api_docs build (elastic#161225) Fix errors in custom metric payload in SLO dev docs (elastic#161141) [data views] Fix overwrite param for create (elastic#160953) [Synthetics] Perform params API HTTP migration (elastic#160575) [Cloud Security][FTR]Refactor API FTR to use .to.eql instead of .to.be (elastic#160694) Have SLO routes return a 403 instead of a 400 when user has an insufficient license (elastic#161193) [Discover] Fix shared links flaky test (elastic#161172) [ftr] Improve FTR error handling for NoSuchSessionError (elastic#161025) skip flaky suite (elastic#151981) ...

justinkambic added Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Jun 26, 2023

justinkambic self-assigned this Jun 26, 2023

justinkambic force-pushed the 153490/params-api-http-migration branch 2 times, most recently from baba894 to 33c5caf Compare June 28, 2023 12:16

justinkambic commented Jun 28, 2023

View reviewed changes

...plugins/synthetics/public/apps/synthetics/components/settings/global_params/delete_param.tsx Show resolved Hide resolved

justinkambic commented Jun 28, 2023

View reviewed changes

x-pack/plugins/synthetics/public/apps/synthetics/components/settings/hooks/use_params_list.ts Show resolved Hide resolved

justinkambic marked this pull request as ready for review June 28, 2023 14:21

justinkambic requested a review from a team as a code owner June 28, 2023 14:21

justinkambic added 14 commits June 28, 2023 10:21

Remove notion of saved objects from naming of params codec/type. Intr…

065ff82

…oduce separate `Attributes` type for use on server.

Adjust return type and filter keys in params POST handler.

5186c9b

DRY off the API helper.

90586cf

Perform conversion on edit route/endpoint.

cf076d6

Convert delete endpoint.

ebd5beb

Add readonly type.

b6e6a45

Extract saved objects typing from client.

5bcbcfe

Extracted saved objects from params route interfaces.

0a35171

Further simplify.

fdda381

Fix delete params request.

9e0ae94

Add tags to get route return.

f81979c

Fix broken API test.

863aec1

Simplify syntax.

745f98a

Replace deprecated call.

4533c4a

justinkambic force-pushed the 153490/params-api-http-migration branch from 0abfd8a to 4533c4a Compare June 28, 2023 14:28

shahzad31 added 2 commits July 3, 2023 12:47

Merge branch 'main' of https://github.com/elastic/kibana into 153490/…

449de7b

…params-api-http-migration

update API

3b6713a

update API

419aa26

shahzad31 approved these changes Jul 3, 2023

View reviewed changes

shahzad31 added 2 commits July 4, 2023 18:08

fix

b8cdcc1

Merge branch 'main' of https://github.com/elastic/kibana into 153490/…

5a30256

…params-api-http-migration

shahzad31 merged commit 6731eb3 into elastic:main Jul 4, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jul 4, 2023

shahzad31 deleted the 153490/params-api-http-migration branch July 4, 2023 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Synthetics] Perform params API HTTP migration #160575

[Synthetics] Perform params API HTTP migration #160575

justinkambic commented Jun 26, 2023 •

edited

Loading

apmmachine commented Jun 26, 2023

justinkambic Jun 28, 2023 •

edited

Loading

elasticmachine commented Jun 28, 2023

kibana-ci commented Jul 4, 2023

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

[Synthetics] Perform params API HTTP migration #160575

[Synthetics] Perform params API HTTP migration #160575

Conversation

justinkambic commented Jun 26, 2023 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

apmmachine commented Jun 26, 2023

🤖 GitHub comments

justinkambic Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Jun 28, 2023

kibana-ci commented Jul 4, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

justinkambic commented Jun 26, 2023 •

edited

Loading

justinkambic Jun 28, 2023 •

edited

Loading