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

[Observability Onboarding Integration] breaks installed integrations page #174804

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

mgiota
Copy link
Contributor

@mgiota mgiota commented Jan 13, 2024

Fixes #174803

@mgiota mgiota self-assigned this Jan 13, 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.
  • /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!)

@@ -201,7 +201,10 @@ export function parseAndVerifyArchive(
): ArchivePackage {
// The top-level directory must match pkgName-pkgVersion, and no other top-level files or directories may be present
const logger = appContextService.getLogger();
const toplevelDir = topLevelDirOverride || paths[0].split('/')[0];
let toplevelDir = topLevelDirOverride || '';
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 fixed my issue. But since I am not aware of the Fleet/Integrations ecosystem, it might not be the correct way to fix the root of the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

the fix looks good to me, if you make the pr ready for review, the CI build will run automatically

Copy link
Member

Choose a reason for hiding this comment

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

I checked this fix locally and it solved the related error 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened it up for review!

@mgiota mgiota changed the title [Observability Onboarding] fix empty integrations issue [Observability Onboarding Integration] breaks installed integrations page Jan 13, 2024
@juliaElastic
Copy link
Contributor

buildkite, test this

@weltenwort
Copy link
Member

weltenwort commented Jan 15, 2024

Could this be a sign that the packages generated by the "custom integration" creation API are not valid anymore? Did anything change about those saved objects that needs to be fixed there?

@weltenwort
Copy link
Member

I suspect #173965 changed how the custom integrations are generated. Maybe something about the assets broke there.

@weltenwort
Copy link
Member

The root cause might be fixed by #174869.

@mgiota
Copy link
Contributor Author

mgiota commented Jan 15, 2024

@weltenwort I verify that your fix works fine! So I am closing this draft PR
Screenshot 2024-01-15 at 22 16 01

@mgiota
Copy link
Contributor Author

mgiota commented Jan 15, 2024

@weltenwort Sorry I misunderstood, your PR was not meant to replace my PR.

So I am reopening this one, since your PR fixes newly created custom integrations and this one fixes existing custom integrations which are already in a broken state. They complement each other and we need both I guess!

@mgiota mgiota reopened this Jan 15, 2024
@mgiota mgiota marked this pull request as ready for review January 15, 2024 22:49
@mgiota mgiota requested a review from a team as a code owner January 15, 2024 22:49
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@mgiota mgiota added the Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project label Jan 16, 2024
@mgiota
Copy link
Contributor Author

mgiota commented Jan 16, 2024

@juliaElastic What should be the proper labels for this PR (version and release notes)

@juliaElastic
Copy link
Contributor

@mgiota Added labels, to backport to 8.12

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

🚀

@juliaElastic
Copy link
Contributor

@elasticmachine merge upstream

@mgiota
Copy link
Contributor Author

mgiota commented Jan 16, 2024

@juliaElastic Cool! I enable auto-merge!

@mgiota mgiota enabled auto-merge (squash) January 16, 2024 09:22
@mgiota
Copy link
Contributor Author

mgiota commented Jan 16, 2024

@maryam-saeidi Could you please approve? Nothing really changed since last time you reviewed.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #5 / Description it changes the description
  • [job] [logs] Jest Tests #5 / Description shows an error when description is empty
  • [job] [logs] Jest Tests #5 / Description shows an error when description is too long

Metrics [docs]

✅ unchanged

History

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

cc @mgiota

@mgiota mgiota merged commit 8aec6ac into elastic:main Jan 16, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.12 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 174804

Questions ?

Please refer to the Backport tool documentation

@mgiota
Copy link
Contributor Author

mgiota commented Jan 16, 2024

@juliaElastic I was about it to do the manual backport now, but looks like you already took care of it! Thanks!

juliaElastic added a commit that referenced this pull request Jan 17, 2024
…tions … (#174933)

Backport #174804 to 8.12

Co-authored-by: Panagiota Mitsopoulou <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v8.12.1 v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Observability Logs Onboarding breaks installed Integrations page
8 participants