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] transactions.kubernetes.pod can be undefined #90044

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

afharo
Copy link
Member

@afharo afharo commented Feb 2, 2021

Summary

We've been notified about some errors where the transaction view doesn't load because the transactions.kubernetes payload looks like this:

"kubernetes": {
  "namespace": "ci1-sass"
}

This PR makes kubernetes.pod an optional parameter in the Types definitions to make sure every logic validates it exists before reaching to its internal properties.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release Notes

Fixes issue that may show empty Transaction page in the APM app.

@afharo afharo requested a review from a team as a code owner February 2, 2021 17:06
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Feb 2, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@afharo
Copy link
Member Author

afharo commented Feb 2, 2021

Q: Should the label point to 7.11.0?

Copy link
Contributor

@ogupte ogupte left a comment

Choose a reason for hiding this comment

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

LGTM

@sorenlouv
Copy link
Member

Thanks for fixing this @afharo ! I'm surprised this can even happen. We have a contract test between apm-server and kibana to ensure it doesn't.
Looking closer at our test I noticed that we don't test this properly. We test transactions here:

https://github.com/elastic/apm-integration-testing/blob/12b688db3b4f5166433532ccc53fbcad80147c77/scripts/kibana/validate-ts-interfaces-against-apm-server-sample-docs/scripts/download-sample-docs.ts#L23-L27

But we should test it against the minimal doc:
https://github.com/elastic/apm-server/blob/master/beater/test_approved_es_documents/TestPublishIntegrationMinimalEvents.approved.json

@sorenlouv
Copy link
Member

Created this follow-up issue: #90065

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 5.2MB 5.2MB +118.0B

History

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

@afharo afharo merged commit 43faa76 into elastic:master Feb 2, 2021
@afharo afharo deleted the apm/kubernetes.pod-optional branch February 2, 2021 20:43
afharo added a commit to afharo/kibana that referenced this pull request Feb 2, 2021
# Conflicts:
#	x-pack/plugins/apm/typings/es_schemas/raw/fields/kubernetes.ts
@afharo afharo added v7.11.0 and removed v7.11.1 labels Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix Team:APM All issues that need APM UI Team support v7.11.0 v7.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants