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

[Telemetry] Remove from and to timestamps from usage stats APIs #81579

Merged
merged 12 commits into from
Nov 2, 2020

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented Oct 25, 2020

  • The usage api now accepts a timestamp string or unix timestamp number. The api no longer accepts a TimeRange parameter.

  • Restrict from and to usage to only inside monitoring collection where it is used.

  • Increase fetching Cluster_uuid query timestamp difference from 20 minutes to 3 hours. Currently if a cluster has not reported anything in the last 20 minutes then we do not send any data related to that cluster. Increasing the query from 20 min to 3 hours provides better guarantees on capturing all the connected monitoring clusters.

This change makes it easier to work on alejandro's RFC to completely decouple monitoring from telemetry.

Closes #57782

@Bamieh Bamieh added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Oct 25, 2020
@Bamieh
Copy link
Member Author

Bamieh commented Oct 25, 2020

@elasticmachine merge upstream

@Bamieh Bamieh marked this pull request as ready for review October 27, 2020 13:09
@Bamieh Bamieh requested a review from a team as a code owner October 27, 2020 13:09
@Bamieh Bamieh requested a review from a team October 27, 2020 13:09
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM for stack monitoring. It seems you only touched your files in the monitoring plugin

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this!

It works as it is right now but we should be using the monitoring/common/constants.ts file for all the hard-coded time intervals, like we do for the TELEMETRY_COLLECTION_INTERVAL. That way we can keep them in one place and document each value.

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for doing this 🧡

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

page load bundle size

id before after diff
monitoring 33.1KB 33.1KB +75.0B
telemetry 63.6KB 63.5KB -79.0B
total -4.0B

History

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

@Bamieh Bamieh dismissed TinaHeiligers’s stale review November 2, 2020 11:51

All requested changes addressed. Approved by @afharo

@Bamieh Bamieh merged commit 1af551b into elastic:master Nov 2, 2020
@Bamieh Bamieh deleted the telemetry/optional_to branch November 2, 2020 11:51
Bamieh added a commit to Bamieh/kibana that referenced this pull request Nov 2, 2020
…lastic#81579)

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	x-pack/test/functional/apps/infra/home_page.ts
#	x-pack/test/functional/apps/infra/logs_source_configuration.ts
Bamieh added a commit that referenced this pull request Nov 2, 2020
…APIs (#81579) (#82266)

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	x-pack/test/functional/apps/infra/home_page.ts
#	x-pack/test/functional/apps/infra/logs_source_configuration.ts
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 2, 2020
…e-details-overlay

* 'master' of github.com:elastic/kibana: (72 commits)
  [CCR] Update README.md on how to start 2 clusters for testing (elastic#81487)
  [APM] Scale transaction rate correctly (elastic#82155)
  Upgrade to hapi version 18 (elastic#80468)
  [Uptime] Remove custom handling of license enabling (elastic#82019)
  [Telemetry] Remove `from` and `to` timestamps from usage stats APIs (elastic#81579)
  Enable send to background in Vega (elastic#82229)
  Enable send to background in Timelion (elastic#82232)
  [Actions & Connectors] removes Connector flyouts after usage (elastic#82126)
  Add derivative function (elastic#81178)
  [Discover] Deangularize context_app.html, part 3 (elastic#81838)
  [Visualize] Vis listing page breaks on unknown vis type (elastic#82018)
  Rename `batchSize` parameter to `batch_size` to be consisten with the API namings guidelines. (elastic#82123)
  Minor edits in Single Metric Viewer (elastic#82159)
  [Actions] Fix type contract (elastic#82168)
  Upgrade EUI to v30.1.1 (elastic#81499)
  Skip failing ES snapshot test (elastic#82207)
  Skip ES snapshot failing suite (elastic#82206)
  [Alerting UI] Grouped list of alert types using producers in Types filter of Alerts tab (elastic#81876)
  [Maps] convert vector style component to typescript round 1 (elastic#81961)
  Fix link to upgrade assistant (elastic#82138)
  ...
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 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Telemetry] Remove from and to timestamps from usage stats APIs
5 participants