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

[APM] Add feature flag for not available apm schema #158911

Merged
merged 6 commits into from
Jun 13, 2023
Merged

[APM] Add feature flag for not available apm schema #158911

merged 6 commits into from
Jun 13, 2023

Conversation

MiriamAparicio
Copy link
Contributor

Part of #153776

How to test:

  • change default value in x-pack/plugins/apm/common/apm_feature_flags.ts

@MiriamAparicio MiriamAparicio added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes v8.9.0 labels Jun 2, 2023
@MiriamAparicio MiriamAparicio requested a review from a team as a code owner June 2, 2023 11:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@apmmachine
Copy link
Contributor

🤖 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!)

sorenlouv
sorenlouv reviewed Jun 5, 2023
View reviewed changes
x-pack/plugins/apm/server/routes/fleet/route.ts Outdated
function throwNotFoundIfApmSchemaNotAvailable(
featureFlags: ApmFeatureFlags
): void {
if (!featureFlags.schemaAvailable) {
Copy link
Member

@sorenlouv sorenlouv Jun 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was never fond of the name "schemaAvailable". It seems so vague.

@ogupte Isn't this about fleet migration? Could we call this featureFlags.isFleetMigrationAvailable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If so, should we also block the other request in x-pack/plugins/apm/server/routes/fleet/route.ts?
@sqren @ogupte

Copy link
Member

Choose a reason for hiding this comment

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

I think we also want to restrict access to POST /internal/apm/fleet/cloud_apm_package_policy if that's the one you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The MigrationToFleetAvailable already exist, should I use this one in this case? I will use the schema for the UI tab then, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@ogupte WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

The feature in question is the fleet migration page in settings, which has the title APM Schema (not sure why exactly). i agree that it should use one feature flag like showFleetMigration or something. There's already already a config that exists to enable it but it doesn't hide it from the UI.

@MiriamAparicio MiriamAparicio mentioned this pull request Jun 5, 2023
Closed
10 tasks
MiriamAparicio
MiriamAparicio commented Jun 6, 2023
View reviewed changes
x-pack/plugins/apm/common/apm_feature_flags.ts Outdated
@@ -49,7 +49,7 @@ const apmFeatureFlagMap = {
default: true,
type: t.boolean,
},
[ApmFeatureFlagName.SchemaAvailable]: {
[ApmFeatureFlagName.FleetMigrationAvailable]: {
Copy link
Contributor Author

@MiriamAparicio MiriamAparicio Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

My mistake I just saw that this already exist
I'll revert the commit

@MiriamAparicio MiriamAparicio force-pushed the 153776-serverless-modifications-part-III branch from efb1bed to 974f05e Compare June 6, 2023 14:42
@MiriamAparicio MiriamAparicio force-pushed the 153776-serverless-modifications-part-III branch from 974f05e to 15e2984 Compare June 6, 2023 14:57
@MiriamAparicio MiriamAparicio force-pushed the 153776-serverless-modifications-part-III branch from 9da0033 to 0f4d872 Compare June 12, 2023 14:55
ogupte
ogupte reviewed Jun 12, 2023
View reviewed changes
x-pack/plugins/apm/server/routes/fleet/route.ts
Copy link
Contributor

@ogupte ogupte Jun 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

should we give the same check (throwNotFoundIfApmSchemaNotAvailable) in GET /internal/apm/fleet/migration_check? It's an API that's call from 2 places:

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #39 / InfraOps App Metrics UI Hosts View #Single host Flyout "after each" hook for "should navigate to APM services after click"
  • [job] [logs] FTR Configs #39 / InfraOps App Metrics UI Hosts View #Single host Flyout should navigate to APM services after click

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
apm 3.6MB 3.6MB +16.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 491 495 +4
total +6

History

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

@MiriamAparicio MiriamAparicio merged commit bda9a56 into elastic:main Jun 13, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 13, 2023
@MiriamAparicio MiriamAparicio deleted the 153776-serverless-modifications-part-III branch June 13, 2023 07:54
@MiriamAparicio MiriamAparicio mentioned this pull request Jun 13, 2023
Merged
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 13, 2023
@jloleysens
Merge branch 'main' into do-not-do-version-check-on-serverless
a67fd82 
* main: (199 commits)
  [Lens] Add custom formatter within the Lens editor (elastic#158468)
  [Uptime] Hide app if no data is available (elastic#159118)
  [CodeEditor] Add grok highlighting (elastic#159334)
  Expose decoded cloudId components from the cloud plugin's contract (elastic#159442)
  [Profiling] Use collector and symbolizer integrations in the add data page (elastic#159481)
  [Infrastructure UI] Hosts View: Unified Search bar with auto-refresh enabled (elastic#157011)
  [APM] Add feature flag for not available apm schema (elastic#158911)
  [Lens] Remove deprecated componentWillReceiveProps usage (elastic#159502)
  [api-docs] 2023-06-13 Daily api_docs build (elastic#159536)
  [data views] Use versioned router for REST routes (elastic#158608)
  [maps] fix geo line source not loaded unless maps application is opened (elastic#159432)
  [Enterprise Search][Search application]Fix Create Api key url (elastic#159519)
  [Security Solution] Increase timeout for indexing hosts (elastic#159518)
  dashboard Reset button disable (elastic#159430)
  [Security Solution] Unskip Endpoint API tests after package fix (elastic#159484)
  [Synthetics] adjust alert timing (elastic#159511)
  [ResponseOps][rule registry] Remove usages of `refresh: true` (elastic#159252)
  Revert "Remove unused package (elastic#158597)"
  [Serverless] Adding config to disable authentication on task manager background worker utilization API (elastic#159505)
  Remove unused package (elastic#158597)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogupte ogupte ogupte approved these changes

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@MiriamAparicio @elasticmachine @apmmachine @kibana-ci @sorenlouv @ogupte @kibanamachine