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.

Sign up for GitHub

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

Jump to bottom

[Profiling] Ignore error frames #176537

Merged
merged 20 commits into from Feb 13, 2024
Merged

[Profiling] Ignore error frames #176537

merged 20 commits into from Feb 13, 2024

Conversation

rockdaboot
Copy link
Contributor

This change allows the Universal Profiling agent to send error frames, which will give us more accurate values for CO2 emission and $ costs.

The reason is that unwinding errors resulting in 0-length stacktraces happen quite often. These are not sent to the backend currently, so the related CPU activity doesn't go into the calculations. This can make up showing 10% less CPU / CO2 / costs in the UI.
Adding artificial error frames in case of unwinding errors guarantees that stacktraces always have a length of > 0.

Once we settled on how error frames can be displayed in a user-friendly way, this code can be removed.

@rockdaboot
[Profiling] Ignore error frames
e99fb2e 
This change allows the Universal Profiling agent to send error frames, which
will give us more accurate values for CO2 emission and $ costs.

The reason is that unwinding errors resulting in 0-length stacktraces happen
quite often. These are not sent to the backend currently, so the related CPU
activity doesn't go into the calculations. This can make up showing 10% less
CPU / CO2 / costs in the UI.
Adding artificial error frames in case of unwinding errors guarantees that
stacktraces always have a length of > 0.

Once we settled on how error frames can be displayed in a user-friendly way,
this code can be removed.
@rockdaboot rockdaboot added release_note:enhancement Team:obs-ux-management Observability Management User Experience Team v8.13.0 labels Feb 8, 2024
@rockdaboot rockdaboot self-assigned this Feb 8, 2024
@rockdaboot rockdaboot requested a review from a team as a code owner February 8, 2024 17:48
@elasticmachine
Copy link
Contributor

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

@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.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@athre0z athre0z mentioned this pull request Feb 9, 2024
Closed
3 tasks
@rockdaboot rockdaboot marked this pull request as draft February 9, 2024 08:21
@rockdaboot rockdaboot marked this pull request as ready for review February 9, 2024 12:24
@rockdaboot rockdaboot requested review from a team as code owners February 9, 2024 12:24
jloleysens
jloleysens approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Core changes LGTM

cauemarcondes
cauemarcondes approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

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/profiling-utils 45 48 +3
observability 617 624 +7
total +10

Async chunks

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

id before after diff
infra 1.3MB 1.3MB +459.0B
profiling 399.1KB 399.8KB +708.0B
total +1.1KB

Page load bundle

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

id before after diff
observability 102.2KB 102.3KB +136.0B
observabilityShared 60.8KB 61.3KB +446.0B
profiling 17.5KB 18.1KB +590.0B
total +1.1KB
Unknown metric groups

API count

id before after diff
@kbn/profiling-utils 156 161 +5
observability 626 633 +7
total +12

ESLint disabled line counts

id before after diff
@kbn/profiling-utils 0 1 +1

Total ESLint disabled count

id before after diff
@kbn/profiling-utils 0 1 +1

History

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

cc @rockdaboot

mdbirnstiehl
mdbirnstiehl approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@mdbirnstiehl mdbirnstiehl left a comment

Choose a reason for hiding this comment

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

LGTM

@rockdaboot rockdaboot merged commit 79f63c2 into elastic:main Feb 13, 2024
18 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 13, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
@rockdaboot @athre0z
@cauemarcondes @CoenWarmer
[Profiling] Ignore error frames (elastic#176537)
9f4f668 
This change allows the Universal Profiling agent to send error frames,
which will give us more accurate values for CO2 emission and $ costs.

The reason is that unwinding errors resulting in 0-length stacktraces
happen quite often. These are not sent to the backend currently, so the
related CPU activity doesn't go into the calculations. This can make up
showing 10% less CPU / CO2 / costs in the UI.
Adding artificial error frames in case of unwinding errors guarantees
that stacktraces always have a length of > 0.

Once we settled on how error frames can be displayed in a user-friendly
way, this code can be removed.

---------

Co-authored-by: Joel Höner <joel@elastic.co>
Co-authored-by: Caue Marcondes <caue.marcondes@elastic.co>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
@rockdaboot @athre0z
@cauemarcondes @CoenWarmer
[Profiling] Ignore error frames (elastic#176537)
c180add 
This change allows the Universal Profiling agent to send error frames,
which will give us more accurate values for CO2 emission and $ costs.

The reason is that unwinding errors resulting in 0-length stacktraces
happen quite often. These are not sent to the backend currently, so the
related CPU activity doesn't go into the calculations. This can make up
showing 10% less CPU / CO2 / costs in the UI.
Adding artificial error frames in case of unwinding errors guarantees
that stacktraces always have a length of > 0.

Once we settled on how error frames can be displayed in a user-friendly
way, this code can be removed.

---------

Co-authored-by: Joel Höner <joel@elastic.co>
Co-authored-by: Caue Marcondes <caue.marcondes@elastic.co>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@rockdaboot @athre0z
@cauemarcondes @fkanout
[Profiling] Ignore error frames (elastic#176537)
43d2271 
This change allows the Universal Profiling agent to send error frames,
which will give us more accurate values for CO2 emission and $ costs.

The reason is that unwinding errors resulting in 0-length stacktraces
happen quite often. These are not sent to the backend currently, so the
related CPU activity doesn't go into the calculations. This can make up
showing 10% less CPU / CO2 / costs in the UI.
Adding artificial error frames in case of unwinding errors guarantees
that stacktraces always have a length of > 0.

Once we settled on how error frames can be displayed in a user-friendly
way, this code can be removed.

---------

Co-authored-by: Joel Höner <joel@elastic.co>
Co-authored-by: Caue Marcondes <caue.marcondes@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahzad31 shahzad31 shahzad31 approved these changes

@athre0z athre0z athre0z approved these changes

@jloleysens jloleysens jloleysens approved these changes

@cauemarcondes cauemarcondes cauemarcondes approved these changes

@mdbirnstiehl mdbirnstiehl mdbirnstiehl approved these changes

Assignees

@rockdaboot rockdaboot

Labels
backport:skip This commit does not require backporting release_note:enhancement Team:obs-ux-management Observability Management User Experience Team v8.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@rockdaboot @elasticmachine @apmmachine @kibana-ci @shahzad31 @athre0z @jloleysens @cauemarcondes @mdbirnstiehl @kibanamachine