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

[Uptime] Handle mode: all in uptime snapshot calculation #41335

Merged
merged 8 commits into from
Jul 18, 2019

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Jul 17, 2019

Patching this function in #41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.

This needs additional tests before merging.

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@andrewvc andrewvc added bug Fixes for quality problems that affect the customer experience review WIP Work in progress Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.3.0 labels Jul 17, 2019
@andrewvc andrewvc requested a review from justinkambic July 17, 2019 10:05
@andrewvc andrewvc self-assigned this Jul 17, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime

@andrewvc andrewvc added the release_note:skip Skip the PR/issue when compiling release notes label Jul 17, 2019
Patching this function in elastic#41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Tested locally, seems to work fine. LGTM, WFG


const fixturesDir = join(__dirname, 'fixtures');

export const expectFixtureEql = (data: any, fixtureName: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Great! This is along the lines of the solution I was planning to use to address this.

@justinkambic
Copy link
Contributor

FYI I created merge conflicts for your branch by merging updates to the README. Took the liberty of merging the changes.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@andrewvc andrewvc removed the WIP Work in progress label Jul 17, 2019
@andrewvc
Copy link
Contributor Author

jenkins, retest this please

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@andrewvc andrewvc merged commit 16ec40f into elastic:master Jul 18, 2019
@andrewvc andrewvc deleted the fix-snapshot-mode-all branch July 18, 2019 01:45
andrewvc added a commit to andrewvc/kibana that referenced this pull request Jul 18, 2019
…1335)

Patching this function in elastic#41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.
andrewvc added a commit to andrewvc/kibana that referenced this pull request Jul 18, 2019
…1335)

Patching this function in elastic#41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.
andrewvc added a commit that referenced this pull request Jul 18, 2019
) (#41424)

* [Uptime] Handle `mode: all` in uptime snapshot calculation (#41335)

Patching this function in #41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.

* Update snapshot to use heartbeat7.0.0 index for backport
andrewvc added a commit that referenced this pull request Jul 18, 2019
) (#41423)

* [Uptime] Handle `mode: all` in uptime snapshot calculation (#41335)

Patching this function in #41210 fixed performance but was missing this filter clause. We depend on only processing summary info. This fixes errors around missing destructured data where we expect summary fields to be present.

* Update snapshot to use heartbeat7.0.0 index for backport
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes review Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.3.0 v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants