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: symbolization created on user opt-in #156089

Merged
merged 6 commits into from
May 2, 2023
Merged

Profiling: symbolization created on user opt-in #156089

merged 6 commits into from
May 2, 2023

Conversation

inge4pres
Copy link
Contributor

@inge4pres inge4pres commented Apr 27, 2023 •
edited
Loading

Signed-off-by: inge4pres francesco.gualazzi@elastic.co

Summary

Include the creation of symbolizer package policy in the setup initiated by users when they opt-in Universal Profiling.

Checklist

For maintainers

Related issues or PRs

@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 elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@inge4pres inge4pres requested a review from a team April 27, 2023 20:05
@inge4pres inge4pres self-assigned this Apr 27, 2023
@inge4pres inge4pres added backport 8.8 candidate ci:build-cloud-image release_note:skip Skip the PR/issue when compiling release notes labels Apr 27, 2023
jen-huang
jen-huang reviewed Apr 27, 2023
View reviewed changes
x-pack/plugins/profiling/server/lib/setup/steps/get_fleet_policy_step.ts
package: {
name: 'profiler_symbolizer',
title: 'Universal Profiling Symbolizer',
version: '8.8.0-preview',
Copy link
Contributor

@jen-huang jen-huang Apr 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

you could consider omitting this field altogether, this will make it always install the latest symbolizer package it finds (including prerelease packages) that is compatible with this version of Kibana

this means that this version field will not need to be manually update for later releases, including patches. but it also means care should be given as to not accidentally release a package incorrectly (i.e. publishing a 8.9.0 without a >8.9.0 kibana version constraint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks 👍🏼 TIL
I'll discuss this with the team and eventually update the PR accordingly after testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jen-huang whan you say omitting this field i suppose you mean not setting package at all? Or does it mean leaving it as an empty object o null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Erased in 7d70af9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently this commit broke the tests... I'll remove it and re-run them

Copy link
Contributor

Choose a reason for hiding this comment

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

I only meant removing version: '8.8.0-preview', line :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks, good to know 😄
Will keep in mind for the next PR 👍🏼

@inge4pres inge4pres marked this pull request as ready for review May 2, 2023 14:36
@inge4pres inge4pres requested a review from a team as a code owner May 2, 2023 14:36
inge4pres and others added 4 commits May 2, 2023 17:13
@inge4pres
Profiling: symbolization created on user opt-in
05d6af9 
Signed-off-by: inge4pres <francesco.gualazzi@elastic.co>
@kibanamachine @inge4pres
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
5724d43 
…-ref HEAD~1..HEAD --fix'
@inge4pres @jen-huang
Fix package policy structure
38e40bc 
Co-authored-by: Jen Huang <its.jenetic@gmail.com>
@inge4pres
PR feedback
7d70af9 
Signed-off-by: inge4pres <francesco.gualazzi@elastic.co>
@inge4pres inge4pres force-pushed the profiling/fix-package-policy branch from 77f2dd2 to 7d70af9 Compare May 2, 2023 15:16
@inge4pres inge4pres requested a review from jen-huang May 2, 2023 15:17
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #10 / apis index_patterns has user index pattern has user index pattern API legacy index pattern api should return true if has user index pattern without data

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 398 401 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 478 481 +3
total +5

History

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

cc @inge4pres

jbcrail
jbcrail approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@jbcrail jbcrail left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jbcrail jbcrail merged commit d4f6cf7 into main May 2, 2023
@jbcrail jbcrail deleted the profiling/fix-package-policy branch May 2, 2023 18:24
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels May 2, 2023
@inge4pres inge4pres added v8.8.0 v8.9.0 and removed backport:skip This commit does not require backporting v8.9.0 labels May 2, 2023
@inge4pres
Copy link
Contributor Author

@kibanamachine backport 8.8

@jbudz jbudz added auto-backport Deprecated - use backport:version if exact versions are needed and removed backport labels May 2, 2023
@inge4pres inge4pres added the backport:prev-minor Backport to the previous minor version (i.e. one version back from main) label May 2, 2023
kibanamachine pushed a commit that referenced this pull request May 2, 2023
@inge4pres
Profiling: symbolization created on user opt-in (#156089)
e3fffe2 
Signed-off-by: inge4pres <francesco.gualazzi@elastic.co>

## Summary

Include the creation of symbolizer package policy in the setup initiated
by users when they opt-in Universal Profiling.

### Checklist

- [X]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [X] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Related issues or PRs
* #155642
* #150885

---------

Signed-off-by: inge4pres <francesco.gualazzi@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Jen Huang <its.jenetic@gmail.com>
(cherry picked from commit d4f6cf7)
@kibanamachine kibanamachine mentioned this pull request May 2, 2023
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.8

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

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request May 2, 2023
@kibanamachine @inge4pres
[8.8] Profiling: symbolization created on user opt-in (#156089) (#156449
4782455 
)

# Backport

This will backport the following commits from `main` to `8.8`:
- [Profiling: symbolization created on user opt-in
(#156089)](#156089)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Francesco
Gualazzi","email":"inge4pres@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-05-02T18:24:54Z","message":"Profiling:
symbolization created on user opt-in (#156089)\n\nSigned-off-by:
inge4pres <francesco.gualazzi@elastic.co>\r\n\r\n##
Summary\r\n\r\nInclude the creation of symbolizer package policy in the
setup initiated\r\nby users when they opt-in Universal
Profiling.\r\n\r\n### Checklist\r\n\r\n-
[X]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [X] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Related issues or PRs\r\n*
#155642
#150885:
inge4pres <francesco.gualazzi@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Jen
Huang
<its.jenetic@gmail.com>","sha":"d4f6cf703da8022aeedfabca6ff7689faf67e071","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","auto-backport","ci:build-cloud-image","8.8
candidate","v8.8.0","v8.9.0"],"number":156089,"url":"#156089:
symbolization created on user opt-in (#156089)\n\nSigned-off-by:
inge4pres <francesco.gualazzi@elastic.co>\r\n\r\n##
Summary\r\n\r\nInclude the creation of symbolizer package policy in the
setup initiated\r\nby users when they opt-in Universal
Profiling.\r\n\r\n### Checklist\r\n\r\n-
[X]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [X] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Related issues or PRs\r\n*
#155642
#150885:
inge4pres <francesco.gualazzi@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Jen
Huang
<its.jenetic@gmail.com>","sha":"d4f6cf703da8022aeedfabca6ff7689faf67e071"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#156089:
symbolization created on user opt-in (#156089)\n\nSigned-off-by:
inge4pres <francesco.gualazzi@elastic.co>\r\n\r\n##
Summary\r\n\r\nInclude the creation of symbolizer package policy in the
setup initiated\r\nby users when they opt-in Universal
Profiling.\r\n\r\n### Checklist\r\n\r\n-
[X]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [X] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Related issues or PRs\r\n*
#155642
#150885:
inge4pres <francesco.gualazzi@elastic.co>\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Jen
Huang
<its.jenetic@gmail.com>","sha":"d4f6cf703da8022aeedfabca6ff7689faf67e071"}}]}]
BACKPORT-->

Co-authored-by: Francesco Gualazzi <inge4pres@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbcrail jbcrail jbcrail approved these changes

@jen-huang jen-huang Awaiting requested review from jen-huang

Assignees

@inge4pres inge4pres

Labels
8.8 candidate auto-backport Deprecated - use backport:version if exact versions are needed backport:prev-minor Backport to the previous minor version (i.e. one version back from main) ci:build-cloud-image release_note:skip Skip the PR/issue when compiling release notes v8.8.0 v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@inge4pres @apmmachine @kibana-ci @kibanamachine @jbcrail @jen-huang @jbudz