Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Synthetics] Format locations for public API's #195295

Merged
merged 5 commits into from
Oct 8, 2024
Merged

[Synthetics] Format locations for public API's #195295

merged 5 commits into from
Oct 8, 2024

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Oct 7, 2024 •
edited
Loading

Summary

Fixes #194901 !!

Format locations for public API's !!

Public API has been refactored to return list of location ids just like they are provided in request !!

Why we need this?

This is done to make sure, we have consistent input/output response for fields, to enable API usage via terraform.

Before

 "locations": [
  {
   "id": "us-east4-a",
   "label": "North America - US East",
   "geo": {
    "lat": 36.57198,
    "lon": -78.77061
   },
   "isServiceManaged": true
  }
 ],

After

but now we only return the id as provided in the request.

"locations": ["us_east"]

@reakaleek
Copy link
Member

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 7, 2024 •
edited
Loading

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #42 / Observability Rules Synthetics SyntheticsCustomStatusRule TimeWindow - Location threshold = 1 - grouped by location - 2 down location should trigger down alert
  • [job] [logs] FTR Configs #42 / Observability Rules Synthetics SyntheticsCustomStatusRule TimeWindow - Location threshold = 1 - grouped by location - 2 down location should trigger down alert
  • [job] [logs] FTR Configs #76 / Synthetics API Tests AddProjectMonitors handles location formatting for both private and public locations
  • [job] [logs] FTR Configs #90 / Synthetics API Tests AddProjectMonitors handles location formatting for both private and public locations
  • [job] [logs] FTR Configs #90 / Synthetics API Tests AddProjectMonitors handles location formatting for both private and public locations
  • [job] [logs] FTR Configs #76 / Synthetics API Tests AddProjectMonitors handles location formatting for both private and public locations

History

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

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #80 / discover/context_awareness extension getAdditionalCellActions data view mode should not render incompatible cell action for message column
  • [job] [logs] FTR Configs #3 / ObservabilityApp o11y sidenav sidenav & breadcrumbs renders the correct nav and navigate to links
  • [job] [logs] FTR Configs #40 / serverless search UI - feature flags Search index detail page index details page overview should show api key

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 1.2MB 1.2MB +31.0B

History

@shahzad31 shahzad31 marked this pull request as ready for review October 8, 2024 14:01
@shahzad31 shahzad31 requested a review from a team as a code owner October 8, 2024 14:01
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 8, 2024
justinkambic
justinkambic approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Tested locally and everything seems fine.

...bility_solution/synthetics/server/routes/monitor_cruds/formatters/saved_object_to_monitor.ts Show resolved Hide resolved
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Oct 8, 2024
@elasticmachine
Copy link
Contributor

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

@shahzad31 shahzad31 merged commit d21495b into elastic:main Oct 8, 2024
33 of 34 checks passed
@shahzad31 shahzad31 deleted the api-locations branch October 8, 2024 15:52
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11239146272

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 8, 2024
@shahzad31
[Synthetics] Format locations for public API's (elastic#195295)
db5affd 
## Summary

Fixes elastic#194901 !!

Format locations for public API's !!

Public API has been refactored to return list of location ids just like
they are provided in request !!

(cherry picked from commit d21495b)
@kibanamachine kibanamachine mentioned this pull request Oct 8, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 8, 2024
@kibanamachine @shahzad31
[8.x] [Synthetics] Format locations for public API's (#195295) (#…
f215bde 
…195460)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Synthetics] Format locations for public API's
(#195295)](#195295)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-08T15:52:33Z","message":"[Synthetics]
Format locations for public API's (#195295)\n\n## Summary\r\n\r\nFixes
#194901 !!\r\n\r\nFormat
locations for public API's !!\r\n\r\nPublic API has been refactored to
return list of location ids just like\r\nthey are provided in request
!!","sha":"d21495bbce07ed39fdcf077c9170d012adab74a4","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management"],"title":"[Synthetics]
Format locations for public
API's","number":195295,"url":"https://github.com/elastic/kibana/pull/195295","mergeCommit":{"message":"[Synthetics]
Format locations for public API's (#195295)\n\n## Summary\r\n\r\nFixes
#194901 !!\r\n\r\nFormat
locations for public API's !!\r\n\r\nPublic API has been refactored to
return list of location ids just like\r\nthey are provided in request
!!","sha":"d21495bbce07ed39fdcf077c9170d012adab74a4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195295","number":195295,"mergeCommit":{"message":"[Synthetics]
Format locations for public API's (#195295)\n\n## Summary\r\n\r\nFixes
#194901 !!\r\n\r\nFormat
locations for public API's !!\r\n\r\nPublic API has been refactored to
return list of location ids just like\r\nthey are provided in request
!!","sha":"d21495bbce07ed39fdcf077c9170d012adab74a4"}}]}] BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
@acrewdson acrewdson mentioned this pull request Oct 28, 2024
Closed
@tobio
Copy link
Member

tobio commented Oct 30, 2024

@shahzad31 @justinkambic

Folks, this is a breaking change in the API response that we're not even including in the 8.16 release notes. It's not clear why the breaking change is even required, as opposed to just adding a new field into the existing model with the location key. It's a pretty frustrating experience for internal clients, let alone customers consuming the API.

We should at least call this out in the release notes.

shahzad31 added a commit to shahzad31/kibana that referenced this pull request Oct 31, 2024
@shahzad31
Revert "[Synthetics] Format locations for public API's (elastic#195295)"
7ee0baa 
This reverts commit d21495b.
@shahzad31 shahzad31 mentioned this pull request Oct 31, 2024
Closed
shahzad31 added a commit that referenced this pull request Oct 31, 2024
@shahzad31
Revert "[Synthetics] Revert 8.15 public API responses (#195295)" (#19…
184d9a0 
…8511)

Fixes #198518
This reverts commit d21495b and 


## Testing

Create few HTTP, ICMP, TCP and Browser monitors via API in 8.15 and in
this PR

Compared JSOn diffs to make sure responses are same
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 31, 2024
@shahzad31
Revert "[Synthetics] Revert 8.15 public API responses (elastic#195295)…
c86bad9 
…" (elastic#198511)

Fixes elastic#198518
This reverts commit d21495b and

## Testing

Create few HTTP, ICMP, TCP and Browser monitors via API in 8.15 and in
this PR

Compared JSOn diffs to make sure responses are same

(cherry picked from commit 184d9a0)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 31, 2024
@shahzad31
Revert "[Synthetics] Revert 8.15 public API responses (elastic#195295)…
6e9be03 
…" (elastic#198511)

Fixes elastic#198518
This reverts commit d21495b and

## Testing

Create few HTTP, ICMP, TCP and Browser monitors via API in 8.15 and in
this PR

Compared JSOn diffs to make sure responses are same

(cherry picked from commit 184d9a0)
kibanamachine added a commit that referenced this pull request Oct 31, 2024
@kibanamachine @shahzad31
[8.x] Revert &quot;[Synthetics] Revert 8.15 public API responses (#19…
7486f9d 
…5295)&quot; (#198511) (#198596)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Revert &quot;[Synthetics] Revert 8.15 public API responses
(#195295)&quot;
(#198511)](#198511)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-31T18:07:06Z","message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version","v8.17.0"],"title":"Revert
\"[Synthetics] Revert 8.15 public API responses
(#195295)\"","number":198511,"url":"https://github.com/elastic/kibana/pull/198511","mergeCommit":{"message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198511","number":198511,"mergeCommit":{"message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
shahzad31 added a commit that referenced this pull request Oct 31, 2024
@kibanamachine @shahzad31
[8.16] Revert &quot;[Synthetics] Revert 8.15 public API responses (#1…
539d756 
…95295)&quot; (#198511) (#198595)

# Backport

This will backport the following commits from `main` to `8.16`:
- [Revert &quot;[Synthetics] Revert 8.15 public API responses
(#195295)&quot;
(#198511)](#198511)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-31T18:07:06Z","message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version","v8.17.0"],"title":"Revert
\"[Synthetics] Revert 8.15 public API responses
(#195295)\"","number":198511,"url":"https://github.com/elastic/kibana/pull/198511","mergeCommit":{"message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198511","number":198511,"mergeCommit":{"message":"Revert
\"[Synthetics] Revert 8.15 public API responses (#195295)\"
(#198511)\n\nFixes
https://github.com/elastic/kibana/issues/198518\r\nThis reverts commit
d21495b and \r\n\r\n\r\n##
Testing\r\n\r\nCreate few HTTP, ICMP, TCP and Browser monitors via API
in 8.15 and in\r\nthis PR\r\n\r\nCompared JSOn diffs to make sure
responses are
same","sha":"184d9a04486d2afdcbc428308a121cb377abda17"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
nreese pushed a commit to nreese/kibana that referenced this pull request Nov 1, 2024
@shahzad31 @nreese
Revert "[Synthetics] Revert 8.15 public API responses (elastic#195295)…
3f21aad 
…" (elastic#198511)

Fixes elastic#198518
This reverts commit d21495b and 


## Testing

Create few HTTP, ICMP, TCP and Browser monitors via API in 8.15 and in
this PR

Compared JSOn diffs to make sure responses are same
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[synthetics API] inconsistencies between location create / get / update
7 participants
@shahzad31 @reakaleek @kibana-ci @elasticmachine @kibanamachine @tobio @justinkambic