Skip to content

[Monitoring] Ensure we pass down all the parameters for fetching logs - #43869

Merged
chrisronline merged 8 commits into
elastic:masterfrom
chrisronline:monitoring/fix_missing_log_reason
Aug 29, 2019
Merged

chrisronline merged 8 commits into
elastic:masterfrom
chrisronline:monitoring/fix_missing_log_reason

Conversation

@chrisronline

Copy link
Copy Markdown
Contributor

Resolves #43805

Unfortunate small issue of improper data marshaling. I updated the code to make this better and added a test.

Testing steps (copied from issue):
Steps to reproduce:

  1. Monitor 2 clusters.
  2. Send logs for 1 cluster.
  3. Look at the cluster without logs.

@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@igoristic

Copy link
Copy Markdown
Contributor

@chrisronline Everything looks good!

Was thinking maybe we should also assign default values here:

In an off case that none of the conditions were met. WDYT?

@chrisronline

Copy link
Copy Markdown
Contributor Author

@igoristic Sure, sounds good! I pushed up that change

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@igoristic

Copy link
Copy Markdown
Contributor

LGTM! Nice work 🥇

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@chrisronline

Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@cachedout cachedout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good and seems very sensible! I added some minor comments and I'll wait for a reply on those for now but if there is a rush on this, I have no objection at all to approving it as it sits and merging it. LMK.

Comment thread x-pack/legacy/plugins/monitoring/public/components/logs/reason.js Outdated
Comment thread x-pack/test/api_integration/apis/monitoring/logs/multiple_clusters.js Outdated
@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@chrisronline

Copy link
Copy Markdown
Contributor Author

retest

@chrisronline

Copy link
Copy Markdown
Contributor Author

Oops, didn't see the email about CI issues. I'll wait until CI is ready before retesting

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@chrisronline

Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@chrisronline
chrisronline merged commit e4ded1f into elastic:master Aug 29, 2019
@chrisronline
chrisronline deleted the monitoring/fix_missing_log_reason branch August 29, 2019 16:27
chrisronline added a commit to chrisronline/kibana that referenced this pull request Aug 29, 2019
…elastic#43869)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
chrisronline added a commit that referenced this pull request Aug 29, 2019
…#43869) (#44397)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
chrisronline added a commit to chrisronline/kibana that referenced this pull request Aug 29, 2019
…elastic#43869)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
chrisronline added a commit that referenced this pull request Aug 30, 2019
…#43869) (#44396)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
chrisronline added a commit that referenced this pull request Aug 30, 2019
…#43869) (#44418)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
@chrisronline

Copy link
Copy Markdown
Contributor Author

Backport:

7.3: c0c9223
7.4: fb9a28d
7.x: 577b424

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
…elastic#43869)

* Ensure we pass these all the way down

* Add additional test

* Fix tests

* PR feedback

* Update copy and test wording
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Stack Monitoring] Logs Cluster Panel completely empty

4 participants