Skip to content

import-beats: Set var properties#334

Merged
mtojek merged 15 commits intoelastic:masterfrom
mtojek:set-var-properties
Apr 14, 2020
Merged

import-beats: Set var properties#334
mtojek merged 15 commits intoelastic:masterfrom
mtojek:set-var-properties

Conversation

@mtojek
Copy link
Copy Markdown
Contributor

@mtojek mtojek commented Apr 9, 2020

This PR adjusts the import-beats script:

Changes:

  • write down variable properties
  • custom order of properties based on their importance
  • replaced []map[string]interface{} with util.Variable for having properties defined in order
  • temporarily hack go.mod to use fork for go-ucfg (issue: panic: reflect: call of reflect.Value.Type on zero Value go-ucfg#159)
  • adjust docs/api/package.json due to changes in util.Dataset (failing mage test)

Closes: #318

@mtojek mtojek requested a review from ruflin April 9, 2020 14:05
@mtojek mtojek self-assigned this Apr 9, 2020
@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Apr 9, 2020

@ruflin

I'm afraid that there is a bug in the go-ucfg that prevents me from merging this change.

While unpacking in the mage build step, there is a problem with the following structure:

title: Openmetrics collector metrics
release: beta
type: metrics
streams:
- input: openmetrics/metrics
  vars:
  - name: hosts
    type: text
    title: Hosts
    multi: true
    required: true
    show_user: true
    default:
    - localhost:9090
  - name: metrics_filters.exclude
    type: text
    title: Metrics Filters Exclude
    multi: true
    required: true
    show_user: true
    default: []

The problem is with default: []:

panic: reflect: call of reflect.flag.mustBeExported on zero Value

goroutine 1 [running]:
reflect.flag.mustBeExportedSlow(0x0)
	/usr/local/Cellar/go/1.13.4/libexec/src/reflect/value.go:222 +0xad
reflect.flag.mustBeExported(...)
	/usr/local/Cellar/go/1.13.4/libexec/src/reflect/value.go:216
reflect.Value.Set(0x11b22e0, 0xc00038a708, 0x194, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.13.4/libexec/src/reflect/value.go:1532 +0x56
github.com/elastic/go-ucfg.reifyGetField(0xc000398cc0, 0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11b014b, 0x7, 0x11b22e0, 0xc00038a708, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:323 +0x2c0
github.com/elastic/go-ucfg.reifyStruct(0xc000273e80, 0x11dc420, 0xc00000a420, 0x199, 0xc000398cc0, 0x0, 0x0)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:281 +0x5e0
github.com/elastic/go-ucfg.reifyMergeValue(0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11dc420, 0xc00000a420, 0x199, 0x122d740, 0xc000398cc0, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:469 +0xe2c
github.com/elastic/go-ucfg.reifyDoArray(0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11a12c0, 0xc00035ce60, 0x97, 0x122e9c0, 0x11dc420, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:567 +0x163
github.com/elastic/go-ucfg.reifySliceMerge(0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11a12c0, 0xc00038a610, 0x197, 0x122e9c0, 0x11a12c0, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:554 +0x247
github.com/elastic/go-ucfg.reifyMergeValue(0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11a12c0, 0xc00038a610, 0x197, 0x122d740, 0xc000398b70, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:475 +0xf0c
github.com/elastic/go-ucfg.reifyGetField(0xc000398b40, 0xc000273e80, 0x0, 0x0, 0x0, 0x0, 0x11a5d18, 0x4, 0x11a12c0, 0xc00038a610, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:318 +0x248
github.com/elastic/go-ucfg.reifyStruct(0xc000273e80, 0x11d2e80, 0xc00038a5a0, 0x199, 0xc000398b40, 0x0, 0x0)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:281 +0x5e0
github.com/elastic/go-ucfg.reifyMergeValue(0xc000273e80, 0x0, 0xc000399470, 0x1, 0x1, 0x11d2e80, 0xc00038a5a0, 0x199, 0x122d740, 0xc000398b40, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:469 +0xe2c
github.com/elastic/go-ucfg.reifyDoArray(0xc000273e80, 0x0, 0xc000399470, 0x1, 0x1, 0x11a1280, 0xc00035ce20, 0x97, 0x122e9c0, 0x11d2e80, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:567 +0x163
github.com/elastic/go-ucfg.reifySliceMerge(0xc000273e80, 0x0, 0xc000399470, 0x1, 0x1, 0x11a1280, 0xc0000b7ef0, 0x197, 0x122e9c0, 0x11a1280, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:554 +0x247
github.com/elastic/go-ucfg.reifyMergeValue(0xc000273e80, 0x0, 0xc000399470, 0x1, 0x1, 0x11a1280, 0xc0000b7ef0, 0x197, 0x122d740, 0xc000398b10, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:475 +0xf0c
github.com/elastic/go-ucfg.reifyGetField(0xc000357bc0, 0xc000273e80, 0x0, 0xc000399470, 0x1, 0x1, 0x11b7b79, 0x7, 0x11a1280, 0xc0000b7ef0, ...)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:318 +0x248
github.com/elastic/go-ucfg.reifyStruct(0xc000273e80, 0x11dc2e0, 0xc0000b7e00, 0x199, 0xc000357bc0, 0x0, 0x0)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:281 +0x5e0
github.com/elastic/go-ucfg.reifyInto(0xc000273e80, 0x11b4340, 0xc0000b7e00, 0x16, 0xc000357bc0, 0xa0, 0x11dc2e0)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:161 +0x3fb
github.com/elastic/go-ucfg.(*Config).Unpack(0xc000357bc0, 0x11b4340, 0xc0000b7e00, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/marcin.tojek/go/pkg/mod/github.com/elastic/go-ucfg@v0.7.0/reify.go:144 +0x103
github.com/elastic/package-registry/util.NewDataset(0xc000377c80, 0x71, 0xc00012ce00, 0xc000377c80, 0x71, 0x0)
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/util/dataset.go:91 +0x266
github.com/elastic/package-registry/util.(*Package).LoadDataSets(0xc00012ce00, 0xc0000e73eb, 0x11, 0x11eabab, 0x1)
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/util/package.go:327 +0x646
main.buildPackage(0xc0000ae2e0, 0xe, 0xc00001b310, 0x5, 0xc00001b330, 0xb, 0xc000307550, 0xc00001b368, 0x5, 0xc000360020, ...)
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/dev/generator/main.go:169 +0x675
main.BuildPackages(0x7ffeefbff959, 0x15, 0xc0000ae2e0, 0xe, 0xe, 0x10f5082)
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/dev/generator/main.go:121 +0x2be
main.Build(0x7ffeefbff959, 0x15, 0x7ffeefbff97a, 0x8, 0x0, 0x0)
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/dev/generator/main.go:70 +0xa3
main.main()
	/Users/marcin.tojek/go/src/github.com/elastic/package-registry/dev/generator/main.go:62 +0x2c7
exit status 2
Error: running "go run ./dev/generator/ -sourceDir=./dev/packages/beats/ -publicDir=./public -tarGz=true" failed with exit code 1

I would expect here []interface{} or nil.

@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Apr 9, 2020

I submitted an issue: elastic/go-ucfg#159

@ruflin
Copy link
Copy Markdown
Collaborator

ruflin commented Apr 14, 2020

Overall the change LGTM, but I would like to have it split up into 2 PR's. One for the go mod updates and the rest if possible so we can separate the two.

+1 on getting vendor check as part of mage check.

How many places do we have where we have default: []? Wondering if there is a way to get this PR in and not be blocked by the ucfg PR (BTW, glad you directly opened a PR with a fix).

@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Apr 14, 2020

One for the go mod updates and the rest if possible so we can separate the two.

Ok, but it will take time to split.

+1 on getting vendor check as part of mage check.

Ok, will do.

How many places do we have where we have default: []?

I can't give you a straightforward answer at the moment, how many items have default: []. The problem is that it can reoccur anytime if we update the Beats source.

Wondering if there is a way to get this PR in and not be blocked by the ucfg PR

I enforced using the forked branch, but I would like to see a quick fix or workaround pushed.

@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Apr 14, 2020

PR for processing vendor directory: #341

Copy link
Copy Markdown
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM.

It is great to ahve some default title inside. We should make sure in our package checklist somewhere is to check it looks good in the UI.

}

func toVariableTitle(name string) string {
name = strings.ReplaceAll(name, "_", " ")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice trick :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should make sure in our package checklist somewhere is to check it looks good in the UI.

https://github.com/elastic/package-registry/blob/master/CONTRIBUTING.md
Point no 7: Review titles and descriptions in manifest files

@@ -2,9 +2,9 @@
github.com/blang/semver
# github.com/davecgh/go-spew v1.1.0
github.com/davecgh/go-spew/spew
# github.com/elastic/go-ucfg v0.7.0
# github.com/elastic/go-ucfg v0.8.3 => github.com/mtojek/go-ucfg v0.8.4-0.20200409161607-b87b280107a8
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lets not forget to revert this when your PR gets merged.

@mtojek mtojek merged commit f2e9799 into elastic:master Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL integration: additional var properties for UI

2 participants