[apache_tomcat] - Migrate to package-spec v3 #8159

shmsr · 2023-10-11T04:57:49Z

Following is an autogenerated response from ecs-update tool:

The format_version in the package manifest changed from 2.3.0 to 3.0.0. Removed dotted YAML keys from package manifest. Added 'owner.type: elastic' to package manifest.

[git-generate]
go run github.com/andrewkroh/go-examples/ecs-update@latest -v -format-version=3.0.0 -skip-format -fix-dotted-yaml-keys -add-owner-type packages/apache_tomcat

Here's how the PR was created:

Manual steps (made for zsh as I'm using zsh specific anonymous func (){}):

# Inside packages dir and last argument is a variable i.e., use the <package_name>. If inside specific package, use `.`
$ (){find $1/data_stream -type f -path "*fields*" -name "*.yml" -exec yq e -i "del .[].release" {} \;} apache_tomcat
$ (){find $1/data_stream -type f -path "*fields*" -name "ecs.yml" -exec yq e -i "unique_by(.name)" {} \;} apache_tomcat
$ (){find $1/data_stream -type f -path "*fields*" -name "*.yml" -exec yq e -i "del .[].fields.[].required" {} \;} apache_tomcat

# Inside specific package
$ (){yq e -i 'del .release' manifest.yml}
$ (){yq e -i "with(select(.license != null); .conditions.elastic.subscription = .license) | del .license" manifest.yml}

^ For this https://github.com/elastic/ep-shell-plugins can also be used but running these as a single zsh script is also easy. Like getting the package names using the following and then looping over them and running the aforementioned commands.

$ cat .github/CODEOWNERS | grep '@elastic/obs-infraobs-integrations' | cut -d ' ' -f 1 | cut -d '/' -f 3 | sort | uniq

Next, run (inside packages dir):

$ ecs-update -format-version=3.0.0 -fix-dotted-yaml-keys -add-owner-type -owner elastic/obs-infraobs-integrations -v -skip-format=true apache_tomcat

-skip-format=true is used because after the v3 upgrade; noticed that elastic-package format fixes dotted-yaml-keys in *.yml files where it shouldn't and hence breaks the elastic-package test pipeline -g --report-format xUnit run as part of ecs-update only.

How to install ecs-update? Do: go install github.com/andrewkroh/go-examples/ecs-update@main

Add validation.yml

Proposed commit message

The format_version in the package manifest changed from 2.3.0 to 3.0.0.

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

[ ]

How to test this PR locally

Related issues

Screenshots

The format_version in the package manifest changed from 2.3.0 to 3.0.0. Removed dotted YAML keys from package manifest. Added 'owner.type: elastic' to package manifest. [git-generate] go run github.com/andrewkroh/go-examples/ecs-update@latest -v -format-version=3.0.0 -skip-format -fix-dotted-yaml-keys -add-owner-type packages/apache_tomcat

elasticmachine · 2023-10-11T05:09:19Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-10-11T05:32:21.228+0000
Duration: 23 min 9 sec

Test stats 🧪

Test	Results
Failed	0
Passed	54
Skipped	0
Total	54

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

ishleenk17 · 2023-10-11T05:13:00Z

@shmsr : Any specific reason why we are changing the spec for this package alone?
I think we would be doing a bulk change for all the infrapbs packages?

shmsr · 2023-10-11T05:19:40Z

@shmsr : Any specific reason why we are changing the spec for this package alone? I think we would be doing a bulk change for all the infraobs packages?

Yep discussed it with Tom. This is just a sample PR where I change just one package and check first. Upcoming PRs will have bulk changes (probably 10-15 packages per PR so that it is easier to review; 48 packages in total).

packages/apache_tomcat/changelog.yml

ishleenk17 · 2023-10-11T05:31:55Z

packages/apache_tomcat/validation.yml

@@ -0,0 +1,3 @@
+errors:
+  exclude_checks:
+    - SVR00004 # References in dashboards.


Can you point to the link which mentions the need of this with the latest package spec ?

https://github.com/elastic/package-spec/blob/main/code/go/pkg/specerrors/constants.go

Set of rules that can be skipped if required. Also see: Filter errors

So after the v3 migration, elastic-package build (which also lints the package) needs to pass but it cannot pass if the checks fail. So there's a set of errors although not recommended to skip; can be skipped so that elastic-package build succeeds with zero return code.

Example errors:

Error: building package failed: invalid content found in built zip package: found 3 validation errors: 1. references found in dashboard kibana/dashboard/apache_tomcat-44a8e0d0-b8f5-11ed-ac9b-cb6bcd97d223.json: apache_tomcat-d0957a70-eda4-11ed-909a-2baec7270d1f (search) (SVR00004) 2. references found in dashboard kibana/dashboard/apache_tomcat-8fd54a20-1f0d-11ee-9d6b-bb41d08322c8.json: apache_tomcat-1f3c6e30-dd11-11ed-9f4f-d97c9f37d195 (search), apache_tomcat-4d39c820-ddcd-1 1ed-8080-ddad81fe2c3c (search) (SVR00004) 3. references found in dashboard kibana/dashboard/apache_tomcat-9c66eb10-dd0c-11ed-9f4f-d97c9f37d195.json: apache_tomcat-1f3c6e30-dd11-11ed-9f4f-d97c9f37d195 (search), apache_tomcat-4d39c820-ddcd-1 1ed-8080-ddad81fe2c3c (search) (SVR00004)

Similar thing was done in this PR too.

What is the general guidance here ? These errors are here because of not having the best practices followed. Should a backlog issue be created for such error overrides and must be relooked (similar to or as part of lens migration initiative)?

cc @lalit-satapathy , @jsoriano

Looks like this code is for Visualization by Value,
So does that mean it expexts the visualiztion to be some other way ?
Can we narrow down on these issues? before skipping them ?

@ishleenk17 found some related issues that could help:

Add warning/error for dashboards including visualizations by reference package-spec#389

Warning when dashboard with visualisations by reference instead of value exist package-spec#316

Value migration script: By value migration script kibana#129303

Play with legacy_vis_analyzer elastic-package#791

Might have to use the value migration script to address this error.

Yes, the solution would be to migrate these visualizations by reference to be included by value. You can decide to delay this migration by adding this exclude rule.

yep we should make sure we track all the work required to remove these skips. but we don't have to do it as part of the package spec v3 migration.

@shmsr : Lets have a meta ticket for this effort. We can track all these skips being made as part of that ticket. So that all information can be obtained from one place.

the meta issue is here #8028

ishleenk17 · 2023-10-11T05:49:02Z

What are the testing plans for this?
System tests and pipeline tests ? These will be run as part of CI.
Any other testing is required?

elasticmachine · 2023-10-11T05:55:42Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`9/9`)	💚
Files	100.0% (`9/9`)	💚 3.346
Classes	100.0% (`9/9`)	💚 3.346
Methods	95.062% (`77/81`)	👍 2.505
Lines	79.897% (`934/1169`)	👎 -8.51
Conditionals	100.0% (`0/0`)	💚

tommyers-elastic · 2023-10-11T14:51:51Z

@ishleenk17 as long as we validate there are no functional changes to the package via code review (the only changes here are formatting and package spec), then no additional testing is required.

ishleenk17

Let's track the skips that we are making for these packages in a meta ticket.
Otherwise, looks good!

elasticmachine · 2023-10-12T07:41:38Z

Package apache_tomcat - 0.14.0 containing this change is available at https://epr.elastic.co/search?package=apache_tomcat

shmsr requested a review from a team as a code owner October 11, 2023 04:57

shmsr requested a review from tommyers-elastic October 11, 2023 05:05

shmsr commented Oct 11, 2023

View reviewed changes

packages/apache_tomcat/changelog.yml Outdated Show resolved Hide resolved

ishleenk17 reviewed Oct 11, 2023

View reviewed changes

Update packages/apache_tomcat/changelog.yml

277ffc8

tommyers-elastic approved these changes Oct 11, 2023

View reviewed changes

ishleenk17 approved these changes Oct 12, 2023

View reviewed changes

tommyers-elastic merged commit 1e8f627 into elastic:main Oct 12, 2023

jsoriano mentioned this pull request Oct 12, 2023

[apache_tomcat] Quote keys with dots in test config files #8174

Merged

4 tasks

shmsr deleted the migrate-apache-tomcat-v3-serverless branch October 12, 2023 18:26

shmsr mentioned this pull request Oct 13, 2023

Bulk package-spec v3 migration of packages for serverless #8147

Closed

shmsr self-assigned this Oct 13, 2023

tommyers-elastic mentioned this pull request Oct 16, 2023

migrate obs-infraobs-integrations owned packages to format_version: 3.0.0 #8028

Closed

96 tasks

andrewkroh added the Integration:apache_tomcat Apache Tomcat label Jul 22, 2024

[apache_tomcat] - Migrate to package-spec v3 #8159

[apache_tomcat] - Migrate to package-spec v3 #8159

Uh oh!

Conversation

shmsr commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

Uh oh!

elasticmachine commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

ishleenk17 commented Oct 11, 2023

Uh oh!

shmsr commented Oct 11, 2023

Uh oh!

Uh oh!

ishleenk17 Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

shmsr Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

agithomas Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishleenk17 Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shmsr Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

tommyers-elastic Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17 Oct 12, 2023

Choose a reason for hiding this comment

Uh oh!

tommyers-elastic Oct 16, 2023

Choose a reason for hiding this comment

Uh oh!

ishleenk17 commented Oct 11, 2023

Uh oh!

elasticmachine commented Oct 11, 2023

🌐 Coverage report

Uh oh!

tommyers-elastic commented Oct 11, 2023

Uh oh!

ishleenk17 left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 12, 2023

Uh oh!

Uh oh!

shmsr commented Oct 11, 2023 •

edited

Loading

elasticmachine commented Oct 11, 2023 •

edited

Loading

agithomas Oct 11, 2023 •

edited

Loading

ishleenk17 Oct 11, 2023 •

edited

Loading

shmsr Oct 11, 2023 •

edited

Loading