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

[Profiling] Differential views based on same data show different values #182001

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

cauemarcondes
Copy link
Contributor

@cauemarcondes cauemarcondes commented Apr 29, 2024

Related to #180677
closes #181886

There's still a small difference on the CO2 values, but that's due to different formats returned by ES apis:

TopN Functions
"self_annual_co2_tons": 0.0068555964801996295

Flamegraph
"AnnualCO2TonsInclusive": [
    0.0069,

After:
Screenshot 2024-04-29 at 16 34 57
Screenshot 2024-04-29 at 16 35 03

@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v8.14.0 labels Apr 29, 2024
@cauemarcondes cauemarcondes requested a review from a team as a code owner April 29, 2024 15:41
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Apr 29, 2024
@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!)

Copy link
Contributor

@rockdaboot rockdaboot left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -173,7 +173,7 @@ export function DifferentialTopNFunctionsView() {
baseValue={
state.data
? {
totalCount: state.data.TotalCount,
totalCount: state.data.selfCPU,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this camelCased and TotalCount Pascal Cased?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the first version of the implementation. I'm slowly changing it.

danielmitterdorfer added a commit to danielmitterdorfer/elasticsearch that referenced this pull request Apr 30, 2024
Typically double values are rounded in profiling APIs. However, we have
missed `self_annual_co2_tons` and `self_annual_cost_usd` in the TopN
functions API. With this commit we also round these two values according
to the existing convention.

Relates elastic/kibana#182001
@danielmitterdorfer
Copy link
Member

There's still a small difference on the CO2 values, but that's due to different formats returned by ES apis:

This is addressed by elastic/elasticsearch#108054.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Apr 30, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #47 / alerting api integration security and spaces enabled Alerts - Group 1 alerts backfill rule runs ad hoc backfill task should handle timeouts

Metrics [docs]

Async chunks

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

id before after diff
profiling 407.9KB 407.9KB -6.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5874 +5874
total size - 6.7MB +6.7MB

History

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

@cauemarcondes cauemarcondes merged commit cb77c2d into elastic:main Apr 30, 2024
20 checks passed
@cauemarcondes cauemarcondes deleted the profiling-diff-view branch April 30, 2024 09:47
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 30, 2024
…es (elastic#182001)

Related to elastic#180677
closes elastic#181886

There's still a small difference on the CO2 values, but that's due to
different formats returned by ES apis:
```
TopN Functions
"self_annual_co2_tons": 0.0068555964801996295

Flamegraph
"AnnualCO2TonsInclusive": [
    0.0069,
```

After:
<img width="1765" alt="Screenshot 2024-04-29 at 16 34 57"
src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188">
<img width="1788" alt="Screenshot 2024-04-29 at 16 35 03"
src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67">

(cherry picked from commit cb77c2d)
@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

delanni added a commit that referenced this pull request Apr 30, 2024
## Summary
The PR #182001 was verified at a
point where typechecking was missing for a bit, this allowed a missing
field to slip in. This PR tries to fix it.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 30, 2024
## Summary
The PR elastic#182001 was verified at a
point where typechecking was missing for a bit, this allowed a missing
field to slip in. This PR tries to fix it.

(cherry picked from commit 370a7f5)
kibanamachine added a commit that referenced this pull request Apr 30, 2024
…nt values (#182001) (#182091)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Profiling] Differential views based on same data show different
values (#182001)](#182001)

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

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

<!--BACKPORT [{"author":{"name":"Cauê
Marcondes","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-04-30T09:47:33Z","message":"[Profiling]
Differential views based on same data show different values
(#182001)\n\nRelated to
https://github.com/elastic/kibana/pull/180677\r\ncloses
https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a
small difference on the CO2 values, but that's due to\r\ndifferent
formats returned by ES apis:\r\n```\r\nTopN
Functions\r\n\"self_annual_co2_tons\":
0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\":
[\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\"
alt=\"Screenshot 2024-04-29 at 16 34
57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img
width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","ci:project-deploy-observability","v8.14.0","v8.15.0"],"title":"[Profiling]
Differential views based on same data show different
values","number":182001,"url":"https://github.com/elastic/kibana/pull/182001","mergeCommit":{"message":"[Profiling]
Differential views based on same data show different values
(#182001)\n\nRelated to
https://github.com/elastic/kibana/pull/180677\r\ncloses
https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a
small difference on the CO2 values, but that's due to\r\ndifferent
formats returned by ES apis:\r\n```\r\nTopN
Functions\r\n\"self_annual_co2_tons\":
0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\":
[\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\"
alt=\"Screenshot 2024-04-29 at 16 34
57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img
width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541"}},"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":"https://github.com/elastic/kibana/pull/182001","number":182001,"mergeCommit":{"message":"[Profiling]
Differential views based on same data show different values
(#182001)\n\nRelated to
https://github.com/elastic/kibana/pull/180677\r\ncloses
https://github.com/elastic/kibana/issues/181886\r\n\r\nThere's still a
small difference on the CO2 values, but that's due to\r\ndifferent
formats returned by ES apis:\r\n```\r\nTopN
Functions\r\n\"self_annual_co2_tons\":
0.0068555964801996295\r\n\r\nFlamegraph\r\n\"AnnualCO2TonsInclusive\":
[\r\n 0.0069,\r\n```\r\n\r\nAfter:\r\n<img width=\"1765\"
alt=\"Screenshot 2024-04-29 at 16 34
57\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188\">\r\n<img
width=\"1788\" alt=\"Screenshot 2024-04-29 at 16 35
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67\">","sha":"cb77c2d13eecf92b064328605a34737222fed541"}}]}]
BACKPORT-->

Co-authored-by: Cauê Marcondes <[email protected]>
danielmitterdorfer added a commit to elastic/elasticsearch that referenced this pull request Apr 30, 2024
Typically double values are rounded in profiling APIs. However, we have
missed `self_annual_co2_tons` and `self_annual_cost_usd` in the TopN
functions API. With this commit we also round these two values according
to the existing convention.

Relates elastic/kibana#182001
danielmitterdorfer added a commit to danielmitterdorfer/elasticsearch that referenced this pull request Apr 30, 2024
Typically double values are rounded in profiling APIs. However, we have
missed `self_annual_co2_tons` and `self_annual_cost_usd` in the TopN
functions API. With this commit we also round these two values according
to the existing convention.

Relates elastic/kibana#182001
elasticsearchmachine pushed a commit to elastic/elasticsearch that referenced this pull request Apr 30, 2024
Typically double values are rounded in profiling APIs. However, we have
missed `self_annual_co2_tons` and `self_annual_cost_usd` in the TopN
functions API. With this commit we also round these two values according
to the existing convention.

Relates elastic/kibana#182001
kibanamachine added a commit that referenced this pull request Apr 30, 2024
# Backport

This will backport the following commits from `main` to `8.14`:
- [[Fix] Add missing totalSeconds field
(#182093)](#182093)

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

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

<!--BACKPORT [{"author":{"name":"Alex
Szabo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-04-30T10:09:02Z","message":"[Fix]
Add missing totalSeconds field (#182093)\n\n## Summary\r\nThe PR
#182001 was verified at a\r\npoint
where typechecking was missing for a bit, this allowed a
missing\r\nfield to slip in. This PR tries to fix
it.","sha":"370a7f5ec625fec1d082b331bfe431b68ed63688","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","ci:project-deploy-observability","v8.14.0","v8.15.0"],"title":"[Fix]
Add missing totalSeconds
field","number":182093,"url":"https://github.com/elastic/kibana/pull/182093","mergeCommit":{"message":"[Fix]
Add missing totalSeconds field (#182093)\n\n## Summary\r\nThe PR
#182001 was verified at a\r\npoint
where typechecking was missing for a bit, this allowed a
missing\r\nfield to slip in. This PR tries to fix
it.","sha":"370a7f5ec625fec1d082b331bfe431b68ed63688"}},"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":"https://github.com/elastic/kibana/pull/182093","number":182093,"mergeCommit":{"message":"[Fix]
Add missing totalSeconds field (#182093)\n\n## Summary\r\nThe PR
#182001 was verified at a\r\npoint
where typechecking was missing for a bit, this allowed a
missing\r\nfield to slip in. This PR tries to fix
it.","sha":"370a7f5ec625fec1d082b331bfe431b68ed63688"}}]}] BACKPORT-->

Co-authored-by: Alex Szabo <[email protected]>
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request May 2, 2024
…es (elastic#182001)

Related to elastic#180677
closes elastic#181886

There's still a small difference on the CO2 values, but that's due to
different formats returned by ES apis:
```
TopN Functions
"self_annual_co2_tons": 0.0068555964801996295

Flamegraph
"AnnualCO2TonsInclusive": [
    0.0069,
```

After:
<img width="1765" alt="Screenshot 2024-04-29 at 16 34 57"
src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188">
<img width="1788" alt="Screenshot 2024-04-29 at 16 35 03"
src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67">
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request May 2, 2024
## Summary
The PR elastic#182001 was verified at a
point where typechecking was missing for a bit, this allowed a missing
field to slip in. This PR tries to fix it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Profiling] Differential views based on same data show different values
7 participants