[APM] Fix service maps when root transaction has a parent.id by crespocarlos · Pull Request #212998 · elastic/kibana

crespocarlos · 2025-03-03T23:45:54Z

Summary

Warning

This can only be merged after https://github.com/elastic/elasticsearch-serverless/pull/3579. Service map tests running against serverless will fail until the aforementioned PR gets merged and deployed. It should happen Thursday/Friday next week (13/14 Feb)

Fixes a bug on the service map causing it not to build the paths when the root transaction of the trace had a parent.id

Global service map

Before	After

Ad service map

Before	After

How to test

Connect to an edge-obl cluster
Navigate the Application > Services inventory > Service Map
Inspect the service map of the Ad service

crespocarlos · 2025-03-04T07:51:18Z

...ns/observability/plugins/apm/server/routes/service_map/fetch_service_paths_from_trace_ids.ts

-                    }
-
-
-                    continue;


This was the issue.

crespocarlos · 2025-03-04T07:59:16Z

...ns/observability/plugins/apm/server/routes/service_map/fetch_service_paths_from_trace_ids.ts

+              while (parentId != null && !parentId.equals(eventId)) {
+                def parent = context.eventsById.get(parentId);
+                if (parent == null || visited.contains(parentId)) {
+                  break;
+                }
+
+                pathStack.push(parentId);
+                visited.add(parentId);
+                parentId = parent['parent.id'];


This now builds a stack from the current event until its root transaction, ignoring whether the root transaction has or not a parent.id

crespocarlos · 2025-03-04T08:00:33Z

...ns/observability/plugins/apm/server/routes/service_map/fetch_service_paths_from_trace_ids.ts

+              }
+
+              // pop the stack starting from the root to current event to build the path
+              while (!pathStack.isEmpty()) {


With the path from current event to root transaction defined, it iterates over the stack from the root transaction up until the current event.

crespocarlos · 2025-03-04T08:01:28Z

...ns/observability/plugins/apm/server/routes/service_map/fetch_service_paths_from_trace_ids.ts

-              while (!stack.isEmpty()) {
-                def currentEventId = stack.pop();
-                def event = context.eventsById.get(currentEventId);
+                 def basePath = new ArrayList();


basePath is cumulative. Each iteration appends a service and a destination, if found, to it

elasticmachine · 2025-03-04T12:09:34Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

crespocarlos · 2025-03-04T12:13:38Z

@elasticmachine merge upstream

…ction-fix

jennypavlova

LGTM 💯 I can see the service map of the ad service 🎉 Thank you for fixing that!

src/platform/packages/shared/kbn-apm-synthtrace-client/src/lib/dsl/service_map.ts

MiriamAparicio

LGTM, thanks for all the detailed explanation of the issue

achyutjhunjhunwala

Synthtrace changes looks good

crespocarlos · 2025-03-10T08:42:26Z

@elasticmachine merge upstream

…ction-fix

crespocarlos · 2025-03-10T11:01:11Z

/ci

elasticmachine · 2025-03-10T12:45:13Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: abd1ef6

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #45 / dashboard app - esql controls dashboard - add a value type ES|QL control should update the Lens chart accordingly

Metrics [docs]

✅ unchanged

History

💚 Build #282043 succeeded abd1ef6
💔 Build #280706 failed 0d94bb6
💔 Build #280548 failed 80b56a1

kibanamachine · 2025-03-10T14:58:13Z

Starting backport for target branches: 8.16, 8.17, 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13767785674

…#212998) fixes [212931](elastic#212931) ## Summary >[!WARNING] > This can only be merged after elastic/elasticsearch-serverless#3579. Service map tests running against serverless will fail until the aforementioned PR gets merged and deployed. It should happen Thursday/Friday next week (13/14 Feb) Fixes a bug on the service map causing it not to build the paths when the root transaction of the trace had a `parent.id` Global service map | Before | After | |--------|------| |<img width="599" alt="image" src="https://github.com/user-attachments/assets/cce72dea-822b-46e2-938c-65ec3f4600da" />|<img width="599" alt="image" src="https://github.com/user-attachments/assets/68b344fb-2e75-46b8-9401-9fce08bfb860" />| `Ad` service map | Before | After | |--------|------| |<img width="1469" alt="image" src="https://github.com/user-attachments/assets/e960a390-4a38-43d5-9445-853ced34bb15" />|<img width="1459" alt="image" src="https://github.com/user-attachments/assets/566e3cf0-3805-4bf2-a511-fffed3480332" />| ### How to test - Connect to an `edge-obl` cluster - Navigate the Application > Services inventory > Service Map - Inspect the service map of the `Ad` service --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 4a67b8b)

kibanamachine · 2025-03-10T15:04:58Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.16	Backport failed because of merge conflicts You might need to backport the following PRs to 8.16: - [ci] Trigger VM image rebuild for cache rebuild only (#213497)
✅	8.17
✅	8.18
✅	8.x
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 212998

Questions ?

Please refer to the Backport tool documentation

…#212998) fixes [212931](elastic#212931) ## Summary >[!WARNING] > This can only be merged after elastic/elasticsearch-serverless#3579. Service map tests running against serverless will fail until the aforementioned PR gets merged and deployed. It should happen Thursday/Friday next week (13/14 Feb) Fixes a bug on the service map causing it not to build the paths when the root transaction of the trace had a `parent.id` Global service map | Before | After | |--------|------| |<img width="599" alt="image" src="https://github.com/user-attachments/assets/cce72dea-822b-46e2-938c-65ec3f4600da" />|<img width="599" alt="image" src="https://github.com/user-attachments/assets/68b344fb-2e75-46b8-9401-9fce08bfb860" />| `Ad` service map | Before | After | |--------|------| |<img width="1469" alt="image" src="https://github.com/user-attachments/assets/e960a390-4a38-43d5-9445-853ced34bb15" />|<img width="1459" alt="image" src="https://github.com/user-attachments/assets/566e3cf0-3805-4bf2-a511-fffed3480332" />| ### How to test - Connect to an `edge-obl` cluster - Navigate the Application > Services inventory > Service Map - Inspect the service map of the `Ad` service --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 4a67b8b) # Conflicts: # x-pack/test/api_integration/deployment_agnostic/apis/observability/apm/service_maps/service_maps.spec.ts

crespocarlos · 2025-03-10T15:24:09Z

💚 All backports created successfully

Status	Branch	Result
✅	8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…212998) (#213765) # Backport This will backport the following commits from `main` to `8.x`: - [[APM] Fix service maps when root transaction has a parent.id (#212998)](#212998)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>

…212998) (#213766) # Backport This will backport the following commits from `main` to `9.0`: - [[APM] Fix service maps when root transaction has a parent.id (#212998)](#212998)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>

…212998) (#213764) # Backport This will backport the following commits from `main` to `8.18`: - [[APM] Fix service maps when root transaction has a parent.id (#212998)](#212998)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>

…212998) (#213768) # Backport This will backport the following commits from `main` to `8.16`: - [[APM] Fix service maps when root transaction has a parent.id (#212998)](#212998)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

kibanamachine · 2025-03-11T17:48:37Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2025-03-12T17:48:51Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

…212998) (#213763) # Backport This will backport the following commits from `main` to `8.17`: - [[APM] Fix service maps when root transaction has a parent.id (#212998)](#212998)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…#212998) fixes [212931](elastic#212931) ## Summary >[!WARNING] > This can only be merged after elastic/elasticsearch-serverless#3579. Service map tests running against serverless will fail until the aforementioned PR gets merged and deployed. It should happen Thursday/Friday next week (13/14 Feb) Fixes a bug on the service map causing it not to build the paths when the root transaction of the trace had a `parent.id` Global service map | Before | After | |--------|------| |<img width="599" alt="image" src="https://github.com/user-attachments/assets/cce72dea-822b-46e2-938c-65ec3f4600da" />|<img width="599" alt="image" src="https://github.com/user-attachments/assets/68b344fb-2e75-46b8-9401-9fce08bfb860" />| `Ad` service map | Before | After | |--------|------| |<img width="1469" alt="image" src="https://github.com/user-attachments/assets/e960a390-4a38-43d5-9445-853ced34bb15" />|<img width="1459" alt="image" src="https://github.com/user-attachments/assets/566e3cf0-3805-4bf2-a511-fffed3480332" />| ### How to test - Connect to an `edge-obl` cluster - Navigate the Application > Services inventory > Service Map - Inspect the service map of the `Ad` service --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Fix service maps when root transaction has a parent.id

80b56a1

crespocarlos added backport:prev-minor release_note:fix Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Mar 3, 2025

crespocarlos commented Mar 4, 2025

View reviewed changes

Small adjustments

4cd8461

crespocarlos marked this pull request as ready for review March 4, 2025 12:09

crespocarlos requested a review from a team March 4, 2025 12:09

crespocarlos requested a review from a team as a code owner March 4, 2025 12:09

Merge branch 'main' into 212931-service-map-without-parentless-transa…

0d94bb6

…ction-fix

jennypavlova self-requested a review March 4, 2025 12:40

jennypavlova approved these changes Mar 4, 2025

View reviewed changes

src/platform/packages/shared/kbn-apm-synthtrace-client/src/lib/dsl/service_map.ts Show resolved Hide resolved

MiriamAparicio approved these changes Mar 4, 2025

View reviewed changes

achyutjhunjhunwala approved these changes Mar 5, 2025

View reviewed changes

Merge branch 'main' into 212931-service-map-without-parentless-transa…

abd1ef6

…ction-fix

crespocarlos merged commit 4a67b8b into elastic:main Mar 10, 2025
9 checks passed

crespocarlos deleted the 212931-service-map-without-parentless-transaction-fix branch March 10, 2025 14:57

kibanamachine added the v9.1.0 label Mar 10, 2025

kibanamachine mentioned this pull request Mar 10, 2025

[8.17] [APM] Fix service maps when root transaction has a parent.id (#212998) #213763

Merged

kibanamachine mentioned this pull request Mar 10, 2025

[8.18] [APM] Fix service maps when root transaction has a parent.id (#212998) #213764

Merged

kibanamachine mentioned this pull request Mar 10, 2025

[8.x] [APM] Fix service maps when root transaction has a parent.id (#212998) #213765

Merged

kibanamachine mentioned this pull request Mar 10, 2025

[9.0] [APM] Fix service maps when root transaction has a parent.id (#212998) #213766

Merged

crespocarlos mentioned this pull request Mar 10, 2025

[8.16] [APM] Fix service maps when root transaction has a parent.id (#212998) #213768

Merged

kibanamachine added the v8.19.0 label Mar 10, 2025

kibanamachine added the v9.0.0 label Mar 10, 2025

kibanamachine added the v8.18.0 label Mar 10, 2025

kibanamachine added the v8.16.6 label Mar 10, 2025

kibanamachine mentioned this pull request Mar 10, 2025

[ResponseOps][Connectors] Fix bug with OAuth form in the ServiceNow connector #213658

Merged

1 task

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 11, 2025

kibanamachine added v8.17.4 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 13, 2025

Conversation

crespocarlos commented Mar 3, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Uh oh!

crespocarlos Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

crespocarlos Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

crespocarlos Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

crespocarlos Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 4, 2025

Uh oh!

crespocarlos commented Mar 4, 2025

Uh oh!

jennypavlova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MiriamAparicio left a comment

Choose a reason for hiding this comment

Uh oh!

achyutjhunjhunwala left a comment

Choose a reason for hiding this comment

Uh oh!

crespocarlos commented Mar 10, 2025

Uh oh!

crespocarlos commented Mar 10, 2025

Uh oh!

elasticmachine commented Mar 10, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

History

Uh oh!

Uh oh!

kibanamachine commented Mar 10, 2025

Uh oh!

kibanamachine commented Mar 10, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

crespocarlos commented Mar 10, 2025

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Mar 11, 2025

Uh oh!

kibanamachine commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

crespocarlos commented Mar 3, 2025 •

edited by kibanamachine

Loading

crespocarlos Mar 4, 2025 •

edited

Loading