Skip to content

Conversation

@chrisronline
Copy link
Contributor

Relates to #84370

This PR updates a couple source files used for APM monitoring that read from source to using Typescript. This is an attempt to identify all fields read from source to help with #73864

I'm going to try and do one PR for each stack product to avoid long reviews.

@chrisronline chrisronline added review Team:Monitoring Stack Monitoring team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 2, 2020
@chrisronline chrisronline self-assigned this Dec 2, 2020
@chrisronline chrisronline requested a review from a team December 2, 2020 20:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@igoristic
Copy link
Contributor

igoristic commented Dec 3, 2020

@chrisronline
Are we sure that all of these paths will always exist? When using get() we had a safeguard against errors if any of the nested path/value did not exist. Maybe we can also do it for some of these (just incase), something like:

const eventsTotalFirst = firstStats?.metrics?.libbeat?.pipeline?.events?.total ?? null;

And, maybe the types can also indicate which of these might be undefined

@chrisronline
Copy link
Contributor Author

Excellent point @igoristic. I don't actually know what can be defined and what might be undefined. We should probably be on the safe side

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Great job on converting these 🏆

@chrisronline chrisronline merged commit 58f9028 into elastic:master Dec 4, 2020
@chrisronline chrisronline deleted the monitoring/source_ts_apm branch December 4, 2020 18:36
chrisronline added a commit to chrisronline/kibana that referenced this pull request Dec 4, 2020
…to typescript (elastic#84829)

* get_apms converted

* More APM ones

* get_beat_summary

* Fix test

* This is optional

* Fix tests

* Be more safe
chrisronline added a commit that referenced this pull request Dec 4, 2020
…to typescript (#84829) (#85053)

* get_apms converted

* More APM ones

* get_beat_summary

* Fix test

* This is optional

* Fix tests

* Be more safe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.11.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants