elastic/integrationsPublic

NotificationsYou must be signed in to change notification settings
Fork545
Star318

Comments

Migrate obs-infraobs-integrations to package-spec v3 #1#8170

Merged

tommyers-elastic merged 52 commits intoelastic:mainfrom

shmsr:migrate-infraobs-ps-v3-batch-1-serverless

Oct 18, 2023

Merged

Migrate obs-infraobs-integrations to package-spec v3 #1#8170
tommyers-elastic merged 52 commits intoelastic:mainfrom
shmsr:migrate-infraobs-ps-v3-batch-1-serverless

Conversation

Copy link

Member

shmsr commentedOct 12, 2023•
edited
Loading

Migrate a batch of packages from v1/v2 to v3 package-spec. Used the following scripts and some manual changes to generate the changes in this PR:

yq_hack.zsh

#!/bin/bashset -eset -uset -o pipefailset -xpushd $1find data_stream -type f -path "*fields*" -name "*.yml" -exec yq e -i "del .[].release" {} \;find data_stream -type f -path "*fields*" -name "ecs.yml" -exec yq e -i "unique_by(.name)" {} \;find data_stream -type f -path "*fields*" -name "*.yml" -exec yq e -i "del .[].fields.[].required" {} \;yq e -i 'del .release' manifest.ymlyq e -i "with(select(.license != null); .conditions.elastic.subscription = .license) | del .license" manifest.ymlpopdgit add -u $1/git commit -m "[$1]: migration with yq"

ecs-update.zsh

#!/bin/bashset -uset -o pipefailset -xecho "-- executing ecs-update for $1 --"ecs-update -format-version=3.0.0 -fix-dotted-yaml-keys -add-owner-type -owner elastic/obs-infraobs-integrations -v -skip-format=true $1if (( $? )) then    echo "-- write validation for $1 --"    pushd $1    go run ../scripting/write_validation.go    popd    # Try again?    git checkout .    ecs-update -format-version=3.0.0 -fix-dotted-yaml-keys -add-owner-type -owner elastic/obs-infraobs-integrations -v -skip-format=true $1fi

And another Go program to write the validation.yml by parsing the elastic-package lint errors.

Then I hooked this scripts to run with:

$ pkgs=($(cat .github/CODEOWNERS| grep'@elastic/obs-infraobs-integrations'| cut -d'' -f 1| cut -d'/' -f 3| sort| uniq))$for((idx=1; idx<=15; idx++))doscripting/yq_hack.zsh$pkgs[idx]done

$for((idx=1; idx<=15; idx++))doscripting/ecs_update.zsh$pkgs[idx]||breakdone

Another variation of the above script:

$for((idx=1; idx<=15; idx++))docase$pkgs[idx]in"citrix_adc" |"azure_app_service" |"apache")echo"-- skip$pkgs[idx] --";;"coredns" |"couchbase" |"etcd" |"cockroachdb" |"ceph" |"azure_functions" |"activemq" |"apache_spark" |"apache_tomcat" |"cassandra" |"couchdb" |"airflow")echo"-- done$pkgs[idx] --";;*) scripting/ecs_update.zsh$pkgs[idx]||break;;esacdone

As I am creating a batch of 15 packages per migration, I am using a for loop. In case the migration is not possible for some packages, I put it in the first case to skip them.

Proposed commit message

Migrate packages owned by obs-infraobs-integrations but few couldn't be migrated. Marking them as follows:

activemq
airflow
apache
apache_spark
apache_tomcat
azure_app_service
azure_functions
cassandra
ceph
citrix_adc
cockroachdb
coredns
couchbase
couchdb
etcd

Please see the comment#8170 (comment) in this PR itself to understand why some packages couldn't be migrated and also learn about the challenges faced when migrating.

Checklist

I have reviewedtips 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'schangelog.yml file.
I have verified that Kibana version constraints are current according toguidelines.

shmsr requested a review froma team as acode owner

October 12, 2023 06:25

shmsr self-assigned this

Oct 12, 2023

Copy link

MemberAuthor

shmsr commentedOct 12, 2023•
edited
Loading

Here are some rough notes I recorded while migrating. I have fixed a few and accordingly improved my script. But there are 3 packages that couldn't be migrated.

airflow
- Problem with duplicate field like container.name, container.image.name in ecs and agent.yml
apache
- elastic: subscription already present (improve script)
- duplicate field tags in error datastream
- file "/Users/subhamsarkar/go/src/github.com/elastic/integrations/packages/apache/kibana/visualization/apache-22057f20-3a12-11eb-8946-296aab7b13db.json" is invalid: found legacy visualization "Uptime [Metrics Apache]" (metric, TSVB)
azure_app_service
- Error: building package failed: resolving external fields failed: can't resolve fields: field geo.continent_name cannot be reused at top level (see:#1)
ceph
- file "/Users/subhamsarkar/go/src/github.com/elastic/integrations/packages/ceph/data_stream/cluster_status/fields/fields.yml" is invalid: field 0.fields.0.fields.5.fields.5: object_type is required
  - Fixed by adding object_type: keyword (see:#1)
citrix_adc
- Error: building package failed: resolving external fields failed: can't resolve fields: field interface.id cannot be reused at top level
cockroachdb
- file "/Users/subhamsarkar/go/src/github.com/elastic/integrations/build/packages/cockroachdb-1.6.0.zip/data_stream/status/manifest.yml" is invalid: field elasticsearch.index_template.settings: Additional property index.mapping.dimension_fields.limit is not allowed
couchbase
- file "/Users/subhamsarkar/go/src/github.com/elastic/integrations/build/packages/couchbase-1.3.1.zip/data_stream/query_index/fields/fields.yml" is invalid: field 0.fields.1.fields.0: Additional property metrics_type is not allowed
  - Will add to my script
cassandra
- Normalize fields to makeelastic-package test -v --report-format xUnit happy. (see:#1)
apache_spark
- Normalize fields to makeelastic-package test -v --report-format xUnit happy. (see:#1)

shmsr requested a review fromtommyers-elastic

October 12, 2023 06:34

shmsr commented

Oct 12, 2023

View reviewed changes

packages/etcd/data_stream/metrics/fields/fields.yml OutdatedShow resolvedHide resolved

Copy link

elasticmachine commentedOct 12, 2023•
edited
Loading

💚 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-17T16:50:08.864+0000
Duration: 27 min 16 sec

Test stats 🧪

Test	Results
Failed	0
Passed	176
Skipped	0
Total	176

🤖 GitHub comments

Expand to view the GitHub comments

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

/test : Re-trigger the build.

Copy link

elasticmachine commentedOct 12, 2023•
edited
Loading

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`23/23`)	💚
Files	100.0% (`25/25`)	💚 4.545
Classes	100.0% (`25/25`)	💚 4.545
Methods	93.2% (`233/250`)	👍 11.662
Lines	95.781% (`2225/2323`)	👍 6.256
Conditionals	100.0% (`0/0`)	💚

shmsr added23 commits

October 12, 2023 13:31

[activemq]: migration with yq

b909cc5

[airflow]: migration with yq

b16b5b3

[apache]: migration with yq

48ff78f

[apache_spark]: migration with yq

264e1bd

[cassandra]: migration with yq

0cc6abb

[cockroachdb]: migration with yq

079d4c2

[couchbase]: migration with yq

82f1cee

[couchdb]: migration with yq

1d94721

[etcd]: migration with yq

a1a8b01

[activemq] - removed dotted YAML keys from manifest

b7aad09

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/activemq

[apache_spark] - removed dotted YAML keys from manifest

ca95568

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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_spark

[apache_tomcat] - removed dotted YAML keys from manifest

c0bb399

The format_version in the package manifest changed from 2.3.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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

[cassandra] - removed dotted YAML keys from manifest

4e56a84

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/cassandra

[couchdb] - removed dotted YAML keys from manifest

11c1b97

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/couchdb

[airflow] - removed dotted YAML keys from manifest

d1eca9b

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/airflow

[azure_functions] - removed dotted YAML keys from manifest

958c974

The format_version in the package manifest changed from 2.5.1 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/azure_functions

[ceph] - removed dotted YAML keys from manifest

7475ef9

The format_version in the package manifest changed from 2.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/ceph

[cockroachdb] - removed dotted YAML keys from manifest

9f3ebda

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/cockroachdb

[coredns] - removed dotted YAML keys from manifest

c664f18

The format_version in the package manifest changed from 2.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/coredns

[couchbase]: migration with yq

3062b70

[couchbase] - removed dotted YAML keys from manifest

75e0ffe

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/couchbase

[etcd] - removed dotted YAML keys from manifest

95c1151

The format_version in the package manifest changed from 1.0.0 to 3.0.0. Removeddotted YAML keys from package manifest. Added 'owner.type: elastic' to packagemanifest.[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/etcd

Add validation.yml

bf0c44e

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

What was happeing to these fields before the individual mapping ?
What did the mapping look like ?

Also, under this group, are these the only sub fields that are there since we didn't have that clarity when type was object ?

Copy link

Contributor

Choose a reason for hiding this comment

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

before, the mapping just looks like this:

"state": {  "type": "object"},

which does nothing at all. the fields are mapped using default types, which arelong andkeyword respectively. so there's no behviour change here. it's just more explicit and intentional.

Copy link

Contributor

Choose a reason for hiding this comment

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

the object is well defined in beats. i'm not sure why we never expclicity mapped them before.

type PgState struct {Count     int64  `json:"count"`StateName string `json:"state_name"`}

muthu-mps reviewed

Oct 18, 2023

View reviewed changes

packages/airflow/data_stream/statsd/fields/ecs.yml

		external: ecs
		- name: container.labels
		external: ecs
		- name: host

Copy link

Contributor

Choose a reason for hiding this comment

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

@tommyers-elastic ,@andrewkroh - Thehost group ECS field reference is included in some integrations and some integrations doesn't include the ECS reference. As thehost field is common, Is it worth adding validations for this field as well? Just a thought.
e.g.Integration not havinghost group field reference.