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.

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

[Synthetics] add monitor id migration #143879

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented Oct 24, 2022

Summary

Relates to #143309

Adds a migration to add two fields to each monitor saved object

  1. [ConfigKey.HEARTBEAT_ID] ('id') - The id to use for ES queries to heartbeat. Either the saved object id for UI monitors, or the custom heartbeat id for project monitors. This new key can be used regardless of monitor type as the single source of truth as the id to use for ES queries
  2. [ConfigKey.CONFIG_ID] ('config_id') - This id is always the saved object id. It is an exact match to savedObject.id. I've added this key as a way to perform aggregations against the monitor saved object id in the future, as it's currently impossible to do terms aggregations against savedObject.id but possible to do aggregations os any field within the savedObject attributes. By moving a copy of the id into the saved object attributes, we can perform aggregations if needed in the future.

This PR is a prerequisite for standardizing the use of ConfigKey.HEARTBEAT_ID across the app. Making use of these new keys in a predictable, standardized way will take place in a subsequent PR.

Testing

  1. Checkout the 8.5 branch before pulling down this PR. (Note, you'll want to do this while running ES locally, instead of using a edge oblt ES. If your edge oblt ES is on an older ES version than 8.6, the migration won't go through)
  2. Create 1 of each monitor for both UI and project monitors
  3. Checkout this PR
  4. Navigate to the Monitor Management page and find the get request for monitors
  5. View each monitor's id and config_id value. For project monitors, the id value should match custom_heartbeat_id. For ui monitors, the id value should match the saved object id. For both monitor types, the config_id value should match the saved object id.

Screen Shot 2022-10-27 at 1 20 25 PM

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@dominiqueclarke dominiqueclarke changed the title synthetics - add monitor id migration [Synthetics] add monitor id migration Oct 31, 2022
@dominiqueclarke dominiqueclarke added enhancement New value added to drive a business result v8.6.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes labels Oct 31, 2022
@dominiqueclarke dominiqueclarke marked this pull request as ready for review October 31, 2022 14:19
@dominiqueclarke dominiqueclarke requested review from a team as code owners October 31, 2022 14:19
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -139,7 +139,8 @@ export const TestRunsTable = ({ paginable = true, from, to }: TestRunsTableProps
},
];

const historyIdParam = monitor?.[ConfigKey.CUSTOM_HEARTBEAT_ID] ?? monitor?.[ConfigKey.ID];
const historyIdParam =
monitor?.[ConfigKey.CUSTOM_HEARTBEAT_ID] ?? monitor?.[ConfigKey.MONITOR_QUERY_ID];
Copy link
Contributor

Choose a reason for hiding this comment

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

so we need migration to get rid of this?

@dominiqueclarke dominiqueclarke force-pushed the feature/synthetics-monitor-id-migration branch from 191490a to 9e62978 Compare November 3, 2022 13:26
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Let's do follow up to clean up the usage of the field !!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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.0MB 1.0MB +156.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 58 64 +6
osquery 106 111 +5
securitySolution 440 446 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 66 72 +6
osquery 107 113 +6
securitySolution 517 523 +6
total +20

History

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

@dominiqueclarke dominiqueclarke merged commit 8ae41f2 into elastic:main Nov 3, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 3, 2022
@dominiqueclarke dominiqueclarke deleted the feature/synthetics-monitor-id-migration branch November 3, 2022 15:47
@dominiqueclarke dominiqueclarke restored the feature/synthetics-monitor-id-migration branch November 3, 2022 17:29
@shahzad31
Copy link
Contributor

POST FF Testing, tested this on cloud staging

  1. Created 8.5.2 deployment
  2. Added a monitor, verified attributes.id field is empty
  3. Upgraded to 8.6, field is populated now with so id

@shahzad31 shahzad31 deleted the feature/synthetics-monitor-id-migration branch November 29, 2022 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants