-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Move apm tutorial from apm plugin into apm_oss plugin #66432
Move apm tutorial from apm plugin into apm_oss plugin #66432
Conversation
Pinging @elastic/apm-ui (Team:apm) |
* under the License. | ||
*/ | ||
|
||
import { ApmOssPlugin } from './plugin'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add the public plugin boilerplate? It looks like this is just a server side thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flash1293 I think it's ok cause we have one asset here: src/plugins/apm_oss/public/assets/apm.png
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point, missed that.
# Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with the following config:
xpack.apm.enabled: false
xpack.cloud.enabled: false
and this looks mostly good to me (besides a bunch of nits).
@@ -4,7 +4,7 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
import { APICaller } from 'kibana/server'; | |||
import { APICaller } from 'src/core/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems like it's not necessary
@@ -4,7 +4,7 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
import { merge } from 'lodash'; | |||
import { Logger, CallAPIOptions } from 'kibana/server'; | |||
import { Logger, CallAPIOptions } from 'src/core/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems like it's not necessary
@@ -21,7 +21,8 @@ import yaml from 'js-yaml'; | |||
import { Client } from 'elasticsearch'; | |||
import { argv } from 'yargs'; | |||
import { promisify } from 'util'; | |||
import { Logger } from 'kibana/server'; | |||
// eslint-disable-next-line @kbn/eslint/no-restricted-paths | |||
import { Logger } from 'src/core/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems like it's not necessary
@@ -11,7 +11,7 @@ import url from 'url'; | |||
import rison, { RisonValue } from 'rison-node'; | |||
import { useLocation } from '../../../../hooks/useLocation'; | |||
import { getTimepickerRisonData } from '../rison_helpers'; | |||
import { APM_STATIC_INDEX_PATTERN_ID } from '../../../../../common/index_pattern_constants'; | |||
import { APM_STATIC_INDEX_PATTERN_ID } from '../../../../../../../../src/plugins/apm_oss/common/index_pattern_constants'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a small thing, but could you re-export this from apm_oss/public/index.ts
? Same like here in the data
plugin: https://github.com/flash1293/kibana/blob/15c0644c9756eb58e86cef7e7cd1ce8266bc7ed0/src/plugins/data/public/index.ts#L443
It's not done right everywhere, but we try to get there.
.i18nrc.json
Outdated
@@ -56,7 +56,8 @@ | |||
"visTypeVislib": "src/plugins/vis_type_vislib", | |||
"visTypeXy": "src/plugins/vis_type_xy", | |||
"visualizations": "src/plugins/visualizations", | |||
"visualize": "src/plugins/visualize" | |||
"visualize": "src/plugins/visualize", | |||
"apm": "src/plugins/apm_oss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change that to apmOss
to be consistent with other prefixes?
11c4a2c
to
ff2345e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think we can run this by the APM team
# Conflicts: # x-pack/plugins/apm/server/tutorial/instructions/apm_agent_instructions.ts
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ent/add-support-in-url-for-hidden-toggle * 'master' of github.com:elastic/kibana: (49 commits) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) [Logs UI] Restore call to `UsageCollector.countLogs` (elastic#67051) Remove unused license check result from LP Security plugin (elastic#66966) [Saved Objects] adds support for including hidden types in saved objects client (elastic#66879) [Discover] Deangularize timechart header (elastic#66532) [Discover] Improve and unskip a11y context view test (elastic#66959) [SIEM] Refactor Timeline.timelineType draft to Timeline.status draft (elastic#66864) ... # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/helpers/home.helpers.ts
Introduced in elastic#66432 and I have a more thorough PR to prevent imports in server from public in elastic#67149 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
* master: [apm] Annotation API documentation (elastic#65963) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) [Logs UI] Restore call to `UsageCollector.countLogs` (elastic#67051) Remove unused license check result from LP Security plugin (elastic#66966)
* master: (21 commits) [Alerting] Hides the `alert` SavedObjects type (elastic#66719) skip flaky suite (elastic#66869) fix visual baseline tests [kbn/optimizer] require fsevents on macos (elastic#67147) [APM] Fix obscured service map connections (elastic#67129) [apm] Annotation API documentation (elastic#65963) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) ...
* Move apm tutorial from apm plugin into apm_oss plugin Closes elastic#65629 * Fix types issues and some paths * Add unregisterTutorial to tutorials_registry.mock * Add apm path to .i18nrc.json to fix internationalization error * Rename apm path in .i18nrc.json into apmOss and revert some imports paths # Conflicts: # x-pack/plugins/apm/server/tutorial/instructions/apm_agent_instructions.ts
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
2 similar comments
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…67179) * Move apm tutorial from apm plugin into apm_oss plugin (#66432) * Move apm tutorial from apm plugin into apm_oss plugin Closes #65629 * Fix types issues and some paths * Add unregisterTutorial to tutorials_registry.mock * Add apm path to .i18nrc.json to fix internationalization error * Rename apm path in .i18nrc.json into apmOss and revert some imports paths # Conflicts: # x-pack/plugins/apm/server/tutorial/instructions/apm_agent_instructions.ts * Fixed some eslint issues * Fix apmOSSPluginSetupMock
* Move apm tutorial from apm plugin into apm_oss plugin Closes elastic#65629 * Fix types issues and some paths * Add unregisterTutorial to tutorials_registry.mock * Add apm path to .i18nrc.json to fix internationalization error * Rename apm path in .i18nrc.json into apmOss and revert some imports paths # Conflicts: # x-pack/plugins/apm/server/plugin.ts # x-pack/plugins/apm/server/tutorial/elastic_cloud.ts # x-pack/plugins/apm/server/tutorial/envs/on_prem.ts # x-pack/plugins/apm/server/tutorial/index.ts # x-pack/plugins/apm/server/tutorial/instructions/apm_agent_instructions.ts # x-pack/plugins/apm/server/tutorial/instructions/apm_server_instructions.ts # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
…67619) * Move apm tutorial from apm plugin into apm_oss plugin (#66432) * Move apm tutorial from apm plugin into apm_oss plugin Closes #65629 * Fix types issues and some paths * Add unregisterTutorial to tutorials_registry.mock * Add apm path to .i18nrc.json to fix internationalization error * Rename apm path in .i18nrc.json into apmOss and revert some imports paths # Conflicts: # x-pack/plugins/apm/server/plugin.ts # x-pack/plugins/apm/server/tutorial/elastic_cloud.ts # x-pack/plugins/apm/server/tutorial/envs/on_prem.ts # x-pack/plugins/apm/server/tutorial/index.ts # x-pack/plugins/apm/server/tutorial/instructions/apm_agent_instructions.ts # x-pack/plugins/apm/server/tutorial/instructions/apm_server_instructions.ts # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json * Fix issues produced by mistake through merge conflict * Fix some eslint issues * Fix type check * Remove unused translations
Closes #65629
Summary
apm
toapm_oss
pluginapm_oss
plugin to display the picture in apm tutorialtutorials_registry.ts
to let the apm plugin re-register the tutorialxpack
from their namesChecklist
Delete any items that are not applicable to this PR.
For maintainers