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][OTel] Use telemetry.sdk as a fallback for missing agent.name on non-tracing data #196529

Merged

Conversation

rmyz
Copy link
Contributor

@rmyz rmyz commented Oct 16, 2024

Summary

Closes #195854

This PR adds a fallback when we are missing agent.name on APM Service Inventory list.
Using OTel semantic convention fields telemetry.sdk.language and telemetry.sdk.name to maintain agent.name format for OTel fields like otlp/${agent} opentelemetry/${agent}.

Screenshots

Before After
image image
image image

How to test it

  1. Open otel-apm-e2e-poc repo
  2. Add the following code under processors -> transform -> metric_statements in otelcol.yaml
    - context: datapoint
      statements:
        - set(attributes["processor.event"], "metric")
  1. Run make start-stack & make build & make run on otel poc repo
  2. Don't forget to run also make start on opentelemetry-demo and follow this guide to make it work.
  3. In Kibana add elasticsearch.ignoreVersionMismatch: true to kibana.dev.yml.
  4. If you go to APM Service Inventory list you will see missing icons.
  5. Checkout to my branch
  6. Try again, they should be fixed

@rmyz rmyz changed the title [APM][OTel] Use telemetry.sdk as a fallback for missing agent.name [APM][OTel] Use telemetry.sdk as a fallback for missing agent.name on non-tracing data Oct 16, 2024
@rmyz
Copy link
Contributor Author

rmyz commented Oct 16, 2024

/ci

