Skip to content

[Monitor management] Enable check for public beta#128240

Merged
shahzad31 merged 22 commits intoelastic:mainfrom
shahzad31:public-beta-check
Mar 24, 2022
Merged

[Monitor management] Enable check for public beta#128240
shahzad31 merged 22 commits intoelastic:mainfrom
shahzad31:public-beta-check

Conversation

@shahzad31
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 commented Mar 22, 2022

Summary

Enabled public beta for synthetics service !!

In production a new endpoint is added which will be called to check if user has permissions to access synthetics service.

UI is also based on that endpoint.

In dev, if user is using basic auth, that will auto enable the monitor managed.

Key xpack.uptime.ui.monitorManagement.enabled: true is removed.

/allowed

end point will be called periodically in the kibana task, to determine if user has access.

We call the random location to make sure , one location is not burdened with calls to check access.

Comment thread x-pack/plugins/uptime/public/routes.tsx Outdated
Comment thread x-pack/plugins/uptime/public/pages/monitor_management/service_enabled_wrapper.tsx Outdated
Comment on lines +89 to +92
if (this.authorization) {
// in case username/password is provided, we assume it's always allowed
return true;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not strictly necessary. We can simply add the basicAuth credentials to the json. Also, when running locally, you'll have both synthetics and elastic in the in-memory allow-list already.


if (this.locations.length > 0 && httpsAgent) {
// get a url from a random location
const url = this.locations[Math.floor(Math.random() * this.locations.length)].url;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume we should eventually have a gateway to load balance between available locations? Maybe worth discussing with the cloud team.

Not for now though, we can keep it simple, just saying in case we want to add that to the sync agenda later.

@shahzad31 shahzad31 marked this pull request as ready for review March 23, 2022 16:39
@shahzad31 shahzad31 requested a review from a team as a code owner March 23, 2022 16:39
@shahzad31 shahzad31 added v8.2.0 release_note:skip Skip the PR/issue when compiling release notes release_note:enhancement backport:skip This PR does not require backporting and removed release_note:skip Skip the PR/issue when compiling release notes labels Mar 23, 2022
@botelastic botelastic Bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Mar 23, 2022
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/uptime (Team:uptime)

);
export const getServiceLocationsFailure = createAction<Error>('GET_SERVICE_LOCATIONS_LIST_FAILURE');

export const getSyntheticsServiceEnabled = createAsyncAction<void, { serviceEnabled: boolean }>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to sync and quickly strategize on terms, since we both have features related to "enablement". The shared terms could get confusing in the codebase. How can we separate the concepts of public beta enablement vs admin enablement?

perhaps we can name this function getSyntheticsServiceBetaEnabled and add a comment? And perhaps rename variables like isEnabled to isBetaEnabled?

After tech preview, this code will be pulled out of the code base, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i have renamed everything from enabled to allowed.

title={<h2>{MONITOR_MANAGEMENT_LABEL}</h2>}
body={<p>{PUBLIC_BETA_DESCRIPTION}</p>}
actions={[
<EuiButton color="primary" fill isDisabled={true}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this only disabled until we get the docs href?

};

export const fetchServiceEnabled = async (): Promise<{ serviceEnabled: boolean }> => {
return await apiService.get(API_URLS.SERVICE_ENABLED);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pretty sure I've also named my api variables and fetch functions similarly in my enablement PR. Let's sync to prevent as much naming confusing as possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i have renamed everything from enabled to allowed.

@shahzad31 shahzad31 requested a review from lucasfcosta March 24, 2022 09:23
Copy link
Copy Markdown
Contributor

@lucasfcosta lucasfcosta left a comment

Choose a reason for hiding this comment

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

LGTM, but please let's wait until I've had the chance to merge my PR in the service with @maneta so that I can test this E2E. Then we can merge it before the end of the day today once everything is tested E2E.

Comment thread x-pack/plugins/uptime/public/state/actions/monitor_management.ts Outdated
Co-authored-by: Dominique Clarke <dominique.clarke@elastic.co>
Copy link
Copy Markdown
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Found a few more references to enabled

Comment thread x-pack/plugins/uptime/public/state/effects/index.ts Outdated
Comment thread x-pack/plugins/uptime/public/state/effects/index.ts Outdated
Comment thread x-pack/plugins/uptime/public/state/effects/monitor_management.ts Outdated
Comment thread x-pack/plugins/uptime/public/pages/monitor_management/service_allowed_wrapper.tsx Outdated
Comment thread x-pack/plugins/uptime/public/components/monitor_management/add_monitor_btn.tsx Outdated
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 731 733 +2

Async chunks

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

id before after diff
uptime 753.8KB 755.8KB +2.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
uptime 24.6KB 24.6KB -40.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
uptime 48 49 +1

Total ESLint disabled count

id before after diff
uptime 55 56 +1

History

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

@shahzad31
Copy link
Copy Markdown
Contributor Author

Tested e2e with Lucas and it works fine !!

@shahzad31 shahzad31 merged commit fd1c766 into elastic:main Mar 24, 2022
@shahzad31 shahzad31 deleted the public-beta-check branch March 24, 2022 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants