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] Fix an error with mobile services coming from synthtrace #196313

Conversation

jennypavlova
Copy link
Member

@jennypavlova jennypavlova commented Oct 15, 2024

Closes #196161

Summary

This PR fixes an issue with the mobile data using synthtrace. After some investigation I saw that the the httpSpan was creating the spans with transaction.type set which resulted in processor.event being set to transaction instead of span - then with the new required transaction fields in get_trace_docs for transactions (checking based on the processor.event) we were throwing an error because the transaction fields were not defined (which is expected because it's a span and not a transaction)

Testing

Generate mobile data using:
node scripts/synthtrace mobile.ts --clean

Open all the mobile traces (ios/Android) - there should not be an error

@jennypavlova jennypavlova added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0 labels Oct 15, 2024
@jennypavlova jennypavlova self-assigned this Oct 15, 2024
@jennypavlova jennypavlova requested review from a team as code owners October 15, 2024 13:36
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Oct 15, 2024
Copy link
Contributor

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -230,7 +230,7 @@ export class MobileDevice extends Entity<ApmFields> {
spanSubtype: 'http',
'http.request.method': httpMethod,
'url.original': httpUrl,
'transaction.type': 'mobile',
'processor.event': 'span',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was it mobile?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it was set because of the parent transaction - it is set there but inside the span function we are not setting the transaction.type so that's why some spans were visible and the httpSpans were throwing an error. I don't see any test checking it and it's not part of any span-specific fields so I guess it was something we had copied from the transaction 🤔

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 15, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 5b1e0bd
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-196313-5b1e0bd264b0

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group6.ts / discover/esql discover esql view switch modal should not show switch modal when switching to a data view while a saved search is open

Metrics [docs]

✅ unchanged

History

cc @jennypavlova

@jennypavlova jennypavlova merged commit c218e7c into elastic:main Oct 15, 2024
29 checks passed
@jennypavlova jennypavlova added backport:version Backport to applied version labels and removed backport:skip This commit does not require backporting labels Oct 15, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 15, 2024
…lastic#196313)

Closes elastic#196161
## Summary

This PR fixes an issue with the mobile data using synthtrace. After some
investigation I saw that the the `httpSpan` was creating the spans with
`transaction.type` set which resulted in `processor.event` being set to
`transaction` instead of `span` - then with [the new required
transaction
fields](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L277)
in get_trace_docs for transactions ([checking based on the
processor.event](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L352))
we were throwing an error because the transaction fields were not
defined (which is expected because it's a span and not a transaction)

## Testing

Generate mobile data using:
`node scripts/synthtrace mobile.ts --clean`

Open all the mobile traces (ios/Android) - there should not be an error

(cherry picked from commit c218e7c)
@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 added a commit that referenced this pull request Oct 15, 2024
…trace (#196313) (#196357)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[APM][Otel] Fix an error with mobile services coming from synthtrace
(#196313)](#196313)

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

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

<!--BACKPORT
[{"author":{"name":"jennypavlova","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-15T15:53:12Z","message":"[APM][Otel]
Fix an error with mobile services coming from synthtrace
(#196313)\n\nCloses #196161 \r\n## Summary\r\n\r\nThis PR fixes an issue
with the mobile data using synthtrace. After some\r\ninvestigation I saw
that the the `httpSpan` was creating the spans
with\r\n`transaction.type` set which resulted in `processor.event` being
set to\r\n`transaction` instead of `span` - then with [the new
required\r\ntransaction\r\nfields](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L277)\r\nin
get_trace_docs for transactions ([checking based on
the\r\nprocessor.event](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L352))\r\nwe
were throwing an error because the transaction fields were
not\r\ndefined (which is expected because it's a span and not a
transaction)\r\n\r\n## Testing \r\n\r\nGenerate mobile data
using:\r\n`node scripts/synthtrace mobile.ts --clean`\r\n\r\nOpen all
the mobile traces (ios/Android) - there should not be an
error","sha":"c218e7cc29c00864d744d886c6e712b99ba97ed5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0","backport:version"],"title":"[APM][Otel]
Fix an error with mobile services coming from
synthtrace","number":196313,"url":"https://github.com/elastic/kibana/pull/196313","mergeCommit":{"message":"[APM][Otel]
Fix an error with mobile services coming from synthtrace
(#196313)\n\nCloses #196161 \r\n## Summary\r\n\r\nThis PR fixes an issue
with the mobile data using synthtrace. After some\r\ninvestigation I saw
that the the `httpSpan` was creating the spans
with\r\n`transaction.type` set which resulted in `processor.event` being
set to\r\n`transaction` instead of `span` - then with [the new
required\r\ntransaction\r\nfields](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L277)\r\nin
get_trace_docs for transactions ([checking based on
the\r\nprocessor.event](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L352))\r\nwe
were throwing an error because the transaction fields were
not\r\ndefined (which is expected because it's a span and not a
transaction)\r\n\r\n## Testing \r\n\r\nGenerate mobile data
using:\r\n`node scripts/synthtrace mobile.ts --clean`\r\n\r\nOpen all
the mobile traces (ios/Android) - there should not be an
error","sha":"c218e7cc29c00864d744d886c6e712b99ba97ed5"}},"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/196313","number":196313,"mergeCommit":{"message":"[APM][Otel]
Fix an error with mobile services coming from synthtrace
(#196313)\n\nCloses #196161 \r\n## Summary\r\n\r\nThis PR fixes an issue
with the mobile data using synthtrace. After some\r\ninvestigation I saw
that the the `httpSpan` was creating the spans
with\r\n`transaction.type` set which resulted in `processor.event` being
set to\r\n`transaction` instead of `span` - then with [the new
required\r\ntransaction\r\nfields](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L277)\r\nin
get_trace_docs for transactions ([checking based on
the\r\nprocessor.event](https://github.com/elastic/kibana/blob/adb558a86bafbe3567915c3fae252ff414147930/x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts#L352))\r\nwe
were throwing an error because the transaction fields were
not\r\ndefined (which is expected because it's a span and not a
transaction)\r\n\r\n## Testing \r\n\r\nGenerate mobile data
using:\r\n`node scripts/synthtrace mobile.ts --clean`\r\n\r\nOpen all
the mobile traces (ios/Android) - there should not be an
error","sha":"c218e7cc29c00864d744d886c6e712b99ba97ed5"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: jennypavlova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels 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.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM][Otel] Fix an error with mobile services
5 participants