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

[APM] Migrate Kibana plugins (that depend on APM) to TS project references #81003

Closed
smith opened this issue Oct 19, 2020 · 4 comments · Fixed by #90049
Closed

[APM] Migrate Kibana plugins (that depend on APM) to TS project references #81003

smith opened this issue Oct 19, 2020 · 4 comments · Fixed by #90049
Assignees
Labels
Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture

Comments

@smith
Copy link
Contributor

smith commented Oct 19, 2020

Like #80508, but for APM-specific plugins and their transitive dependencies.

Implementation instructions.

This list is based on looking at APM's optional and required plugins in its kibana.json and manually walking the transitive dependencies. Other dependencies may also exist and can be added here when found.

It is not complete at this time.

plugin migration status
apm_oss (#87676)
apm 🔴
features
licensing
triggers_actions_ui
embeddable
infra (circular dependency here) (#80995)
cloud
usage_collection
task_manager
actions
observability (#89320)
security
ml (#89270)
home
maps (#82194)
url_forwarding (#81177)
data
home
security_oss (#82135)
inspector (#81792)
share (#82051)
kibana_legacy (#80992)
kibana_react
kibana_utils
@smith smith added the Team:APM All issues that need APM UI Team support label Oct 19, 2020
@smith smith self-assigned this Oct 19, 2020
@elasticmachine
Copy link
Contributor

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

smith added a commit to smith/kibana that referenced this issue Oct 20, 2020
Also add missing pieces to kibana_react, as a follow-up to elastic#80992.

References elastic#80508
References elastic#81003
smith added a commit that referenced this issue Oct 27, 2020
Also add missing pieces to kibana_react, as a follow-up to #80992.

References #80508
References #81003
smith added a commit to smith/kibana that referenced this issue Oct 27, 2020
Also add missing pieces to kibana_react, as a follow-up to elastic#80992.

References elastic#80508
References elastic#81003

# Conflicts:
#	tsconfig.json
smith added a commit that referenced this issue Oct 27, 2020
Also add missing pieces to kibana_react, as a follow-up to #80992.

References #80508
References #81003
@sorenlouv sorenlouv added the technical debt Improvement of the software architecture and operational architecture label Nov 4, 2020
@smith smith added the blocked label Nov 20, 2020
@smith
Copy link
Contributor Author

smith commented Jan 7, 2021

data plugin and others have now been updated in #87294 so we can make some more progress on this now.

@smith smith removed the blocked label Jan 7, 2021
smith added a commit to smith/kibana that referenced this issue Jan 7, 2021
smith added a commit that referenced this issue Jan 7, 2021
smith added a commit to smith/kibana that referenced this issue Jan 7, 2021
smith added a commit that referenced this issue Jan 7, 2021
smith added a commit to smith/kibana that referenced this issue Jan 13, 2021
smith added a commit to smith/kibana that referenced this issue Jan 26, 2021
@smith
Copy link
Contributor Author

smith commented Jan 29, 2021

About the infra <> apm circular dependency:

  • APM imports LogStream in public/components/app/TransactionDetails/WaterfallWithSummmary/TransactionTabs.tsx

  • APM imports InfraAppId in public/components/shared/Links/InfraLink.tsx

  • APM imports eui types from infra in typings/common.d.ts

  • Infra imports getTraceUrl in x-pack/plugins/infra/public/components/logging/log_entry_flyout/log_entry_actions_menu.tsx

  • Infra imports APMPluginSetup in x-pack/plugins/infra/server/lib/adapters/framework/adapter_types.ts

It looks like the APMPluginSetup isn't used (it's defining an apm dependency for the server plugin that isn't used anywhere as far as I can tell.)

getTraceUrl is implemented here https://github.com/smith/kibana/blob/e7cbdd3050d65c8e875e7baeac25983cfbb7b733/x-pack/plugins/apm/public/components/shared/Links/apm/ExternalLinks.ts#L7.

Seems like it would be ok go move getTraceUrl to the observability plugin and remove the unused dep. Then infra will not depend on APM.

@weltenwort
Copy link
Member

Thanks, @smith, that sounds like a good short-term solution to break the cycle. If you move that function we can try to projectify infra.

smith added a commit that referenced this issue Feb 2, 2021
smith added a commit to smith/kibana that referenced this issue Feb 2, 2021
References elastic#80508. References elastic#81003.
# Conflicts:
#	x-pack/tsconfig.refs.json
smith added a commit that referenced this issue Feb 2, 2021
References #80508. References #81003.
# Conflicts:
#	x-pack/tsconfig.refs.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants