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

[SLOS] fix APM group by cardinality count #183171

Merged

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented May 10, 2024

Summary

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

Applies filters based on the selected APM indicator params to the cardinality count query.

Screenshot 2024-05-10 at 1 07 27 PM

Testing.

  1. Create mock api data via node scripts/synthtrace continuous_rollups --live
  2. Navigate to create SLI and choose APM latency
  3. Select a service
  4. Select group by as service.name. Cardinality count should be 1
  5. Select environment all and change the group by to service.environment. Cardinality count should be 2.
  6. Now select a specific environment. Notice cardinality count changes to 1.
  7. Repeat this testing strategy with transaction name and transaction type, changing the group by field to transaction.name or transaction.type, respectively and playing around with the ALL_VALUE versus a specific value.
  8. Also make sure to test the global filter for regressions

Release note

The cardinality count for SLOs generated from a single SLI definition was previously incorrect for APM latency and APM availability SLIs. The cardinality count is now accurate.

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@dominiqueclarke
Copy link
Contributor Author

/ci

@dominiqueclarke dominiqueclarke added bug Fixes for quality problems that affect the customer experience release_note:fix v8.14.0 v8.15.0 Feature:SLO Team:obs-ux-management Observability Management User Experience Team labels May 10, 2024
@dominiqueclarke dominiqueclarke marked this pull request as ready for review May 10, 2024 17:14
@dominiqueclarke dominiqueclarke requested a review from a team as a code owner May 10, 2024 17:14
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@dominiqueclarke dominiqueclarke changed the title slos - fix apm group by cardinality count [SLOS] fix APM group by cardinality count May 10, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #57 / Monitoring Endpoints Beats "before all" hook in "Beats"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
slo 747 749 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/slo-schema 180 182 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
slo 765.8KB 767.4KB +1.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observability 151.2KB 151.2KB +6.0B
Unknown metric groups

API count

id before after diff
@kbn/slo-schema 180 182 +2

History

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

@kdelemme kdelemme self-requested a review May 14, 2024 17:33
Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

🚢 it

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label May 14, 2024
@dominiqueclarke dominiqueclarke merged commit d3945a3 into elastic:main May 14, 2024
20 checks passed
@dominiqueclarke dominiqueclarke deleted the fix/apm-group-by-cardinality branch May 14, 2024 18:45
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 14, 2024
## Summary
Resolves elastic#179046
Summarize your PR. If it involves visual changes include a screenshot or
gif.

Applies filters based on the selected APM indicator params to the
cardinality count query.

<img width="845" alt="Screenshot 2024-05-10 at 1 07 27 PM"
src="https://github.com/elastic/kibana/assets/11356435/7283f7cc-b141-47e2-883e-e463556e4aec">

### Testing.
1. Create mock api data via `node scripts/synthtrace continuous_rollups
--live`
2. Navigate to create SLI and choose APM latency
3. Select a service
4. Select group by as `service.name`. Cardinality count should be 1
5. Select environment all and change the group by to
`service.environment`. Cardinality count should be 2.
6. Now select a specific environment. Notice cardinality count changes
to 1.
7. Repeat this testing strategy with transaction name and transaction
type, changing the group by field to `transaction.name` or
`transaction.type`, respectively and playing around with the ALL_VALUE
versus a specific value.
8. Also make sure to test the global filter for regressions

### Release note
The cardinality count for SLOs generated from a single SLI definition
was previously incorrect for APM latency and APM availability SLIs. The
cardinality count is now accurate.

(cherry picked from commit d3945a3)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.14

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request May 14, 2024
# Backport

This will backport the following commits from `main` to `8.14`:
- [[SLOS] fix APM group by cardinality count
(#183171)](#183171)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dominique
Clarke","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-05-14T18:45:38Z","message":"[SLOS]
fix APM group by cardinality count (#183171)\n\n## Summary\r\nResolves
#179046 your PR. If
it involves visual changes include a screenshot
or\r\ngif.\r\n\r\nApplies filters based on the selected APM indicator
params to the\r\ncardinality count query.\r\n\r\n<img width=\"845\"
alt=\"Screenshot 2024-05-10 at 1 07 27
PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/11356435/7283f7cc-b141-47e2-883e-e463556e4aec\">\r\n\r\n###
Testing.\r\n1. Create mock api data via `node scripts/synthtrace
continuous_rollups\r\n--live`\r\n2. Navigate to create SLI and choose
APM latency\r\n3. Select a service\r\n4. Select group by as
`service.name`. Cardinality count should be 1\r\n5. Select environment
all and change the group by to\r\n`service.environment`. Cardinality
count should be 2.\r\n6. Now select a specific environment. Notice
cardinality count changes\r\nto 1.\r\n7. Repeat this testing strategy
with transaction name and transaction\r\ntype, changing the group by
field to `transaction.name` or\r\n`transaction.type`, respectively and
playing around with the ALL_VALUE\r\nversus a specific value.\r\n8. Also
make sure to test the global filter for regressions\r\n\r\n### Release
note\r\nThe cardinality count for SLOs generated from a single SLI
definition\r\nwas previously incorrect for APM latency and APM
availability SLIs. The\r\ncardinality count is now
accurate.","sha":"d3945a3e50eaad9f850bb4d88863e6a91026acbc","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Feature:SLO","ci:project-deploy-observability","Team:obs-ux-management","v8.14.0","v8.15.0"],"title":"[SLOS]
fix APM group by cardinality
count","number":183171,"url":"#183171
fix APM group by cardinality count (#183171)\n\n## Summary\r\nResolves
#179046 your PR. If
it involves visual changes include a screenshot
or\r\ngif.\r\n\r\nApplies filters based on the selected APM indicator
params to the\r\ncardinality count query.\r\n\r\n<img width=\"845\"
alt=\"Screenshot 2024-05-10 at 1 07 27
PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/11356435/7283f7cc-b141-47e2-883e-e463556e4aec\">\r\n\r\n###
Testing.\r\n1. Create mock api data via `node scripts/synthtrace
continuous_rollups\r\n--live`\r\n2. Navigate to create SLI and choose
APM latency\r\n3. Select a service\r\n4. Select group by as
`service.name`. Cardinality count should be 1\r\n5. Select environment
all and change the group by to\r\n`service.environment`. Cardinality
count should be 2.\r\n6. Now select a specific environment. Notice
cardinality count changes\r\nto 1.\r\n7. Repeat this testing strategy
with transaction name and transaction\r\ntype, changing the group by
field to `transaction.name` or\r\n`transaction.type`, respectively and
playing around with the ALL_VALUE\r\nversus a specific value.\r\n8. Also
make sure to test the global filter for regressions\r\n\r\n### Release
note\r\nThe cardinality count for SLOs generated from a single SLI
definition\r\nwas previously incorrect for APM latency and APM
availability SLIs. The\r\ncardinality count is now
accurate.","sha":"d3945a3e50eaad9f850bb4d88863e6a91026acbc"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#183171
fix APM group by cardinality count (#183171)\n\n## Summary\r\nResolves
#179046 your PR. If
it involves visual changes include a screenshot
or\r\ngif.\r\n\r\nApplies filters based on the selected APM indicator
params to the\r\ncardinality count query.\r\n\r\n<img width=\"845\"
alt=\"Screenshot 2024-05-10 at 1 07 27
PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/11356435/7283f7cc-b141-47e2-883e-e463556e4aec\">\r\n\r\n###
Testing.\r\n1. Create mock api data via `node scripts/synthtrace
continuous_rollups\r\n--live`\r\n2. Navigate to create SLI and choose
APM latency\r\n3. Select a service\r\n4. Select group by as
`service.name`. Cardinality count should be 1\r\n5. Select environment
all and change the group by to\r\n`service.environment`. Cardinality
count should be 2.\r\n6. Now select a specific environment. Notice
cardinality count changes\r\nto 1.\r\n7. Repeat this testing strategy
with transaction name and transaction\r\ntype, changing the group by
field to `transaction.name` or\r\n`transaction.type`, respectively and
playing around with the ALL_VALUE\r\nversus a specific value.\r\n8. Also
make sure to test the global filter for regressions\r\n\r\n### Release
note\r\nThe cardinality count for SLOs generated from a single SLI
definition\r\nwas previously incorrect for APM latency and APM
availability SLIs. The\r\ncardinality count is now
accurate.","sha":"d3945a3e50eaad9f850bb4d88863e6a91026acbc"}}]}]
BACKPORT-->

Co-authored-by: Dominique Clarke <[email protected]>
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 ci:project-deploy-observability Create an Observability project Feature:SLO release_note:fix Team:obs-ux-management Observability Management User Experience Team v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SLOs] APM indicators - Group by cardinality shows inaccurate value when APM filters are used
6 participants