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

🚧 WIP 🚧 Replace _source in the APM queries #192608

Closed
wants to merge 92 commits into from
Closed

🚧 WIP 🚧 Replace _source in the APM queries #192608

wants to merge 92 commits into from

Conversation

jennypavlova
Copy link
Member

@jennypavlova jennypavlova commented Sep 11, 2024 •
edited
Loading

🚧

This PR replaces the _source usage in APM queries with fields to support Otel data. The idea is to get rid of existing UI errors we have and make sure that otel data is shown correctly in the UI. One way to check it is using the e2e PoC.

  • The PR doesn't cover Errors because of some known blockers

  • The PR will be extended by explicit typing instead of the current draft type casting

  • The PR changes should work with Otel or APM data but the span links for Otel will be addressed in a separate issue (they should work for APM)

  • Testing with Otel-only indices (can help find potential issues that are not visible when the apm indices are used)

Screen.Recording.2024-09-30.at.13.22.42.mov
  • Testing with otel PoC service
otel_poc_test_sedotlp_hd.mov
  • Testing with otel demo instance/service
otel_poc_demo_service.mov

@elasticmachine
Copy link
Contributor

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!

@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!)

@jennypavlova
Copy link
Member Author

/ci

@jennypavlova jennypavlova linked an issue Sep 12, 2024 that may be closed by this pull request
[APM][Otel] PoC Otel data with APM UI: Replacing _source with fields #192606
Closed
@jennypavlova jennypavlova force-pushed the 192606-poc-otel-data-with-apm-ui-replacing-_source-with-fields branch from 08019f8 to 8d921bb Compare September 16, 2024 12:33
@jennypavlova
Copy link
Member Author

/ci

@jennypavlova
Copy link
Member Author

/ci

@jennypavlova
Copy link
Member Author

/ci

@jennypavlova
Copy link
Member Author

/ci

@miloszmarcinkowski miloszmarcinkowski force-pushed the 192606-poc-otel-data-with-apm-ui-replacing-_source-with-fields branch from 03e02dc to 00d88b5 Compare September 18, 2024 11:49
@bryce-b
Copy link
Contributor

bryce-b commented Oct 1, 2024

/ci

@bryce-b
Copy link
Contributor

bryce-b commented Oct 1, 2024

/ci

@jennypavlova
Copy link
Member Author

/ci

@jennypavlova
Copy link
Member Author

/ci

@MiriamAparicio
Copy link
Contributor

/ci

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 4, 2024 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #10 / APM API tests feature_controls.spec.ts basic no archive apm feature controls APIs can be accessed by global_all user
  • [job] [logs] FTR Configs #10 / APM API tests feature_controls.spec.ts basic no archive apm feature controls APIs can be accessed by global_all user
  • [job] [logs] FTR Configs #60 / EPM Endpoints installs packages that include settings and mappings overrides should install the overrides package correctly
  • [job] [logs] FTR Configs #60 / EPM Endpoints installs packages that include settings and mappings overrides should install the overrides package correctly
  • [job] [logs] FTR Configs #9 / Maps endpoints apis bsearch ES|QL should return getValues response in expected shape
  • [job] [logs] FTR Configs #9 / Maps endpoints apis bsearch ES|QL should return getValues response in expected shape

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 316 344 +28

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 +255.0B
Unknown metric groups

API count

id before after diff
@kbn/apm-types 317 345 +28

History

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

miloszmarcinkowski
miloszmarcinkowski reviewed Oct 7, 2024
View reviewed changes
...ck/plugins/observability_solution/apm/server/routes/services/get_service_metadata_details.ts
Comment on lines +172 to +174
const fieldsNorm = (fields ? serviceMetadataDetailsMapping(fields) : undefined) as
| ServiceMetadataDetailsRaw
| undefined;
Copy link
Contributor

@miloszmarcinkowski miloszmarcinkowski Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We have a check for undefined in serviceMetadataDetailsMapping therefore we can pass undefined safely. No need to double-check. Wdyt? I can update it my PR if you agree

Suggested change
const fieldsNorm = (fields ? serviceMetadataDetailsMapping(fields) : undefined) as
| ServiceMetadataDetailsRaw
| undefined;
const fieldsNorm = serviceMetadataDetailsMapping(fields);

@crespocarlos crespocarlos mentioned this pull request Oct 7, 2024
Merged
@jennypavlova
Copy link
Member Author

Close in favor of the refactored version: #195242

@AlexanderWert AlexanderWert removed a link to an issue Oct 11, 2024
[APM][Otel] PoC Otel data with APM UI: Replacing _source with fields #192606
Closed
crespocarlos added a commit that referenced this pull request Oct 15, 2024
@crespocarlos @kibanamachine
@elasticmachine @jennypavlova
[APM][Otel] Use fields instead of _source on APM queries (#195242)
7235ed0 
closes #192606

## Summary

v2 based on the work done in this PR
#192608 and the suggestion from
Dario #194424

This PR replaces the _source usage in APM queries with fields to support
Otel data. The idea is to get rid of existing UI errors we have and make
sure that otel data is shown correctly in the UI.

One way to check it is using the [e2e
PoC](https://github.com/elastic/otel-apm-e2e-poc/blob/main/README.md).

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Jenny <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 15, 2024
@crespocarlos
[APM][Otel] Use fields instead of _source on APM queries (elastic…
09f2c7c 
…#195242)

closes elastic#192606

## Summary

v2 based on the work done in this PR
elastic#192608 and the suggestion from
Dario elastic#194424

This PR replaces the _source usage in APM queries with fields to support
Otel data. The idea is to get rid of existing UI errors we have and make
sure that otel data is shown correctly in the UI.

One way to check it is using the [e2e
PoC](https://github.com/elastic/otel-apm-e2e-poc/blob/main/README.md).

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Jenny <[email protected]>
(cherry picked from commit 7235ed0)
kibanamachine added a commit that referenced this pull request Oct 15, 2024
@kibanamachine @crespocarlos
[8.x] [APM][Otel] Use &#x60;fields&#x60; instead of &#x60;_source&#x6…
ba5a8fc 
…0; on APM queries (#195242) (#196265)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[APM][Otel] Use &#x60;fields&#x60; instead of &#x60;_source&#x60; on
APM queries (#195242)](#195242)

<!--- 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-10-15T09:38:44Z","message":"[APM][Otel]
Use `fields` instead of `_source` on APM queries (#195242)\n\ncloses
https://github.com/elastic/kibana/issues/192606\r\n\r\n##
Summary\r\n\r\nv2 based on the work done in this
PR\r\nhttps://github.com//pull/192608 and the suggestion
from\r\nDario https://github.com/elastic/kibana/pull/194424\r\n\r\nThis
PR replaces the _source usage in APM queries with fields to
support\r\nOtel data. The idea is to get rid of existing UI errors we
have and make\r\nsure that otel data is shown correctly in the
UI.\r\n\r\nOne way to check it is using the
[e2e\r\nPoC](https://github.com/elastic/otel-apm-e2e-poc/blob/main/README.md).\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Jenny
<[email protected]>","sha":"7235ed0425100bbf04ff157d0af7980875473c99","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","apm:opentelemetry","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services"],"title":"[APM][Otel]
Use `fields` instead of `_source` on APM
queries","number":195242,"url":"https://github.com/elastic/kibana/pull/195242","mergeCommit":{"message":"[APM][Otel]
Use `fields` instead of `_source` on APM queries (#195242)\n\ncloses
https://github.com/elastic/kibana/issues/192606\r\n\r\n##
Summary\r\n\r\nv2 based on the work done in this
PR\r\nhttps://github.com//pull/192608 and the suggestion
from\r\nDario https://github.com/elastic/kibana/pull/194424\r\n\r\nThis
PR replaces the _source usage in APM queries with fields to
support\r\nOtel data. The idea is to get rid of existing UI errors we
have and make\r\nsure that otel data is shown correctly in the
UI.\r\n\r\nOne way to check it is using the
[e2e\r\nPoC](https://github.com/elastic/otel-apm-e2e-poc/blob/main/README.md).\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Jenny
<[email protected]>","sha":"7235ed0425100bbf04ff157d0af7980875473c99"}},"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/195242","number":195242,"mergeCommit":{"message":"[APM][Otel]
Use `fields` instead of `_source` on APM queries (#195242)\n\ncloses
https://github.com/elastic/kibana/issues/192606\r\n\r\n##
Summary\r\n\r\nv2 based on the work done in this
PR\r\nhttps://github.com//pull/192608 and the suggestion
from\r\nDario https://github.com/elastic/kibana/pull/194424\r\n\r\nThis
PR replaces the _source usage in APM queries with fields to
support\r\nOtel data. The idea is to get rid of existing UI errors we
have and make\r\nsure that otel data is shown correctly in the
UI.\r\n\r\nOne way to check it is using the
[e2e\r\nPoC](https://github.com/elastic/otel-apm-e2e-poc/blob/main/README.md).\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Jenny
<[email protected]>","sha":"7235ed0425100bbf04ff157d0af7980875473c99"}}]}]
BACKPORT-->

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

@bryce-b bryce-b bryce-b left review comments

@miloszmarcinkowski miloszmarcinkowski miloszmarcinkowski left review comments

@MiriamAparicio MiriamAparicio MiriamAparicio left review comments

@crespocarlos crespocarlos crespocarlos left review comments

@felixbarny felixbarny felixbarny left review comments

@AlexanderWert AlexanderWert AlexanderWert left review comments

@dgieselaar dgieselaar dgieselaar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.