@rmyz rmyz added release_note:fix apm:opentelemetry APM UI - OTEL Work 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 Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Oct 16, 2024
@rmyz rmyz self-assigned this Oct 16, 2024
@@ -124,6 +127,16 @@ export async function getServiceTransactionStats({
size: maxNumServices,
},
aggs: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Warning

I don't know if this is the best place in the query for this.

@rmyz
Copy link
Contributor Author

rmyz commented Oct 16, 2024

/ci

@rmyz
Copy link
Contributor Author

rmyz commented Oct 16, 2024

/ci

@rmyz rmyz marked this pull request as ready for review October 16, 2024 17:09
@rmyz rmyz requested a review from a team as a code owner October 16, 2024 17:09
@elasticmachine
Copy link
Contributor

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

@jennypavlova jennypavlova self-requested a review October 17, 2024 11:00
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.

Code LGTM, could not run it.

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.

Nice, thanks for the fix! I compared the service lists with the table icons and they are fixed there 🎉
I wasn't able to add the collector changes you mentioned (it resulted in an error) but based on my testing without them I couldn't see some of the icons in the service details pages:

Screen.Recording.2024-10-17.at.14.06.37.mov

Should we change the logic to check for the agent name here as well 🤔 Not sure if that's the issue or other information is missing can you please take a look?

Edit: Forgot to mention that using sendotlp the icons are there but I saw unknown when I clicked on the second one (should be the condition I mentioned or something missing in my env so if you see the info there you can ignore that)

packages/kbn-apm-types/src/es_fields/apm.ts Show resolved Hide resolved
@rmyz rmyz requested a review from a team as a code owner October 18, 2024 08:48
@rmyz
Copy link
Contributor Author

rmyz commented Oct 18, 2024

/ci

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! Thank you for the fix, I see the icons now 🚀

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 18, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 8c376c7
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-196529-8c376c746299

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/apm-types 333 336 +3
@kbn/elastic-agent-utils 37 41 +4
total +7
Unknown metric groups

API count

id before after diff
@kbn/apm-types 334 337 +3
@kbn/elastic-agent-utils 38 42 +4
total +7

History

cc @rmyz

Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM

@rmyz rmyz merged commit 16098c4 into elastic:main Oct 18, 2024
31 of 32 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2024
…` on non-tracing data (elastic#196529)

## Summary

Related to elastic#195854

This PR adds a fallback when we are missing `agent.name` on APM Service
Inventory list.
Using [OTel semantic convention
fields](https://opentelemetry.io/docs/specs/semconv/resource/#telemetry-sdk)
`telemetry.sdk.language` and `telemetry.sdk.name` to maintain
`agent.name` format for OTel fields like `otlp/${agent}`
`opentelemetry/${agent}`.

## Screenshots
| Before | After |

|-------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------|
|
![image](https://github.com/user-attachments/assets/792ca256-f01d-4eae-8a2d-af16fa34ea61)
|
![image](https://github.com/user-attachments/assets/2816d1c7-1207-4da8-adb5-ec417b3fd26e)
|

![image](https://github.com/user-attachments/assets/27df0ffc-8d5f-475c-ad6b-04086521871b)|![image](https://github.com/user-attachments/assets/d088a746-1375-4918-8e55-d3968a80772d)

## How to test it
1. Open otel-apm-e2e-poc repo
2. Add the following code under `processors -> transform ->
metric_statements` in `otelcol.yaml`
```yaml
    - context: datapoint
      statements:
        - set(attributes["processor.event"], "metric")
```
3. Run `make start-stack` & `make build` & `make run` on otel poc repo
4. Don't forget to run also `make start` on opentelemetry-demo and
[follow this
guide](https://github.com/elastic/otel-apm-e2e-poc?tab=readme-ov-file#working-with-opentelemetry-demo)
to make it work.
5. In Kibana add `elasticsearch.ignoreVersionMismatch: true` to
`kibana.dev.yml`.
6. If you go to APM Service Inventory list you will see missing icons.
7. Checkout to my branch
8. Try again, they should be fixed

(cherry picked from commit 16098c4)
@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

@rmyz rmyz deleted the 195854-apm-make-ui-more-resilent-to-missing-fields-otel branch October 21, 2024 08:35
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 21, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine added a commit that referenced this pull request Oct 21, 2024
…sing `agent.name` on non-tracing data (#196529) (#196854)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[APM][OTel] Use `telemetry.sdk` as a fallback for missing
`agent.name` on non-tracing data
(#196529)](#196529)

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

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

<!--BACKPORT [{"author":{"name":"Sergi
Romeu","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-18T11:30:19Z","message":"[APM][OTel]
Use `telemetry.sdk` as a fallback for missing `agent.name` on
non-tracing data (#196529)\n\n## Summary\r\n\r\nRelated to
#195854\r\n\r\nThis PR adds a fallback when we are missing `agent.name`
on APM Service\r\nInventory list.\r\nUsing [OTel semantic
convention\r\nfields](https://opentelemetry.io/docs/specs/semconv/resource/#telemetry-sdk)\r\n`telemetry.sdk.language`
and `telemetry.sdk.name` to maintain\r\n`agent.name` format for OTel
fields like `otlp/${agent}`\r\n`opentelemetry/${agent}`.\r\n\r\n##
Screenshots\r\n| Before | After
|\r\n\r\n|-------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------|\r\n|\r\n![image](https://github.com/user-attachments/assets/792ca256-f01d-4eae-8a2d-af16fa34ea61)\r\n|\r\n![image](https://github.com/user-attachments/assets/2816d1c7-1207-4da8-adb5-ec417b3fd26e)\r\n|\r\n\r\n![image](https://github.com/user-attachments/assets/27df0ffc-8d5f-475c-ad6b-04086521871b)|![image](https://github.com/user-attachments/assets/d088a746-1375-4918-8e55-d3968a80772d)\r\n\r\n\r\n##
How to test it\r\n1. Open otel-apm-e2e-poc repo\r\n2. Add the following
code under `processors -> transform ->\r\nmetric_statements` in
`otelcol.yaml`\r\n```yaml\r\n - context: datapoint\r\n statements:\r\n -
set(attributes[\"processor.event\"], \"metric\")\r\n```\r\n3. Run `make
start-stack` & `make build` & `make run` on otel poc repo \r\n4. Don't
forget to run also `make start` on opentelemetry-demo and\r\n[follow
this\r\nguide](https://github.com/elastic/otel-apm-e2e-poc?tab=readme-ov-file#working-with-opentelemetry-demo)\r\nto
make it work.\r\n5. In Kibana add `elasticsearch.ignoreVersionMismatch:
true` to\r\n`kibana.dev.yml`.\r\n6. If you go to APM Service Inventory
list you will see missing icons.\r\n7. Checkout to my branch\r\n8. Try
again, they should be
fixed","sha":"16098c454751939d9e3587ff458bb652b28e3d79","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","apm:opentelemetry","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services"],"title":"[APM][OTel]
Use `telemetry.sdk` as a fallback for missing `agent.name` on
non-tracing
data","number":196529,"url":"https://github.com/elastic/kibana/pull/196529","mergeCommit":{"message":"[APM][OTel]
Use `telemetry.sdk` as a fallback for missing `agent.name` on
non-tracing data (#196529)\n\n## Summary\r\n\r\nRelated to
#195854\r\n\r\nThis PR adds a fallback when we are missing `agent.name`
on APM Service\r\nInventory list.\r\nUsing [OTel semantic
convention\r\nfields](https://opentelemetry.io/docs/specs/semconv/resource/#telemetry-sdk)\r\n`telemetry.sdk.language`
and `telemetry.sdk.name` to maintain\r\n`agent.name` format for OTel
fields like `otlp/${agent}`\r\n`opentelemetry/${agent}`.\r\n\r\n##
Screenshots\r\n| Before | After
|\r\n\r\n|-------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------|\r\n|\r\n![image](https://github.com/user-attachments/assets/792ca256-f01d-4eae-8a2d-af16fa34ea61)\r\n|\r\n![image](https://github.com/user-attachments/assets/2816d1c7-1207-4da8-adb5-ec417b3fd26e)\r\n|\r\n\r\n![image](https://github.com/user-attachments/assets/27df0ffc-8d5f-475c-ad6b-04086521871b)|![image](https://github.com/user-attachments/assets/d088a746-1375-4918-8e55-d3968a80772d)\r\n\r\n\r\n##
How to test it\r\n1. Open otel-apm-e2e-poc repo\r\n2. Add the following
code under `processors -> transform ->\r\nmetric_statements` in
`otelcol.yaml`\r\n```yaml\r\n - context: datapoint\r\n statements:\r\n -
set(attributes[\"processor.event\"], \"metric\")\r\n```\r\n3. Run `make
start-stack` & `make build` & `make run` on otel poc repo \r\n4. Don't
forget to run also `make start` on opentelemetry-demo and\r\n[follow
this\r\nguide](https://github.com/elastic/otel-apm-e2e-poc?tab=readme-ov-file#working-with-opentelemetry-demo)\r\nto
make it work.\r\n5. In Kibana add `elasticsearch.ignoreVersionMismatch:
true` to\r\n`kibana.dev.yml`.\r\n6. If you go to APM Service Inventory
list you will see missing icons.\r\n7. Checkout to my branch\r\n8. Try
again, they should be
fixed","sha":"16098c454751939d9e3587ff458bb652b28e3d79"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196529","number":196529,"mergeCommit":{"message":"[APM][OTel]
Use `telemetry.sdk` as a fallback for missing `agent.name` on
non-tracing data (#196529)\n\n## Summary\r\n\r\nRelated to
#195854\r\n\r\nThis PR adds a fallback when we are missing `agent.name`
on APM Service\r\nInventory list.\r\nUsing [OTel semantic
convention\r\nfields](https://opentelemetry.io/docs/specs/semconv/resource/#telemetry-sdk)\r\n`telemetry.sdk.language`
and `telemetry.sdk.name` to maintain\r\n`agent.name` format for OTel
fields like `otlp/${agent}`\r\n`opentelemetry/${agent}`.\r\n\r\n##
Screenshots\r\n| Before | After
|\r\n\r\n|-------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------|\r\n|\r\n![image](https://github.com/user-attachments/assets/792ca256-f01d-4eae-8a2d-af16fa34ea61)\r\n|\r\n![image](https://github.com/user-attachments/assets/2816d1c7-1207-4da8-adb5-ec417b3fd26e)\r\n|\r\n\r\n![image](https://github.com/user-attachments/assets/27df0ffc-8d5f-475c-ad6b-04086521871b)|![image](https://github.com/user-attachments/assets/d088a746-1375-4918-8e55-d3968a80772d)\r\n\r\n\r\n##
How to test it\r\n1. Open otel-apm-e2e-poc repo\r\n2. Add the following
code under `processors -> transform ->\r\nmetric_statements` in
`otelcol.yaml`\r\n```yaml\r\n - context: datapoint\r\n statements:\r\n -
set(attributes[\"processor.event\"], \"metric\")\r\n```\r\n3. Run `make
start-stack` & `make build` & `make run` on otel poc repo \r\n4. Don't
forget to run also `make start` on opentelemetry-demo and\r\n[follow
this\r\nguide](https://github.com/elastic/otel-apm-e2e-poc?tab=readme-ov-file#working-with-opentelemetry-demo)\r\nto
make it work.\r\n5. In Kibana add `elasticsearch.ignoreVersionMismatch:
true` to\r\n`kibana.dev.yml`.\r\n6. If you go to APM Service Inventory
list you will see missing icons.\r\n7. Checkout to my branch\r\n8. Try
again, they should be
fixed","sha":"16098c454751939d9e3587ff458bb652b28e3d79"}}]}] BACKPORT-->

Co-authored-by: Sergi Romeu <[email protected]>
@kibanamachine kibanamachine added v8.17.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Oct 21, 2024
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-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:fix Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM][OTel] Empty service icons when custom data exists (without agent.name)
7 participants