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

[core] docLinks server-side service #95389

Closed
rudolf opened this issue Mar 25, 2021 · 9 comments · Fixed by #123818
Closed

[core] docLinks server-side service #95389

rudolf opened this issue Mar 25, 2021 · 9 comments · Fixed by #123818
Assignees
Labels
enhancement New value added to drive a business result impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc technical debt Improvement of the software architecture and operational architecture

Comments

@rudolf
Copy link
Contributor

rudolf commented Mar 25, 2021

When converting the legacy ui/documentation_links to the new platform we built it as a browser-side only service to match the features of the legacy implementation.

However there are a number of places where we it would be helpful to be able to construct a message on the server-side to point users to our documentation e.g.:

  • HTTP API Error responses
  • Server error logs
  • Deprecation logs

(This will probably also require updating the doc build script)

@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result labels Mar 25, 2021
@pgayvallet
Copy link
Contributor

I think the best place for the links currently living in src/core/public/doc_links/doc_links_service.ts would be a package.
Core's client and server-side service would just consume this list.

The doc build script will have to be updated to also look for the package file where we moved the links definition.

@TinaHeiligers
Copy link
Contributor

What do you think about the package also allowing devs to register new doc links? We have a few different types ATM and would need to account for that and probably have different methods to generate the link fairly easily.

For example,
Kibana-related docs: ${ELASTIC_WEBSITE_URL}guide/en/kibana/${DOC_LINK_VERSION}/dashboard.html
Solutions docs: ${ELASTIC_WEBSITE_URL}guide/en/app-search/${DOC_LINK_VERSION} and ${ELASTIC_WEBSITE_URL}guide/en/security/${DOC_LINK_VERSION}/index.html etc.

The DocLinksService gets the current version from injectedMetadata (const DOC_LINK_VERSION = injectedMetadata.getKibanaBranch()), consumes ELASTIC_WEBSITE_URL (a static string) and then uses those to generate further links.

Is this a crazy idea?

@pgayvallet
Copy link
Contributor

pgayvallet commented Mar 26, 2021

What do you think about the package also allowing devs to register new doc links?

The problem is that the doc build script is basically just regexp-parsing the content of the docLink file to extract the links, interpolate them against the variables, and then check the validity of the URLs (it's as scary as it sounds but that how that works currently), which is why we can't really use any kind of dynamic registration for links at the moment, as it will cause the doc CI job to just miss/ignore these links

As the CI job is meant to be 'very generic', as it gather and validate links from all products of the stack, I don't think this could be improved specifically for Kibana without a lot of work.

cc @gtback

@gtback
Copy link
Member

gtback commented Apr 5, 2021

Thanks for the ping @pgayvallet.

Yes, the current implementation doesn't evaluate any TypeScript code at all, it just munges the contents of the TypeScript file with Perl to pull out links and do some basic substitution. I'm not tied to that implementation, though (if there were some service that spit out a list of links to check as part of CI, or some other way to build a list of docs pages that Kibana links to, I'd be happy to adapt the docs link-checking code).

@lcawl
Copy link
Contributor

lcawl commented Oct 5, 2021

Hopefully this issue would also address the need for hard-coded links in server-side code like the Fleet integration UX (per discussion in #113666 (comment))

@pgayvallet
Copy link
Contributor

That would also allow deprecations registered on the server to use the docLink service instead of hardcoding documentation urls as it's done atm. See #112221 for more context

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Nov 4, 2021
@mshustov mshustov added the technical debt Improvement of the software architecture and operational architecture label Jan 6, 2022
@exalate-issue-sync exalate-issue-sync bot added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Jan 11, 2022
@pgayvallet
Copy link
Contributor

pgayvallet commented Jan 19, 2022

So, we'll want to:

Also, the core team will NOT take ownership of the @kbn/doc-links package , to avoid unnecessary reviews when teams are adding links.

@pgayvallet pgayvallet self-assigned this Jan 19, 2022
@gtback
Copy link
Member

gtback commented Jan 19, 2022

Thanks, @pgayvallet . One issue we've run into before is if there's a "stub file" in one of the old locations, it won't check the links in the newer location, unless we add new paths to the top of the if/else chain in build_docs.pl. Let me know if I can help with any of those changes.

@pgayvallet
Copy link
Contributor

it won't check the links in the newer location, unless we add new paths to the top of the if/else chain in build_docs.pl

Yea, that was what I was planning to try. I'll make sure to ping you if I need anything, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants