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

Service map backend links #107317

Merged
merged 20 commits into from
Aug 6, 2021
Merged

Conversation

smith
Copy link
Contributor

@smith smith commented Jul 30, 2021

Add links to backends on service map popovers.

CleanShot 2021-08-01 at 10 31 57@2x

Includes a lot of refactoring of these, but functionally the only difference should be that the backends now show a link to the backends page and stats.

Make the redis icon show up for cache/redis in addition to db/redis.

Fixes #103259

@smith smith added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v7.15.0 labels Jul 30, 2021
@smith smith marked this pull request as ready for review August 1, 2021 15:30
@smith smith requested a review from a team as a code owner August 1, 2021 15:30
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Aug 1, 2021
@elasticmachine
Copy link
Contributor

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

@smith
Copy link
Contributor Author

smith commented Aug 1, 2021

@elasticmachine run the e2e

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

Link to backend detail page is broken

@smith smith requested a review from dgieselaar August 3, 2021 14:42
Comment on lines +47 to +49
if (selectedNodeData[SPAN_TYPE] === 'resource') {
return ResourceContents;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this the RUM agent(s) fall-back? Should we exclude by agent rather than by span type?

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 it's an HTTP request (external/http) it could be called by a RUM agent or any other service, so we could be excluding links that do have backend data.

I suppose we could also make it so these do have stats and links, but then we would have the asymmetry of them being excluded from the dependencies table for RUM services.

Copy link
Member

Choose a reason for hiding this comment

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

not sure I follow, we exclude all RUM calls, regardless of span types in the backend views

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 node data on the client does not have the agent name of the service it was called by, so we would have to make some modifications on the server to get this.

The node data available looks like:

{
  id: ">sqlite"
  label: "sqlite"
  span.destination.service.resource: "sqlite"
  span.subtype: "sqlite"
  span.type: "db"
}

So I think looking for resource is a reasonable way to do this for now.

@dgieselaar
Copy link
Member

re:

Make the redis icon show up for cache/redis in addition to db/redis.

I think this will (should) be fixed by elastic/apm#443

@smith smith requested a review from dgieselaar August 4, 2021 13:52
import { StatsList } from './stats_list';

export function BackendContents({ nodeData }: ContentsProps) {
const { query } = useApmParams('/*');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dgieselaar

if I do useApmParams('/*');, then when I call apmRouter.link below with query I get:

info x-pack/plugins/apm/public/components/app/service_map/Popover/backend_contents.tsx:53:5 - error TS2322: Type '{} | { pageStep?: AgentConfigurationPageStep | undefined; } | { name?: string | undefined; environment?: string | undefined; pageStep?: AgentConfigurationPageStep | undefined; } | ... 5 more ... | ({ ...; } & { ...; })' is not assignable to type '({ rangeFrom?: string | undefined; rangeTo?: string | undefined; environment?: string | undefined; kuery?: string | undefined; } & { comparisonEnabled?: string | undefined; comparisonType?: string | undefined; }) | undefined'.
        Type '{ pageStep?: AgentConfigurationPageStep | undefined; }' has no properties in common with type '{ rangeFrom?: string | undefined; rangeTo?: string | undefined; environment?: string | undefined; kuery?: string | undefined; } & { comparisonEnabled?: string | undefined; comparisonType?: string | undefined; }'.

      53     query,
             ~~~~~

        x-pack/plugins/apm/public/components/routing/home/index.tsx:57:5
           57     query: t.partial({
                  ~~~~~~~~~~~~~~~~~~
           58       rangeFrom: t.string,
              ~~~~~~~~~~~~~~~~~~~~~~~~~~
          ... 
           61       kuery: t.string,
              ~~~~~~~~~~~~~~~~~~~~~~
           62     }),
              ~~~~~~
          The expected type comes from property 'query' which is declared here on type '{ query?: { rangeFrom?: string | undefined; rangeTo?: string | undefined; environment?: string | undefined; kuery?: string | undefined; } | undefined; } & { query?: { comparisonEnabled?: string | undefined; comparisonType?: string | undefined; } | undefined; } & { ...; }'

Since the only think I'm using query for is to pass into link below should I just do useApmParams('/backends/:backendName/overview') here? Or just as up the call below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind I can't do useApmParams('/backends/:backendName/overview'), because then I get no matching route.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...I just used query as any below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...or query as TypeOf< ApmRoutes, '/backends/:backendName/overview' >['query'],

@smith
Copy link
Contributor Author

smith commented Aug 6, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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 4.3MB 4.3MB +280.0B

History

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

@dgieselaar dgieselaar merged commit 43868ac into elastic:master Aug 6, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 6, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 6, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 9, 2021
…-png-pdf-report-type

* 'master' of github.com:elastic/kibana: (392 commits)
  update linting doc (elastic#105748)
  [APM] Various improvements from elastic#104851 (elastic#107726)
  Update dependency @elastic/charts to v33.2.0 (master) (elastic#107842)
  Fix default route link on kibana homepage (elastic#107809)
  [APM] Invalidate trackPageview on route change (elastic#107741)
  Service map backend links (elastic#107317)
  [index patterns] index pattern create modal (elastic#101853)
  [RAC] integrating rbac search strategy with alert table (elastic#107242)
  [Security Solution] Siem signals -> alerts as data field and index aliases (elastic#106049)
  [Metrics UI] Add checkbox to optionally drop partial buckets (elastic#107676)
  [Metrics UI] Fix metric threshold preview regression (elastic#107674)
  Disable Product check in @elastic/elasticsearch-js (elastic#107642)
  [App Search] Migrate Crawler Status Indicator, Crawler Status Banner, and Crawl Request polling (elastic#107603)
  [Security Solution, Lists] Replace legacy imports from 'elasticsearch' package (elastic#107226)
  [maps] asset tracking tutorial (elastic#104552)
  [scripts/build_ts_refs] when using `--clean` initialize caches (elastic#107777)
  Upgrade EUI to v36.1.0 (elastic#107231)
  [RAC] [TGrid] Implements cell actions in the TGrid (elastic#107771)
  Realign cypress/ccs_integration with cypress/integration (elastic#107743)
  Allow optional OSS to X-Pack dependencies (elastic#107432)
  ...

# Conflicts:
#	x-pack/examples/reporting_example/public/application.tsx
#	x-pack/examples/reporting_example/public/components/app.tsx
#	x-pack/plugins/canvas/public/services/legacy/stubs/reporting.ts
#	x-pack/plugins/reporting/common/types.ts
#	x-pack/plugins/reporting/public/lib/reporting_api_client/context.tsx
#	x-pack/plugins/reporting/public/management/mount_management_section.tsx
#	x-pack/plugins/reporting/public/management/report_listing.test.tsx
#	x-pack/plugins/reporting/public/plugin.ts
#	x-pack/plugins/reporting/public/share_context_menu/register_pdf_png_reporting.tsx
#	x-pack/plugins/reporting/server/export_types/printable_pdf/execute_job/index.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Backends UI: Add link from Service Map backend nodes
4 participants