-
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
[ProfilingxAPM] Link APM from Profiling UI #180677
[ProfilingxAPM] Link APM from Profiling UI #180677
Conversation
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
A documentation preview will be available soon. Request a new doc build by commenting
If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here. |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
@cauemarcondes - suggested copy for the accordion, using the style-for-forms accordion as an inspiration: Tittle: Distributed Tracing Correlation Sub-title: A curated view of APM services and transactions that call this function @mdbirnstiehl please review. FYA @athre0z @christos68k |
Also, as discussed in Slack, the list should be sorted by the sample count in desc order, with the ability to filter by both service.name and transaction.name. Thank you |
.../plugins/observability_solution/apm/public/components/app/transaction_details_link/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability_solution/profiling/server/routes/apm.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability_solution/profiling/public/services.ts
Outdated
Show resolved
Hide resolved
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.
Looks good on my side. Thanks!
}), | ||
truncateText: true, | ||
sortable: true, | ||
render(_, { serviceName, transactionName }) { |
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.
The API doesn't return the transaction type? it could be an edge case but a service could have a 2 same transaction names but with different transaction type
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.
Right now fetching the transaction type would mean making more ES queries for each service and transaction just to fetch the transaction types associated. At this point, I don't think it's worth doing it. Let's just make APM pick the first transaction type it finds based on the transaction name.
…a into profiling-apm-link-2
x-pack/plugins/observability_solution/profiling/server/routes/apm.ts
Outdated
Show resolved
Hide resolved
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 didn't test it manually though cause I don't have profiling data locally
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
…es (#182001) Related to #180677 closes #181886 There's still a small difference on the CO2 values, but that's due to different formats returned by ES apis: ``` TopN Functions "self_annual_co2_tons": 0.0068555964801996295 Flamegraph "AnnualCO2TonsInclusive": [ 0.0069, ``` After: <img width="1765" alt="Screenshot 2024-04-29 at 16 34 57" src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188"> <img width="1788" alt="Screenshot 2024-04-29 at 16 35 03" src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67">
…es (elastic#182001) Related to elastic#180677 closes elastic#181886 There's still a small difference on the CO2 values, but that's due to different formats returned by ES apis: ``` TopN Functions "self_annual_co2_tons": 0.0068555964801996295 Flamegraph "AnnualCO2TonsInclusive": [ 0.0069, ``` After: <img width="1765" alt="Screenshot 2024-04-29 at 16 34 57" src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188"> <img width="1788" alt="Screenshot 2024-04-29 at 16 35 03" src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67"> (cherry picked from commit cb77c2d)
…nt values (#182001) (#182091) # Backport This will backport the following commits from `main` to `8.14`: - [[Profiling] Differential views based on same data show different values (#182001)](#182001) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Cauê Marcondes","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-04-30T09:47:33Z","message":"[Profiling] Differential views based on same data show different values (#182001)\n\nRelated to https://github.com/elastic/kibana/pull/180677\r\ncloses https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a small difference on the CO2 values, but that's due to\r\ndifferent formats returned by ES apis:\r\n```\r\nTopN Functions\r\n\"self_annual_co2_tons\": 0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\": [\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\" alt=\"Screenshot 2024-04-29 at 16 34 57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35 03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","ci:project-deploy-observability","v8.14.0","v8.15.0"],"title":"[Profiling] Differential views based on same data show different values","number":182001,"url":"https://github.com/elastic/kibana/pull/182001","mergeCommit":{"message":"[Profiling] Differential views based on same data show different values (#182001)\n\nRelated to https://github.com/elastic/kibana/pull/180677\r\ncloses https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a small difference on the CO2 values, but that's due to\r\ndifferent formats returned by ES apis:\r\n```\r\nTopN Functions\r\n\"self_annual_co2_tons\": 0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\": [\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\" alt=\"Screenshot 2024-04-29 at 16 34 57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35 03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/182001","number":182001,"mergeCommit":{"message":"[Profiling] Differential views based on same data show different values (#182001)\n\nRelated to https://github.com/elastic/kibana/pull/180677\r\ncloses https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a small difference on the CO2 values, but that's due to\r\ndifferent formats returned by ES apis:\r\n```\r\nTopN Functions\r\n\"self_annual_co2_tons\": 0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\": [\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\" alt=\"Screenshot 2024-04-29 at 16 34 57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35 03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541"}}]}] BACKPORT--> Co-authored-by: Cauê Marcondes <[email protected]>
…es (elastic#182001) Related to elastic#180677 closes elastic#181886 There's still a small difference on the CO2 values, but that's due to different formats returned by ES apis: ``` TopN Functions "self_annual_co2_tons": 0.0068555964801996295 Flamegraph "AnnualCO2TonsInclusive": [ 0.0069, ``` After: <img width="1765" alt="Screenshot 2024-04-29 at 16 34 57" src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188"> <img width="1788" alt="Screenshot 2024-04-29 at 16 35 03" src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67">
closes #178719
A new ES API has been created to support linking APM from the Profiling UI. It's called
topN/functions
. The new API allows grouping fields. So we first fetch functions grouping byservice.name
and when the user opens the APM Transactions we make another request grouping bytransaction.name
.A new Advanced setting was created to toggle the old API on (fetch functions from Stacktraces API): It's turned off by default.
When there are services on the selected function:
*If we cannot find the transaction, we show
N/A
.When there are no services on the selected function:
*hide the APM transactions section
--
Performance boost:
The new API is faster than the Stacktraces API, especially because there's no logic on the Kibana side.
Stacktraces API:
TopN/Functions API: