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

[APM UI] Fix OpenTelemetry agent names #193134

Merged
merged 21 commits into from
Sep 20, 2024

Conversation

rmyz
Copy link
Contributor

@rmyz rmyz commented Sep 17, 2024

Summary

Fixes #180444

This PR fixes the agent names not being able to properly be retrieved by the APM UI, changing the way we map OpenTelemetry agent names.
As the format changed from (opentelemetry|otlp)/{agentName} to (opentelemetry|otlp)/{agentName}/{details}, we now get the second part splitting by /.

Added mappings for RUM, Android, and iOS OpenTelemetry client, also fixed get_service_metadata_details to get the correct OpenTelemetry details.

Before After
image image

How to test

  1. Checkout to this branch
  2. Run node scripts/synthtrace many_otel_services.ts --live --clean which will fill some APM Otel services.
  3. Check that the icon is now rendering

@rmyz rmyz added release_note:fix apm:opentelemetry APM UI - OTEL Work ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Sep 17, 2024
@rmyz rmyz self-assigned this Sep 17, 2024
@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!)

@rmyz rmyz marked this pull request as ready for review September 17, 2024 10:19
@rmyz rmyz requested review from a team as code owners September 17, 2024 10:19
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

I haven't tested it, but the code is looking good. Thanks for the synthtrace scenario. Left a few comments

packages/kbn-elastic-agent-utils/src/agent_guards.ts Outdated Show resolved Hide resolved
packages/kbn-elastic-agent-utils/src/agent_guards.ts Outdated Show resolved Hide resolved
packages/kbn-elastic-agent-utils/src/agent_guards.ts Outdated Show resolved Hide resolved
@rmyz rmyz requested review from a team as code owners September 17, 2024 12:04
@rogercoll
Copy link

Thanks for pushing this! 🚀

Will this PR also cover the "otlp" prefix? For example, this is the case for the OpenTelemetry Go auto-instrumentation agent:
image

@AlexanderWert
Copy link
Member

Will this PR also cover the "otlp" prefix?

Great point, @rogercoll !

@rmyz Can we cover that case as well please? So that the prefix can be either opentelemetry or otlp

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.

Good job, checked with the scenario and works as expected, added some small comments/questions, should be quick to add

@rmyz
Copy link
Contributor Author

rmyz commented Sep 17, 2024

Will this PR also cover the "otlp" prefix?

Great point, @rogercoll !

@rmyz Can we cover that case as well please? So that the prefix can be either opentelemetry or otlp

will add that case @rogercoll @AlexanderWert , thanks for checking guys! 🚀

@rmyz
Copy link
Contributor Author

rmyz commented Sep 17, 2024

Added otlp/ prefix check and fixed get_service_metadata_details to get the proper OpenTelemetry agent details.
CC @rogercoll @AlexanderWert

I will be fixing the failing tests

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.

Code LGTM 💯 Thanks for the changes!

}

return (
agentName.startsWith(`opentelemetry/${language}`) || agentName.startsWith(`otlp/${language}`)
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
agentName.startsWith(`opentelemetry/${language}`) || agentName.startsWith(`otlp/${language}`)
agentName.startsWith(`opentelemetry/${language}` || `otlp/${language}`);

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 think the proposed solution does not apply here, as || acts as a fallback value if the first one is falsy, but as it isn't it's not going to work, the function is not able to know which value should compare it to, so it does it for the first one always.

See the example below
image

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Telemetry changes LGTM

@rmyz rmyz force-pushed the 180444-apm-ui-fix-opentelemetry-agent-names branch from 238b91f to 7c1f216 Compare September 18, 2024 11:20
@rmyz
Copy link
Contributor Author

rmyz commented Sep 18, 2024

@elasticmachine merge upstream

@@ -8,6 +8,7 @@
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

💪 !

Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the changes.

@rmyz
Copy link
Contributor Author

rmyz commented Sep 18, 2024

@elasticmachine merge upstream

@rmyz rmyz force-pushed the 180444-apm-ui-fix-opentelemetry-agent-names branch from 9ca7365 to 548d7a3 Compare September 19, 2024 15:45
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 19, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 548d7a3
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-193134-548d7a3545a8

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/elastic-agent-utils 34 37 +3

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.4MB 3.4MB +783.0B
discover 812.5KB 812.9KB +423.0B
infra 1.6MB 1.6MB +425.0B
total +1.6KB
Unknown metric groups

API count

id before after diff
@kbn/elastic-agent-utils 35 38 +3

History

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

cc @rmyz

@PhilippeOberti PhilippeOberti removed the request for review from a team September 19, 2024 21:51
@rmyz rmyz merged commit 735e216 into elastic:main Sep 20, 2024
24 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2024
## Summary

Fixes elastic#180444

This PR fixes the agent names not being able to properly be retrieved by
the APM UI, changing the way we map OpenTelemetry agent names.
As the format changed from `(opentelemetry|otlp)/{agentName}` to
`(opentelemetry|otlp)/{agentName}/{details}`, we now get the second part
splitting by `/`.

Added mappings for RUM, Android, and iOS OpenTelemetry client, also
fixed `get_service_metadata_details` to get the correct OpenTelemetry
details.

|Before|After|
|-|-|

|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|

## How to test
1. Checkout to this branch
2. Run `node scripts/synthtrace many_otel_services.ts --live --clean`
which will fill some APM Otel services.
3. Check that the icon is now rendering

(cherry picked from commit 735e216)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.15 Backport failed because of merge conflicts
8.x

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 193134

Questions ?

Please refer to the Backport tool documentation

@rmyz
Copy link
Contributor Author

rmyz commented Sep 20, 2024

💚 All backports created successfully

Status Branch Result
8.15

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

Questions ?

Please refer to the Backport tool documentation

rmyz added a commit to rmyz/kibana that referenced this pull request Sep 20, 2024
## Summary

Fixes elastic#180444

This PR fixes the agent names not being able to properly be retrieved by
the APM UI, changing the way we map OpenTelemetry agent names.
As the format changed from `(opentelemetry|otlp)/{agentName}` to
`(opentelemetry|otlp)/{agentName}/{details}`, we now get the second part
splitting by `/`.

Added mappings for RUM, Android, and iOS OpenTelemetry client, also
fixed `get_service_metadata_details` to get the correct OpenTelemetry
details.

|Before|After|
|-|-|

|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|

## How to test
1. Checkout to this branch
2. Run `node scripts/synthtrace many_otel_services.ts --live --clean`
which will fill some APM Otel services.
3. Check that the icon is now rendering

(cherry picked from commit 735e216)

# Conflicts:
#	src/plugins/telemetry/schema/oss_plugins.json
@rmyz rmyz deleted the 180444-apm-ui-fix-opentelemetry-agent-names branch September 20, 2024 08:21
kibanamachine added a commit that referenced this pull request Sep 20, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[APM UI] Fix OpenTelemetry agent names
(#193134)](#193134)

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

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

<!--BACKPORT [{"author":{"name":"Sergi
Romeu","email":"sergi.romeu@elastic.co"},"sourceCommit":{"committedDate":"2024-09-20T07:07:14Z","message":"[APM
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","apm:opentelemetry","backport:prev-major","ci:project-deploy-observability","Team:obs-ux-infra_services","backport:version"],"title":"[APM
UI] Fix OpenTelemetry agent
names","number":193134,"url":"#193134
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#193134
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd"}}]}]
BACKPORT-->

---------

Co-authored-by: Sergi Romeu <sergi.romeu@elastic.co>
rmyz added a commit that referenced this pull request Sep 20, 2024
# Backport

This will backport the following commits from `main` to `8.15`:
- [[APM UI] Fix OpenTelemetry agent names
(#193134)](#193134)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Sergi
Romeu","email":"sergi.romeu@elastic.co"},"sourceCommit":{"committedDate":"2024-09-20T07:07:14Z","message":"[APM
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","apm:opentelemetry","backport:prev-major","ci:project-deploy-observability","Team:obs-ux-infra_services","backport:version"],"number":193134,"url":"#193134
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"#193134
UI] Fix OpenTelemetry agent names (#193134)\n\n## Summary\r\n\r\nFixes
#180444 PR fixes the
agent names not being able to properly be retrieved by\r\nthe APM UI,
changing the way we map OpenTelemetry agent names.\r\nAs the format
changed from `(opentelemetry|otlp)/{agentName}`
to\r\n`(opentelemetry|otlp)/{agentName}/{details}`, we now get the
second part\r\nsplitting by `/`.\r\n\r\nAdded mappings for RUM, Android,
and iOS OpenTelemetry client, also\r\nfixed
`get_service_metadata_details` to get the correct
OpenTelemetry\r\ndetails.\r\n\r\n|Before|After|\r\n|-|-|\r\n\r\n|![image](https://github.com/user-attachments/assets/28732018-511b-44e0-ac86-cdbe7ed0d1e0)|![image](https://github.com/user-attachments/assets/45a29cc6-f939-4c52-bcc7-54dc15b1a403)|\r\n\r\n##
How to test\r\n1. Checkout to this branch\r\n2. Run `node
scripts/synthtrace many_otel_services.ts --live --clean`\r\nwhich will
fill some APM Otel services.\r\n3. Check that the icon is now
rendering","sha":"735e216a952670eb57eaea1229be16e89f9bf1cd"}},{"url":"#193509"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:opentelemetry APM UI - OTEL Work backport:prev-major Backport to the previous major branch and all later branches still in development backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project release_note:fix Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.15.2 v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] OTel Agent names not recognized correctly after a change in mapping