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.

Sign up for GitHub

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

Jump to bottom

[APM] Fix service maps #192859

Merged
merged 10 commits into from
Nov 11, 2024
Merged

[APM] Fix service maps #192859

merged 10 commits into from
Nov 11, 2024

Conversation

crespocarlos
Copy link
Contributor

@crespocarlos crespocarlos commented Sep 13, 2024 •
edited by kibanamachine
Loading

Summary

image

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@crespocarlos crespocarlos mentioned this pull request Sep 13, 2024
Closed
MiriamAparicio
MiriamAparicio reviewed Nov 5, 2024
View reviewed changes
...gins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts Outdated
connection.sourceData,
connection.targetData,
]);
// .concat(serviceNodes);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I think it can be removed

@MiriamAparicio
Copy link
Contributor

MiriamAparicio commented Nov 5, 2024 •
edited
Loading

I tested the fix out of the scenario with edge-lite local cluster and works fine
Also tested with otel-poc-apm and otel-demo 👍

@MiriamAparicio MiriamAparicio force-pushed the 192857-service-map-possible-fix branch from 04e73ed to aa66780 Compare November 5, 2024 17:31
@MiriamAparicio MiriamAparicio marked this pull request as ready for review November 5, 2024 17:32
@MiriamAparicio MiriamAparicio requested a review from a team as a code owner November 5, 2024 17:32
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Nov 5, 2024
@elasticmachine
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2024

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@MiriamAparicio MiriamAparicio added v8.17.0 backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes labels Nov 5, 2024
MiriamAparicio
MiriamAparicio approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@MiriamAparicio MiriamAparicio 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 the fix
LGTM 🌟

@MiriamAparicio MiriamAparicio added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Nov 6, 2024
jennypavlova
jennypavlova approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@MiriamAparicio MiriamAparicio force-pushed the 192857-service-map-possible-fix branch from f7d1965 to dadaa6c Compare November 6, 2024 13:18
cauemarcondes
cauemarcondes approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LTGM, some nits.

...gins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts Outdated
@@ -51,7 +51,7 @@ function addMessagingConnections(
const newConnections = messagingDestinations
.map((node) => {
const matchedService = discoveredServices.find(
({ from }) =>
({ from, to }) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you using this to?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, it's not used there

...gins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts Outdated
Comment on lines 130 to 136
const externalNodes = [
...new Set(
allNodes.filter(
(node) => SPAN_DESTINATION_SERVICE_RESOURCE in node
) as ExternalConnectionNode[]
),
];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const externalNodes = [
...new Set(
allNodes.filter(
(node) => SPAN_DESTINATION_SERVICE_RESOURCE in node
) as ExternalConnectionNode[]
),
];
const externalNodes = Array.from(new Set(
allNodes.filter(
(node) => SPAN_DESTINATION_SERVICE_RESOURCE in node
) as ExternalConnectionNode[]
)
);

@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 11, 2024 •
edited
Loading

💚 Build Succeeded

  • Buildkite Build
  • Commit: c33dc51
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-192859-c33dc51a257b

Metrics [docs]

✅ unchanged

History

@MiriamAparicio MiriamAparicio merged commit c19551f into elastic:main Nov 11, 2024
25 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@MiriamAparicio MiriamAparicio changed the title [APM] Attempt to fix service maps [APM] Fix service maps Nov 11, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 11, 2024
@crespocarlos
[APM] Attempt to fix service maps (elastic#192859)
ad90245 
## Summary

<img width="1029" alt="image"
src="https://github.com/user-attachments/assets/155d5dff-dc9e-44c7-a5ad-532867088763">

---------

Co-authored-by: miriam.aparicio <[email protected]>
Co-authored-by: Miriam <[email protected]>
(cherry picked from commit c19551f)
@kibanamachine kibanamachine mentioned this pull request Nov 11, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

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

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Nov 11, 2024
Merged
5 tasks
kibanamachine added a commit that referenced this pull request Nov 11, 2024
@kibanamachine @crespocarlos
[8.x] [APM] Attempt to fix service maps (#192859) (#199615)
54c8329 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[APM] Attempt to fix service maps
(#192859)](#192859)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Carlos
Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-11T10:52:39Z","message":"[APM]
Attempt to fix service maps (#192859)\n\n## Summary\r\n\r\n<img
width=\"1029\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/155d5dff-dc9e-44c7-a5ad-532867088763\">\r\n\r\n---------\r\n\r\nCo-authored-by:
miriam.aparicio <[email protected]>\r\nCo-authored-by: Miriam
<[email protected]>","sha":"c19551f89a039c26ece79b505b4d9ff7ad729f47","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.17.0"],"title":"[APM]
Attempt to fix service
maps","number":192859,"url":"https://github.com/elastic/kibana/pull/192859","mergeCommit":{"message":"[APM]
Attempt to fix service maps (#192859)\n\n## Summary\r\n\r\n<img
width=\"1029\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/155d5dff-dc9e-44c7-a5ad-532867088763\">\r\n\r\n---------\r\n\r\nCo-authored-by:
miriam.aparicio <[email protected]>\r\nCo-authored-by: Miriam
<[email protected]>","sha":"c19551f89a039c26ece79b505b4d9ff7ad729f47"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192859","number":192859,"mergeCommit":{"message":"[APM]
Attempt to fix service maps (#192859)\n\n## Summary\r\n\r\n<img
width=\"1029\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/155d5dff-dc9e-44c7-a5ad-532867088763\">\r\n\r\n---------\r\n\r\nCo-authored-by:
miriam.aparicio <[email protected]>\r\nCo-authored-by: Miriam
<[email protected]>","sha":"c19551f89a039c26ece79b505b4d9ff7ad729f47"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Carlos Crespo <[email protected]>
tkajtoch pushed a commit to tkajtoch/kibana that referenced this pull request Nov 12, 2024
@crespocarlos @MiriamAparicio @tkajtoch
[APM] Attempt to fix service maps (elastic#192859)
ab7a950 
## Summary

<img width="1029" alt="image"
src="https://github.com/user-attachments/assets/155d5dff-dc9e-44c7-a5ad-532867088763">

---------

Co-authored-by: miriam.aparicio <[email protected]>
Co-authored-by: Miriam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiriamAparicio MiriamAparicio MiriamAparicio approved these changes

@cauemarcondes cauemarcondes cauemarcondes approved these changes

@jennypavlova jennypavlova jennypavlova approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@crespocarlos @obltmachine @MiriamAparicio @elasticmachine @kibanamachine @jennypavlova @cauemarcondes