Skip to content

[APM] Refactoring metrics route#142770

Merged
cauemarcondes merged 5 commits intoelastic:mainfrom
cauemarcondes:apm-aws-lambda-v2
Oct 11, 2022
Merged

[APM] Refactoring metrics route#142770
cauemarcondes merged 5 commits intoelastic:mainfrom
cauemarcondes:apm-aws-lambda-v2

Conversation

@cauemarcondes
Copy link
Copy Markdown
Contributor

Related to #142328.

  • Removes /services/${serviceName}/nodes/ route and redirect it to /services/${serviceName}/metrics/
  • Place components under a Metrics folder.

@cauemarcondes cauemarcondes added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. release_note:skip Skip the PR/issue when compiling release notes v8.6.0 labels Oct 5, 2022
@cauemarcondes cauemarcondes requested a review from a team October 5, 2022 15:47
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@gbamparop gbamparop left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

import { ServiceMetrics } from './service_metrics';
import { ServiceNodeOverview } from './service_node_overview';

export function Metrics() {
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv Oct 8, 2022

Choose a reason for hiding this comment

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

Difficult ask: Can we find a more narrow definition of metrics? We have many types of metrics: transaction metrics, service metrics, runtime metrics, system metrics.

import { isJavaAgentName, isJRubyAgent } from '../../../../common/agent_name';
import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context';
import { ServiceMetrics } from './service_metrics';
import { ServiceNodeOverview } from './service_node_overview';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this jvm specific? I think we should make that clear in the name:

Suggested change
import { ServiceNodeOverview } from './service_node_overview';
import { JvmMetricsOverview } from './jvm_metric_overview';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the same vain, you should also rename the backend APIs and files:

x-pack/plugins/apm/server/routes/service_nodes/route.ts -> x-pack/plugins/apm/server/routes/metrics/route.ts

There's also the API:
GET /internal/apm/services/{serviceName}/serviceNodes which should be renamed to something like:
GET /internal/apm/services/{serviceName}/metrics/nodes

@cauemarcondes cauemarcondes enabled auto-merge (squash) October 11, 2022 13:47
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1316 1318 +2

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.1MB 3.1MB +425.0B

History

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

@cauemarcondes cauemarcondes merged commit b8d58c8 into elastic:main Oct 11, 2022
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants