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] Aggregated transaction telemetry #71594

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

smith
Copy link
Contributor

@smith smith commented Jul 14, 2020

Add a task to gather transasctions telemetry as described in #71593.

Also:

  • Fix typo in mapping for country code mapping
  • Add back mergeTelemetryMapping function to fix broken upload-telemetry-data script.

Fixes #71593

@smith smith requested a review from a team as a code owner July 14, 2020 05:28
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Jul 14, 2020
@elasticmachine
Copy link
Contributor

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

@smith smith marked this pull request as draft July 14, 2020 05:30
Comment on lines 151 to 211
return {
aggregated_transactions: {
current_implementation: {
transaction_count: allCount,
expected_metric_document_count:
allResult.aggregations?.current_implementation.buckets.length,
},
no_observer_name: {
transaction_count: allCount,
expected_metric_document_count:
allResult.aggregations?.no_observer_name.buckets.length,
},
no_rum: {
transaction_count: noRumCount,
expected_metric_document_count:
noRumResult.aggregations?.no_rum.buckets.length,
},
no_rum_no_observer_name: {
transaction_count: noRumCount,
expected_metric_document_count:
noRumResult.aggregations?.no_rum_no_observer_name.buckets.length,
},
only_rum: {
transaction_count: rumCount,
expected_metric_document_count:
rumResult.aggregations?.only_rum.buckets.length,
},
only_rum_no_observer_name: {
transaction_count: rumCount,
expected_metric_document_count:
rumResult.aggregations?.only_rum_no_observer_name.buckets.length,
},
},
};
},
},
Copy link
Member

Choose a reason for hiding this comment

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

FWICT the results of the buckets are not being paginated through, so they will always return <= 10k. It might be a (lot) more than that (I've seen 60k+). The script I mentioned also paginates through the buckets (by using after_key).

@smith smith requested a review from dgieselaar July 14, 2020 22:15
Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

I don't see any other issues but I'm on my phone so no guarantees 😬 approving with the assumption the pagination is added. & Thank you again @smith !

Add a task to gather transasctions telemetry as described in elastic#71593.

Also:

* Fix typo in mapping for country code mapping
* Add back `mergeTelemetryMapping` function to fix broken `upload-telemetry-data` script.
@smith smith marked this pull request as ready for review July 16, 2020 00:30
@smith smith requested a review from a team July 16, 2020 00:30
@smith smith added v7.9.0 release_note:skip Skip the PR/issue when compiling release notes labels Jul 16, 2020
@smith smith requested a review from a team July 16, 2020 00:31
@smith
Copy link
Contributor Author

smith commented Jul 16, 2020

@elastic/telemetry this PR contains updates to our mapping. See the snapshot diff for details: https://github.com/elastic/kibana/pull/71594/files#diff-229bb3e007b2caea8d59390783ccb586

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
apm 3.7MB +92.0B 3.7MB

History

  • 💔 Build #61810 failed db98355e61a811a1c6dff8f5dd2fb1bbdfc61359
  • 💔 Build #61705 failed fada97f3d7be46ceb013074f617a103ad09afd89
  • 💔 Build #61676 failed b948779854d23b76feb8b94ce913220b3b3e474a
  • 💔 Build #61638 failed a65be0a775d9140f5b2eb1a2db4cc34c6e4bcbcf
  • 💔 Build #61415 failed ef84a6f83e3d360fd1454030b6cc78645e544253

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

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

@smith smith merged commit 8d9d323 into elastic:master Jul 16, 2020
@smith smith deleted the nls/trans-agg-telem branch July 16, 2020 15:22
smith added a commit to smith/kibana that referenced this pull request Jul 16, 2020
Add a task to gather transasctions telemetry as described in elastic#71593.

Also:

* Fix typo in mapping for country code mapping
* Add back `mergeTelemetryMapping` function to fix broken `upload-telemetry-data` script.
smith added a commit to smith/kibana that referenced this pull request Jul 16, 2020
Add a task to gather transasctions telemetry as described in elastic#71593.

Also:

* Fix typo in mapping for country code mapping
* Add back `mergeTelemetryMapping` function to fix broken `upload-telemetry-data` script.
smith added a commit that referenced this pull request Jul 16, 2020
Add a task to gather transasctions telemetry as described in #71593.

Also:

* Fix typo in mapping for country code mapping
* Add back `mergeTelemetryMapping` function to fix broken `upload-telemetry-data` script.

Co-authored-by: Elastic Machine <[email protected]>
smith added a commit that referenced this pull request Jul 17, 2020
Add a task to gather transasctions telemetry as described in #71593.

Also:

* Fix typo in mapping for country code mapping
* Add back `mergeTelemetryMapping` function to fix broken `upload-telemetry-data` script.

Co-authored-by: Elastic Machine <[email protected]>
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 Team:APM All issues that need APM UI Team support v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

APM telemetry for aggregated transactions
5 participants